material-theme / vsc-material-theme

Material Theme, the most epic theme for Visual Studio Code
https://www.material-theme.dev
Other
11.21k stars 789 forks source link

Issues with recent updates in Python schema #436

Closed aldanor closed 9 years ago

aldanor commented 9 years ago

I just took the latest master with Python fixes for a spin and noticed a few things that seem off:

image

Here's Tomorrow 80s:

image

Here's github:

hello = 'world'
(x, y, z) = (int, float, dict)
print('{}, {}'.format(hello, 'world'))
TypeError('{} {}'.format(hello, 'world'))
if not isinstance(x, X) or not issubclass(X, Y)
{k: f(hello, 'world', k, v) for k, v in x.items()}

Thanks!

equinusocio commented 9 years ago

Thanks @aldanor i will check for this.

equinusocio commented 9 years ago

This is what you'll see from the next release, make sure you using the latest build:

screen shot 2015-09-19 at 22 16 31
aldanor commented 9 years ago

@equinusocio hmm, this highlighting looks quite wrong to me actually.

equinusocio commented 9 years ago

I cant style some elements because the package doesn't give me the scope. I think that the python syntax have some issue with the new system.

evenicoulddoit commented 9 years ago

Hey - unfortunately I think the recent changes have actually made things quite significantly worse.

The most obvious one is the inability to distinguish between variables and strings within function calls, below:

image

ghost commented 9 years ago

I'm in the same boat with @evenicoulddoit. Had to switch color schemes because it was unbearable. Strings and variable names were all the same color.

equinusocio commented 9 years ago

@evenicoulddoit i need some snippet code

aldanor commented 9 years ago

I think tomorrow night could be a good reference here in general (re: what should be highlighted and what things can be of the same colour) since it got Python highlighting almost completely right.

evenicoulddoit commented 9 years ago

@equinusocio what would you like? The above snippet was:

url(r'^home/$',
    views.CustomerLoginHome.as_view(),
    name='home')
equinusocio commented 9 years ago

I think this is related with the python syntax because i get one scope for all the "views" string:

screen shot 2015-09-27 at 11 29 03
evenicoulddoit commented 9 years ago

Unfortunately I don't know enough about the construct of the syntax highlighting to comment, though both strings mentioning "home" should be noticeably different from the views.CustomerLoginHome.as_view() method. Its definitely possible because I switched to another scheme which differentiates between the two

equinusocio commented 9 years ago

@evenicoulddoit Yes i can make strings different, but the major issue is the "views.Customer... " that i can't fix. What color do you like for strings?

evenicoulddoit commented 9 years ago

image

I think green works well for strings, and perhaps just white for variables used within functions?

equinusocio commented 9 years ago

@evenicoulddoit do you like this?

screen shot 2015-09-30 at 19 38 03
evenicoulddoit commented 9 years ago

@equinusocio Yeh, that works :)

tcwalther commented 9 years ago

@equinusocio Yes, that is much better. +1 on the issue, and also +1 on your latest proposed solution!

equinusocio commented 9 years ago

Check the latest release

evenicoulddoit commented 9 years ago

I think this is a big improvement for Python:

image

But JavaScript seems almost entirely blue:

image

That might be a separate issue, and I can't remember if its a regression.

equinusocio commented 9 years ago

@evenicoulddoit Paste this JS snippet please. What ST build are you using? i remember that from 3085 build sublime text use a new syntax system and this theme support only this new system. There is no legacy support for the oldest syntax system because there is a lot of changes in the native language scopes.

Revod commented 9 years ago

Here a pic of my js file, st build 3083 captura de pantalla de 2015-10-11 10 35 30 captura de pantalla de 2015-10-11 10 37 03

equinusocio commented 9 years ago

I need a portion of your code snippet

Revod commented 9 years ago
            // window resize
            $(window).bind('resize.elastislide', function( event ) {

                // set values again
                instance._setCurrentValues();

                // need to resize items
                if( instance.visibleWidth < instance.options.minItems * ( instance.options.imageW + 2 * instance.options.border ) + ( instance.options.minItems - 1 ) * instance.options.margin ) {
                    instance._setDim( ( instance.visibleWidth - ( instance.options.minItems - 1 ) * instance.options.margin ) / instance.options.minItems );
                    instance._setCurrentValues();
                    instance.fitCount   = instance.options.minItems;
                }   
                else{
                    instance._setDim();
                    instance._setCurrentValues();
                }

                instance.$slider.css({
                    width   : instance.sliderW + 10 // TODO: +10px seems to solve a firefox "bug" :S
                });

                // slide to the current element
                clearTimeout( instance.resetTimeout );
                instance.resetTimeout   = setTimeout(function() {
                    instance._slideToCurrent();
                }, 200);

            });

            // navigation buttons events
            this.$navNext.bind('click.elastislide', function( event ) {
                instance._slide('right');
            });

            this.$navPrev.bind('click.elastislide', function( event ) {
                instance._slide('left');
            });

            // item click event
            this.$items.bind('click.elastislide', function( event ) {
                instance.options.onClick( $(this) );
                //return false;
            });

            // touch events
            instance.$slider.touchwipe({
                wipeLeft            : function() {
                    instance._slide('right');
                },
                wipeRight           : function() {
                    instance._slide('left');
                }
            });

        }
equinusocio commented 9 years ago

Ok same thing.

What ST build are you using? i remember that from 3085 build sublime text use a new syntax system and this theme support only this new system. There is no legacy support for the oldest syntax system because there is a lot of changes in the native language scopes.

screen shot 2015-10-11 at 18 02 12
Revod commented 9 years ago

I update sublime text 3 to the dev 3095 and work 100% captura de pantalla de 2015-10-11 11 57 45

evenicoulddoit commented 9 years ago

Belle, belle!

image

I understand that its a lot more work to support older versions of Sublime, but bear in mind that 3095 is a bleeding edge version, and that its unlikely most will have it. Perhaps you could add a message to install.txt so that people won't be surprised when it doesn't look as expected?

evenicoulddoit commented 9 years ago

The only criticism I have now with the Python scheme is the lack of distinction between a function declaration and a function invocation, e.g.:

image

Here's what Monokai looks like as a reference: image

ghost commented 9 years ago

So the JS fixes are present in Develop or also in Master? Cause I just pulled the latest master into ST3 and JS still looks bad.

equinusocio commented 9 years ago

@Fossil01 this theme support only the syntax-system used from build 3085+

ghost commented 9 years ago

Ah that looks much better :-) One more question. The sidebar divider is gone, it now blends into the editor field. Can I add that back in? Same goes for statusbar/searchbar.

equinusocio commented 9 years ago

@Fossil01 The sidebar separator never existed :dango:

niosus commented 9 years ago

Hm... I guess c++ is still broken, And I guess it is related... Issue #444 (even though it is closed) is not resolved.

UPD: will provide a snippet if needed

equinusocio commented 9 years ago

@niosus #444 is closed becase is related to this "issue". And to fix this you have to update ST.