Various highlighting issues

[rwols]

This looks very good, thanks for this! There are a few issues with it though.

https://www.sublimetext.com/docs/3/scope_naming.html

echo $$v[1][2]
#       ^ should be punctuation.section.brackets.begin
#         ^ should be punctuation.section.brackets.end
#          ^ should be punctuation.section.brackets.begin
#            ^ should be punctuation.section.brackets.begin

foobar
# ^^^^ variable.function, not support.function

foo -a -b --a-long-option
#   ^ should be punctuation.definition.parameter
#   ^^ should be variable.parameter
#      ^ should be punctuation.definition.parameter
#      ^^ should be variable.parameter
#         ^^ should be punctuation.definition.parameter
#         ^^^^^^^^^^^^^^^ should be variable.parameter

ls \
#  ^ should be punctuation.separator.continuation
-a

foo\
bar
#^^ still part of foo, this reads "invoke the command foobar"

function show --no-scope-shadowing
    #         ^^^^^^^^^^^^^^^^^^^^ variable.parameter
    ls
end

for i in $PATH; echo $i is in the path; end
#             ^ should be keyword.operator
#                                     ^ should be keyword.operator

echo $FOO
#     ^^^ variable.other.readwrite ?

echo hello > output.txt
#          ^ should be keyword.operator
cat < output.txt
#   ^ shoul be keyword.operator

echo Hello > all_output.txt ^&1
#                           ^ should be keyword.operator ("redirect stderr to")
#                            ^^ ("filedescriptor 1 aka stdout")

cat foo.txt | head
#           ^ keyword.operator
#             ^^^^ excellent!

make fish 2>| less
#         ^^^ pipe stderr to stdin of "less"
emacs &
#     ^ keyword.operator ("do this in the background")

You could consider copying my bash rewrite here and adjusting the function and parameter syntax accordingly.

[rwols]

As for the string.unquoted, I think you should remove that and leave things unscoped. So many tokens in a shell language are "unquoted string" that everything starts to look like a string, and it's hard to see where an actual quoted string begins and ends.

[Phidica]

Thank you for the comments. I've made a few notes, and I will see about implementing some of these changes soon (at the very least, the simple cases of renaming scopes to align with other shell script convention, which admittedly I did not cross-check too closely with). I will bring back some points for further discussion as well

[rwols]

Cool, I'll keep an eye on this repo ;)

[Phidica]

I've broken down your comments as follows — to make referring to individual points easier; perhaps consider a similar approach next time ;) — and offered my take

A. Rename scopes

1. keyword.control.index-expansion -> punctuation.section.brackets: Agree.
2. support.function.user -> variable.function: Agree; consequently allows support.function.builtin -> support.function.
3. keyword.control -> keyword.operator for ;, |, and &: Agree.
4. variable.other -> variable.other.readwrite: Disagree; I don't see the point of this distinction when fish has nothing that would require the contrasting variable.other.constant.
5. constant.character.escape -> punctuation.separator.continuation for escaped newlines within function calls: Disagree; although the escaping of the newline is achieving what can be used as a line continuation, within the shell it is literally escaping the newline in the unquoted string environment (and thus constant.character.escape is appropriate). In other languages, a backslash at the end of the line may be simply interpretive of the concept of escaping the newline, and in that case I believe punctuation.separator.continuation would be appropriate.

B. Recognise new scopes

1. variable.parameter for arguments that begin with one or two dashes: I'm cautious about this, because whether an argument that starts with a dash is interpreted as an option parameter is ultimately decided by the program being called. A case where this might come up is in the use of negative numbers as arguments, which could be incorrectly scoped as parameters. This is probably rare, though, so parameter recognition being correct 99 percent of the time is likely still okay.
2. punctuation.definition.parameter for dashes at the start of arguments: Pending the above.
3. Redirection: This is entirely unimplemented and I do really need to fix that.

C. Remove scopes

1. string.unquoted: Disagree; the scope naming convention suggests use of string.unquoted and I'm not inclined to remove it. Yes, most things are unquoted strings, but that's just how it is in a shell. I suggest excluding unquoted strings from your colour scheme to help distinguish them from quoted strings. I note that your Bash rewrite appears to retain a concept of unquoted strings as well, however it seems to recognise them at a higher level. I find it convenient to keep them low level and locate them as the shell itself would

D. Bugfixes

1. Escaped newline in function name isn't recognised: Unfortunately this cannot be fixed as the function name is identified with a begin match, and these cannot span multiple lines. The only solution I could see is rewriting to use *.sublime-syntax. At present, I will be leaving the syntax as it is for ST2 compatibility (which was a condition I inherited from the original author) Edit: It turns out it can be done just by being smarter with scopes. I've managed to fix this :)

[rwols]

Thanks for continuing the conversation with your commits! I'll enumerate my issues from now...

1. Line continuations.

   although the escaping of the newline is achieving what can be used as a line continuation, within the shell it is literally escaping the newline in the unquoted string environment (and thus constant.character.escape is appropriate). In other languages, a backslash at the end of the line may be simply interpretive of the concept of escaping the newline

   Sounds like in both cases you're eating the newline and continue parsing as if there's no newline. punctuation.separator gets a different color than constant.character.escape in the new color schemes from the dev builds of ST, so it's easier to distinguish a line-continuation from an escaped space character. I'm still for punctuation.separator.continuation, but it's your syntax.

2. variable.other. Okay, good point.

3. variable.parameter. I get your concern and yeah, from the point of view of the fish interpreter anything after a command is a string and is passed to the command to be invoked, but I'd still go for variable.parameter for anything that starts with a dash. You can even go so far as to interpret two dashes without word characters after it (--) as meaning "end of options" and interpreting everything after that as a literal string (so no variable.parameter highlighting for tokens that start with a dash anymore). The math builtin from fish interprets its arguments as a mathematical expression, so in that particular case you should then highlight things like you would in a C expression. This variable.parameter scope feels very familiar when you've used languages that utilized this scope well (C, Javascript, for example), and just looking at a fish script from a distance you'd be able to see what the function calls are and what the switches/options would be.

4. string.unquoted. Most things in TCL are also unquoted strings; they don't get highlighted as an unquoted string because that would be unhelpful. Every user knows their unquoted argument tokens are unquoted strings, I don't think they need a scope for that. Same reasoning applies here. You can use meta.string.unquoted if you want your users to be able to find those symbols with view.find_by_selector. In YAML, the keys of a dictionary are now string.unquoted and I feel that was a mistake. The whole purpose of string.unquoted is debatable, honestly. I cannot think of a single strongly-typed language that has unquoted strings as a data type. Whereas for "stringly"-typed languages (shell languages, TCL, CMake, YAML), it feels pointless to highlight an unquoted string.

5. I see you highlight and and or as keyword.operator.word. Maybe keyword.operator.logical? I don't have a strong preference either way.

6. The &, ; and | now get highlighted as keyword.operator, but they also still receive punctuation.definition.keyword. In the new color schemes from the dev build of ST punctuation scopes get precedence over all other scopes it seems, so I suggest to remove that punctuation.definition.keyword scope.

7. Highlighting of array accesses seems off:

   echo $$var[ 1 ][ 1 ]

8. It'd be nice to highlight .. in an array access as keyword.operator. They receive string.unquoted now.

9. I suggest to scope the command expansion parentheses ( foo ) as punctuation.section.parens.begin and punctuation.section.parens.end.

10. Inside of a brace expansion, use punctuation.separator for a comma (,).

11. For the tokens of a brace expansion { ... } I suggest punctuation.section.braces.begin and punctuation.section.braces.end (and no keyword.control)

12. Highlight the tilde (~) as keyword.operator when tilde-expansion can occur.

13. It looks like you nailed the line-continuation parsing, nice job!

[Phidica]

(1) Line continuations Okay, that's an interesting point that it gets a different highlight in the dev schemes. I'll reconsider making the change once I've seen the new style and/or it becomes the ST default. In the meantime I'll leave it as is

(4) variable.parameter A unique override for math is a good idea. Of the cases where negative numbers could come up, that's probably the majority of them anyway. And despite fish not treating them as such, I could see the parameter scope being visually useful

(5) string.unquoted Hm. Those are fair points. meta.string.unquoted is a reasonable compromise too. Ok, I'll go with it

(6) keyword.operator.word I would probably reserve .logical for the logic operators when they appear specifically as symbols, such as arguments to math or expr

(7a) keyword.operator punctuation.definition.keyword Whoops, sorry that's a mistake, I should have taken that out. Call this bug D2

(8) Array access When writing the array access syntax I concluded that without a *.sublime-syntax and scope stack, I couldn't limit the number of index-expansion bracket pairs to be less than or equal to the number of dollar signs. I felt uneasy about letting $foo[1][2][3][4] highlight as if it were valid syntax, and fell down on the side of only accurately highlighting as far as could be considered legitimate: down to $$foo[...]. Of course, I didn't anticipate it being much of a problem since I've barely used 1D arrays, let alone 2D. Considering that other users may desire more dimensions, I'm considering going to the other option and permitting an arbitrary number of index-expansion brackets and leaving it to the user to check they have a reasonable number

The remainder are all fine suggestions that I have no problems with accepting :)

[Phidica]

At this stage I think the only thing keeping this issue open is the request for the variable.parameter scope. I still intend to do this but it's a little further down the to-do list than some other tasks

[rwols]

Hi again! I checked out the latest master and yes, most issues seem to be fixed. Let's close this large all-encompassing issue ticket and make smaller, more focused issues for the things left to do. Nice work!

Also I highly, highly recommend you switch to a .sublime-syntax, that's the future after all.

[Phidica]

Sounds good to me

Rewriting to a sublime-syntax which properly takes advantage of the features it provides is certainly a future goal, however it's a big job. My current rough intent is to target the syntax for fish 3.0 being in sublime-syntax, and to round out the highlighting for fish 2.x in tmLanguage