visual-studio-code icon indicating copy to clipboard operation
visual-studio-code copied to clipboard

shellscript strings are not highlighted inside a `for ((` loop

Open lilyball opened this issue 3 years ago • 2 comments

If I write a for (( exp1; exp2; exp3; )); do loop in shellscript, any strings in the body of the loop do not get colored.

screenshot demonstrating the lack of color on this type of shellscript for loop

The editor scope inspector says that the color is coming from meta.scope.for-loop.shell string, which appears to be the following rule:

{
    "name": "Edge cases (foreground color resets)",
    "scope": [
        "constant.other.symbol.hashkey.ruby",
        "keyword.operator.dereference.java",
        "keyword.operator.navigation.groovy",
        "meta.scope.for-loop.shell punctuation.definition.string.begin",
        "meta.scope.for-loop.shell punctuation.definition.string.end",
        "meta.scope.for-loop.shell string",
        "storage.modifier.import",
        "punctuation.section.embedded.begin.tsx",
        "punctuation.section.embedded.end.tsx",
        "punctuation.section.embedded.begin.jsx",
        "punctuation.section.embedded.end.jsx",
        "punctuation.separator.list.comma.css",
        "constant.language.empty-list.haskell"
    ],
    "settings": {
        "foreground": "#F8F8F2"
    }
}

I do not understand why this rule even exists. I haven't experimented with the other matching scopes here, but the meta.scope.for-loop.shell rules certainly seem out of place. meta.scope.for-loop.shell doesn't match anything else in the theme so I don't know what it's supposed to be resetting, and strings are not supposed to have the source foreground color.

lilyball avatar Mar 09 '22 21:03 lilyball

I couldn't recall either what the edge case was for, but after looking into it, it looks like without that set then the contents inside the (( )) of the loop are highlighted like this:

image

So one way or the other, something looks incorrect.

dsifford avatar Mar 20 '22 22:03 dsifford

@dsifford What you show is the highlighting that you would also get with just

(( a = 1; a <= 3; a++ ))

(except your parens aren't highlighted because I'm guessing you only disabled meta.scope.for-loop.shell string and not meta.scope.for-loop.shell punctuation.definition.string.begin/end)

In particular, this highlighting is because this construct is a math expression and ends up with the scope string.other.math.shell, which ends up with the base color being highlighted as string[^1]. So removing the meta.scope.for-loop.shell string rule (and punctuation variants) is still correct. I expect it's using a string base for the scope because it's assuming that this is usually going to be a $(( … )) construct even when it's not.

In any case there are certainly issues that should be fixed in the shellscript syntax definition, but with the current state of things it's perfectly correct for the theme to color for (( … )) exactly as it colors (( … )), and since there's no way to distinguish between the (( … )) expressions in for (( … )); do (( … )); done then the theme must treat them the same (and should treat them as a (( … )) expression outside of a for loop).

[^1]: Ideally the ; chars would actually be highlighted as operators in the for loop (but it looks like the for loop syntax doesn't bother giving the repeat expression its own scope so we can't actually color that differently without affecting math expressions in the loop body) but that's the only real quibble I have with this. Also this same scope isn't used for let a = 1 which also seems like a syntax definition bug as that's what (( … )) is sugar for.

lilyball avatar Mar 20 '22 23:03 lilyball