PowerShell / EditorSyntax

PowerShell syntax highlighting for editors (VS Code, Atom, SublimeText, TextMate, etc.) and GitHub!
MIT License
133 stars 45 forks source link

Line comments should scope the trailing newline char to ensure color schemes work as intended #123

Closed keith-hall closed 6 years ago

keith-hall commented 6 years ago

Environment

Issue Description

When typing a line comment, Sublime Text shows completions

Screenshots

image

Expected Behavior

Completions should not be offered while typing line comments, to make the behavior consistent with typing line comments in other syntaxes. This is normally achieved by scoping the \n character at the end of the line. (i.e. match $\n?)

Code Samples

#he
#  ^ type `l` here

$hello
TylerLeonhardt commented 6 years ago

EditorSyntax is not responsible for completions - it only is responsible for syntax highlighting.

I have a hunch that what you're seeing is Sublime trying to be "helpful" by offering completions for words in the file you've already used - vscode does something similar in some cases.

That would make this either an issue on Sublime itself (which I think is not really an issue... It's by design) or an issue with https://github.com/SublimeText/PowerShell

I'm not too familiar with that package and what it does - @vors would be able to tell you more.

TylerLeonhardt commented 6 years ago

I'm going to close this since it's not related to Editor Syntax - but we can continue to have a discussion here if needed.

keith-hall commented 6 years ago

Okay, I probably shouldn't have made this issue about completions, I didn't realize that the other editors don't follow the same standard. Let's make it about color schemes instead, in the hope that you'll see it's very much related to this syntax definition and worth fixing. The color scheme I am using targets comment.line and gives it a background color. In all syntaxes except the PowerShell EditorSyntax, the background color continues on the line all the way across the viewport.

PHP, as an example of correct behavior: image

PowerShell EditorSyntax: image

Like I said, it's only a small change to https://github.com/PowerShell/EditorSyntax/blob/146e421358945dbfbd24a9dcf56d759bdb0693db/PowerShellSyntax.tmLanguage#L401, so it'd be great if you could clarify what motivation you have for thinking it's not worth fixing.

TylerLeonhardt commented 6 years ago

Now that's a different story :)

We should probably be consistent with other languages. @keith-hall would you like to take a crack at contributing a fix for this?

msftrncs commented 5 years ago

Sorry, I am late to the party.

I don't think this was implemented correctly.

Consuming the newline character should be reserved for such things which enforce line continuation, (ie, in powershell, the backtick at the end of a line). The reason for this, is such that an item that MUST end at the end of a line unless such a continuation is present, such as PowerShell enumeration declarations (or really any PowerShell statement), can find the end of line marker once the other scope releases.

enum crazy {
    assignment1 # must end at end of line
    assignment2 = 3 #must still end at end of line
    assignment3 = `
        4 # was allowed to continue because of PowerShell's line continuation marker, but now ends here
    assignment5 = 5 -bxor #doesn't end yet, because an operator automatically continues the line
        7 # but now it must end
    assignment6 = #this line is an error because '=' does not continue the line
    assignment7 #while linting errors above, scoping should see this as a new assignment.
}

The above give some examples. PowerShell is far from the only language that permits these kinds of line continuations, but its probably PowerShell's lack of a requirement to use a line terminator that causes the most trouble.

Instead, the newline character should be scoped using a lookahead with capture, which at least works on VS Code (though not on the newline since its not included in the scoping process that I am aware of, you cannot see it in 'Inspect TM Scopes')

snippet of section of 'commentLine'

            "end": "$(?=(\\n)?)",
            "endCaptures":{
                "1":{
                    "name": "comment.line.powershell"
                }
            },
omniomi commented 5 years ago

@msftrncs I don't understand what the issue here is? Is something actually broken or do you just not agree with capturing the newline character.

On an unrelated note is there anything we can do to encourage you to create some pull requests? You frequently comment with solutions and I'm curious if we can do anything to help you take the next step and implement some of them. We vastly expanded the contribution guide to make it easy and every PR gets multiple eyes on it including mine and @TylerLeonhardt 's.