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

switch statement default clause condition keyword should be flagged as keyword token #10470

Closed
msftrncs opened this issue Aug 31, 2019 · 2 comments · Fixed by #10487
Closed
Labels
Committee-Reviewed PS-Committee has reviewed this and made a decision Issue-Enhancement the issue is more of a feature request than a bug Resolution-Fixed The issue is fixed. WG-Language parser, language semantics

Comments

@msftrncs
Copy link
Contributor

Summary of the new feature/enhancement

In PowerShell ISE and with PSReadLine, the default clause condition keyword of the switch statement receives only 'bareword' treatment, making it hard to spot relative to other bareword conditions.

(PowerShell 7 with custom PSReadLine theme)
image

(Example shown here with PowerShell/EditorSyntax#156 (which marks the bareword conditions as unquoted strings) and a custom theme in VS Code, with the keyword highlighted)

image

Proposed technical implementation details (optional)

I would propose a change to Parser.GetCommandArgument() that would mark the token as a keyword when it meets the criteria for being the default clause condition keyword.

I can include a commit to PR #10295 that demonstrates this change.

image

@msftrncs msftrncs added the Issue-Enhancement the issue is more of a feature request than a bug label Aug 31, 2019
@iSazonov iSazonov added Review - Committee The PR/Issue needs a review from the PowerShell Committee WG-Engine core PowerShell engine, interpreter, and runtime WG-Language parser, language semantics and removed WG-Engine core PowerShell engine, interpreter, and runtime labels Sep 3, 2019
@joeyaiello
Copy link
Contributor

Conceptually, @PowerShell/powershell-committee agrees that this behavioral part of the change is fine. We defer to the Maintainers and Area Experts to review the technical and implementation aspects of the code. (And we expect that they'll mandate you include some tests in your PR as well :) )

Having looked over #10295, we'd prefer that you add your proposed commit to a separate PR.

But hey, thanks for the change. I always appreciate seeing improvements to syntax highlighting.

@joeyaiello joeyaiello added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Sep 4, 2019
@iSazonov iSazonov added the Resolution-Fixed The issue is fixed. label Jun 10, 2020
@ghost
Copy link

ghost commented Jun 25, 2020

🎉This issue was addressed in #10487, which has now been successfully released as v7.1.0-preview.4.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committee-Reviewed PS-Committee has reviewed this and made a decision Issue-Enhancement the issue is more of a feature request than a bug Resolution-Fixed The issue is fixed. WG-Language parser, language semantics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants