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] configuration and node keywords aren't highlighted #3342

Closed
pcgeek86 opened this issue Feb 23, 2016 · 22 comments
Closed

[powershell] configuration and node keywords aren't highlighted #3342

pcgeek86 opened this issue Feb 23, 2016 · 22 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug languages-basic Basic language support issues verified Verification succeeded
Milestone

Comments

@pcgeek86
Copy link

On Mac OS X, the configuration keyword isn't being syntax highlighted for the PowerShell language. Is this an easy fix?

screen shot 2016-02-23 at 8 43 24 am

Same goes for node keyword.

128ee370-da21-11e5-816a-329e47b35084

Cheers,
Trevor Sullivan
Microsoft MVP: PowerShell

@joaomoreno joaomoreno added the feature-request Request for new features or functionality label Feb 23, 2016
@joaomoreno
Copy link
Member

This would be a great contribution! Would you like to give it a try and submit a PR?

@joaomoreno joaomoreno added the help wanted Issues identified as good community contribution opportunities label Feb 23, 2016
@pcgeek86
Copy link
Author

@joaomoreno I don't think I'll be very efficient at fixing it, since I'm not on the dev team. It would take me quite a while to research the specifics about implementing syntax highlighting for VS Code, and then track down the problem.

My guess is that the problem lies somewhere here: https://github.com/Microsoft/vscode/blob/master/extensions/powershell/syntaxes/PowershellSyntax.tmLanguage#L525

@egamma
Copy link
Member

egamma commented Feb 23, 2016

FYI @daviwil

@joaomoreno
Copy link
Member

Interesting. From your code snippet, it appears that configuration is indeed captured but the theme somehow is not colouring it. Is there any editor theme that colors it? You can toggle between them using the F1, Change Theme action.

ping @aeschli

@pcgeek86
Copy link
Author

I talked to Keith Hill on Skype this morning, and he pointed out that this bug is resolved in an alpha build, that I don't currently have access to. I'll close this ticket unless things change after the February release.

Cheers,
Trevor Sullivan

@pcgeek86
Copy link
Author

@joaomoreno Actually, you were right. I just tested out changing the theme, and sure enough, it is colored. I never changed the them from the default (didn't even know this was available), so maybe the theme needs to be updated? Re-opening issue.

Despite the naming, my "default" theme was actually Dark Visual Studio, and not the Dark+ (default dark) theme.

screen shot 2016-02-23 at 11 23 47 am

screen shot 2016-02-23 at 11 23 55 am

@pcgeek86 pcgeek86 reopened this Feb 23, 2016
@joaomoreno
Copy link
Member

Awesome. @aeschli, what can be done about this?

@joaomoreno joaomoreno added bug Issue identified by VS Code Team member as probable bug and removed feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities labels Feb 23, 2016
@joaomoreno joaomoreno changed the title PowerShell "configuration" keyword isn't colored / highlighted PowerShell configuration and node keywords aren't highlighted Feb 24, 2016
@aeschli
Copy link
Contributor

aeschli commented Feb 24, 2016

configuration seems to use the storage.type scope, which is a bit of a complicated case.
Some grammars use that for type references, other for keywords like 'var'.
Our default themes color it with the 'type' color which is turquise in the dark+/light+ and regular background color in the dark VS/light VS themes.

I have to read more on what's the expected use of storage.type and might change it in te future to a keyword color. But if you want to move foreard, better use a scope like `keyword'.

@egamma
Copy link
Member

egamma commented Feb 24, 2016

Actually, this was intentional, we did not want to change the selected theme behind the user's back. Consider users that like the less colorful theme, they would not appreciate if we upgrade them automatically.

However, given this issue I wonder whether we should provide some more active guidance to users to upgrade to the '+' theme (they obviously do not read the release notes 😞).

One idea would be a 'once message' (shown only once) to a user that uses the Dark, Light theme that there now is a Dark+/Light+. The message could provide a command to change the theme to the corresponding '+' one.

@egamma egamma added this to the Feb 2016 milestone Feb 24, 2016
@pcgeek86
Copy link
Author

@aeschli Thanks for the analysis. I'm not sure why the storage.type was used, because I don't understand custom grammar authorship for Visual Studio Code at this time. I could go ahead and update it to keyword, but unfortunately I don't understand the potentially negative implications of doing so. Any thoughts on what that could affect?

@egamma Good idea to mention this in the release notes, somehow. In fact, I didn't even realize that release notes were available. Normally I love reading them. Perhaps we could do something to encourage reading release notes, like showing them in the editor after an upgrade or new installation of VS Code?

@joaomoreno joaomoreno modified the milestones: Backlog, Feb 2016 Feb 24, 2016
@egamma
Copy link
Member

egamma commented Feb 24, 2016

@pcgeek86 the release notes are here https://code.visualstudio.com/Updates 😄 and whenever you get an auto update there is message box with a button to open the release notes.

image

The fact that you never noticed them makes me thinking...

@bgse
Copy link
Contributor

bgse commented Feb 26, 2016

@egamma This might be a psychology thing. I guess when the update notification comes up, users can get a bit excited about new toys, and the obvious thing to do is to hit that update button asap.

Maybe it would be a good idea to display the release notes notification again once the user starts the updated VSCode for the first time.

E.g. "Visual Studio Code was recently updated. Do you want to read release notes now", offering options like "Release Notes", "Not now", "Do not ask again".

@pcgeek86
Copy link
Author

@bgse @egamma omg yes, you're spot on with that analysis @bgse. I literally do that, especially because of how fast the VS Code update process is!!! 😄

I still like my idea of displaying the release notes after an upgrade, or alternatively, showing just the release notes button / notification as @bgse suggested. 😄 😄 Maybe have an option to disable it in the VS Code config?

@egamma
Copy link
Member

egamma commented Feb 26, 2016

@bgse and @pcgeek86 thanks for your insights, we spend some real time on the release notes, it would be a pity if they are not getting read.

// @joaomoreno

@joaomoreno
Copy link
Member

#3525

@pcgeek86
Copy link
Author

@joaomoreno Thanks for filing that.

@aeschli
Copy link
Contributor

aeschli commented Mar 4, 2016

@daviwil David, ok if we move this issue to https://github.com/PowerShell/vscode-powershell ? Once fixed we're happy to update our powershell text mate grammar.

@aeschli aeschli changed the title PowerShell configuration and node keywords aren't highlighted [powershell] configuration and node keywords aren't highlighted Mar 4, 2016
@aeschli aeschli added the languages-basic Basic language support issues label Mar 4, 2016
@daviwil
Copy link
Contributor

daviwil commented Mar 4, 2016

We're actually about to start maintaining the official PowerShell TextMate grammar in its own repo so you'll be able to move all PowerShell grammar bugs there. I'll see about getting that repo created today and I'll create a bug there to track this issue.

@daviwil
Copy link
Contributor

daviwil commented Jul 25, 2016

This was fixed in the VS Code 1.3, let us know if you see any issues with these keywords in the future. Thanks!

@daviwil daviwil closed this as completed Jul 25, 2016
@aeschli
Copy link
Contributor

aeschli commented Sep 2, 2016

image
'node' is not a separate token, and therefor doesn't get a different color. @daviwil is this a bug?

@aeschli aeschli added the verified Verification succeeded label Sep 2, 2016
@pcgeek86
Copy link
Author

pcgeek86 commented Sep 2, 2016

@aeschli Yes, it would be a bug if it still exists, but @daviwil indicated it was fixed. Unfortunately it seems it's still a problem.

Here's what I see in VSCode:

image

Here's what I see in PowerShell ISE:

image

@daviwil
Copy link
Contributor

daviwil commented Sep 2, 2016

It was fixed in our new syntax definition in VS Code 1.3 but we had to pull it back out in 1.4 due to other bugs that were severely impacting user experience. We're going to start fixing up the new syntax definition over at https://github.com/PowerShell/EditorSyntax and hopefully bring it back within the next couple of VS Code releases.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug languages-basic Basic language support issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants