Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PowerShell grammar categorizing member values differently. #160

Open
alexr00 opened this issue Feb 18, 2019 · 15 comments
Open

PowerShell grammar categorizing member values differently. #160

alexr00 opened this issue Feb 18, 2019 · 15 comments

Comments

@alexr00
Copy link

alexr00 commented Feb 18, 2019

From @cblackuk on February 18, 2019 10:16

  • VSCode Version: >1.27.2
  • OS Version: Win10, 2016, 2019 - does not matter

Steps to Reproduce:

On VSC with no extensions 1.27.2, using an older version of the PowerShell grammar Name is classified as entity.name.function.invocation.powershell.
image

On VSC with no extensions >1.27.2, using a more recent version of the PowerShell grammar Name is classified as variable.other.member.powershell
image

Was this change intentional? It changes the way the code looks in VS Code since the built in themes don't use variable.other.memeber

Copied from original issue: microsoft/vscode#68910

@alexr00 alexr00 changed the title PowerShell syntax highlighting issues for dot notation. PowerShell grammar categorizing member values differently. Feb 18, 2019
@cblackuk
Copy link

cblackuk commented Feb 18, 2019

Thanks @alexr00 for moving it to the potentially correct repo.

Does that mean that it is an actual issue? I believe it is as it makes the code nigh on impossible to read... and in old versions of VSC it still works so...?

@alexr00
Copy link
Author

alexr00 commented Feb 18, 2019

I'm not sure if the change in the PowerShell grammar is intentional. I've updated the first comment above to include some extra details for the owners of the grammar. If the change is intentional, then maybe there is an alternate theme that uses the new token names.

@cblackuk
Copy link

Thank you ever so much for looking into it @alexr00

Any workaround would be a godsend at this point as I am almost at the point of moving to Atom or other alternatives as writing PowerShell code in VSC has become almost impossible because of this change.

@msftrncs
Copy link
Contributor

@cblackuk, The way the grammar works now is actually the more correct scoping that's accepted by other languages as well. A theme can better support the original coloring if that was preferred, by adding a specific scope color for 'variable.other.member', however an even better scope that's used by other langauges is 'variable.other.object.property', and 'variable.other.property' but most themes I have seen color that the same as the variable. The 'invocation' scope would be better used for a function invocation and not for property access, and that is why its no longer used.

If you don't mind experimenting, there are 2 open PR's in this repository that I have authored that extend this area significantly, and I would love feedback.

#155 and #156.
A VS Code ready JSON file can be obtained from: https://github.com/msftrncs/PowerShell.tmLanguage/tree/wip_goal (#155) or https://github.com/msftrncs/PowerShell.tmLanguage/tree/argumentmode_2ndtry (#156). You can install the JSON file over the one at ~\AppData\Local\Programs\Microsoft VS Code\resources\app\extensions\powershell\syntaxes and then reload VSCode. Be sure to post comments about these versions in the appropriate PR. Also, understand they are Work In Progress so there is some parts that are not working correctly, and I have open issues back in VS Code regarding some issues that cause the grammar not to work correctly (compared to other editors that can use the same grammar file). They may also work better with a more complete theme.

I have been customizing the Monokai Dimmed theme for better support in several languages including Batch, PowerShell, JS/JSON/CSON, TypeScript and C#. It doesn't hurt to try some other themes to see if they provide results more as you are expecting.

@msftrncs
Copy link
Contributor

FYI, this is the same as #150.

@cblackuk
Copy link

cblackuk commented Feb 18, 2019

People there are saying that they have tried different themes and nothing happened... any chance we can maybe roll these changes back? I might be wrong but I am yet to see one person who sees this as an improvement... if anything it makes a lot of people move to different tools...

@msftrncs
Copy link
Contributor

@cblackuk, if you were to add the following to your settings file, you could restore what you originally experienced. (I might not have the right theme or the right color, so modify as needed.)

    "editor.tokenColorCustomizations": {
        "[Default Dark+]": {
            "textMateRules": [
                {
                    "scope": [
                        "variable.other.member",
                    ],
                    "settings": {
                        "foreground": "#CE6700"
                    },
                }
            ]
        }

However, if you use any other language with VS Code, namely JavaScript or TypeScript or C#, you will notice that those syntaxes will color the variable and all but the final property the same variable color. The above snippet will not change them, because they generally use the scopes I referenced above, 'variable.other.(object|property)' and the themes tend to color all 'variable.other' the same, except maybe 'variable.other.property'.

Here is a comment that didn't agree with the 'entity.name.function' scoping, #107 (comment).

In my PR's, I have the scoping set to use the same as JS/TS/C# (as closely as they all agree) and also include 'variable.function' for method access (same as JS/TS/C#). In many themes, 'variable.other.property' does color differently, but often the same as a keyword, which I did not appreciate, so hence why I customized a theme to meet my desires: https://github.com/msftrncs/theme-monokai-less-dimmed; but I did not color members any differently.

@msftrncs
Copy link
Contributor

A reference, the original post's code via PR #156 and the theme mentioned above:
image

The final properties do stand out, but if there had been others chained, they would not:
image

@cblackuk
Copy link

@msftrncs I will try to put my custom settings and thanks for sending that over... question that is still in my mind though...

The colouring scheme originally matched mostly PowerShell ISE which is how PowerShell SHOULD be highlighted - it was created by Microsoft and I always believed that the creators did a great job over there - why break it?

@msftrncs
Copy link
Contributor

That reminds me, I need to spend a little time with ISE. Ohh… ISE only colors the variable... Whether fortunate or not, I didn't start using PowerShell until I ran in to VS Code, so I've used ISE very little. Something to be noted, ISE uses a statement style syntax coloring, where the current VS Code/EditorSyntax grammar is based more or less on keyword only syntax coloring, hence PR #156.

However, as VS Code is a much larger environment than ISE, PowerShell should not be a unique language (at the point of the grammar decoder). PowerShell is very much related to C# because of its .NET base, and its very likely that someone working in PowerShell may eventually use C#, since it would be natural to move PowerShell functions to C# (and then made as a CMDLET to PowerShell, and someone has already provided a converter for doing just that) for improving speed. At that point, I don't think PowerShell should be highlighted only one way, because that would be to say the same for all languages. In reality, the IDE has typically set the appearance of a language (or all of them that it supports). ISE has one appearance, VSCode has another, Sublime yet another, Atom again another, and so on,

In this case it would be better to provide specific theme support to match PowerShell ISE while the grammar scopes things in a way compatible with other languages the editor may be used with along with externally provided themes. There is a good possibility that the current PowerShell ISE theme hasn't kept up, since its provided by EditorServices instead of EditorSyntax. This is not to mention that a Dark version of the PowerShell ISE theme is not included, which might lead to the confusion as well.

I have said this elsewhere, the purpose of the grammar/syntax is to syntax the language as detailed as possible, while the purpose of the theme is to make it pleasant to your eyes.

You can create language specific rules in a theme, by using a combination scope selector that specifies the root scope, in this case 'powershell'. I have done this for instances where I don't think the grammar has scoped things the best, and I twist the highlighting back to the way I want to see it.

@cblackuk
Copy link

As much as I would love to agree, most people who started writing in PS - started with ISE as it was the only tool back in the day and we are accustomed to the way it highlights items after ".". I do not really care for the theme... I care that I cannot distinguish when I am using something after the dot as it is just flood of one colour and it is a mess... Whilst in ISE and in VSC before it was clear to see...

@cblackuk
Copy link

cblackuk commented Feb 19, 2019

image
That is how ISE does it, and I would have loved to see it like that. Your custom code actually does that for me now :) Thank you for that! I wish we could go back to what it was before but a "workaround" helps me a ton!

    "workbench.colorTheme": "PowerShell ISE",
    "editor.tokenColorCustomizations": {
        "[PowerShell ISE]": {
            "textMateRules": [
                {
                    "scope": [
                        "variable.other.member",
                    ],
                    "settings": {
                        "foreground": "#000000"
                    },
                }
            ]
        }
    }

VSCode now:
image

VSCode before changes:
image

@cblackuk
Copy link

@msftrncs One last thing I am missing is the "." itself. From the screenshots above you can see that "." in VSC is black but the "." in ISE is gray like "-not" "!" etc... any idea what can I change to make it a different colour?

@msftrncs
Copy link
Contributor

msftrncs commented Feb 19, 2019

@cblackuk, unfortunately the current grammar here does not separate the dot "." (accessor). Its been mentioned in a couple of issues. Both my PR's resolve that, as l well as allow the accessor to flow to the next line like PowerShell actually allows. The accessor becomes scoped 'punctuation.accessor' in most languages. I include the :: static accessor in that as well, which I don't think the current grammar handles at all, other than catching it as two : separators in a row.

@cblackuk
Copy link

It is a shame that it does not understand the dot "." but at least now with my (your) custom changes I can read the code again so I am happy :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants