improve vi syntax highlighting

[Andriamanitra]

• make sure strings end at newline -- the syntax still sometimes thinks comments are strings so I'm not sure if this should close Comments in .vimrc aren't highlighted properly #1345, but at least with these changes the breakage does not spill over to following lines
• highlight registers (to prevent @" / @' starting new strings)
• support more keywords

Known issue: Always ending strings at the end of line means we no longer properly highlight strings spanning multiple lines using line continuations. I don't think our syntax highlighting system is sophisticated enough to implement line continuations properly: without the backslashes you'd get syntax errors so it wouldn't make sense to highlight it as string. Doing multiline strings with line continuations seems to be quite rare in practice so I'm comfortable making this tradeoff to improve highlighting in the more common cases (the other parts are real examples from a popular vimrc repository).

Test file

let lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.
    \ Integer mollis quam vitae justo semper ultricies. Curabitur et libero
    \ at odio faucibus rutrum et at nunc.'

set ai "Auto indent
set si "Smart indent

vmap <M-j> :m'>+<cr>`<my`>mzgv`yo`z
vmap <M-k> :m'<-2<cr>`>my`<mzgv`yo`z

let l:saved_reg = @"
execute "normal! vgvy"

before:

after:

[JoeKar]

But isn't that heresy to edit Vi(m) files with micro? 😆

[Andriamanitra]

But isn't that heresy to edit Vi(m) files with micro? 😆

I don't think so. What other editor could you possibly use to edit them? 🤪

[dmaluka]

the syntax still sometimes thinks comments are strings

Could you share an example file to reproduce this?

[Andriamanitra]

the syntax still sometimes thinks comments are strings

Could you share an example file to reproduce this?

The one in the PR description is the simplest one -- "Auto indent is a comment but it's highlighted as a string.

Here's a small file that demonstrates three four different meanings of ":

" these are all supposed to be highlighted as comments, we only get the first one correct
set ai " comment
set ai "comment
set ai "comment"

" strings
let s = "string"
exec "!micro -version"

" in this context the " is neither a string nor comment but a keypress
nnoremap <A-y> "ayy
nnoremap <A-p> "ap

" and @" is a register
let l:saved_reg = @"

[dmaluka]

Ah, ok. I thought "Auto indent was meant to be a string (because with your PR it is, well, still highlighted as a string, not as a comment).

So, I'm not sure this PR actually makes things better. Comments are still incorrectly highlighted as strings, and worse, incomplete (unclosed) strings are incorrectly and confusingly highlighted as complete strings, as if vimscript syntax allowed them. (Your PR genuinely made me think that in vimscript "Auto indent is a string, not a comment, unless it is at the beginning of a line, and that in your example this string is assigned to ai via set ai. You see, I don't write vim scripts every day, or ever at all.)

[Andriamanitra]

So, I'm not sure this PR actually makes things better. Comments are still incorrectly highlighted as strings

That's fair. The other option would be to remove string highlighting and only highlight comments when we are sure they are comments (when " is the first non-whitespace character on a line). In its current state the vimscript highlighting is worse than having no highlighting at all. Maybe we could keep a simple '"[^"]*"' for strings (with the tradeoff that escaped closing quotes then don't get skipped).

and worse, incomplete (unclosed) strings are incorrectly and confusingly highlighted as complete strings, as if vimscript syntax allowed them.

It is definitely not worse than highlighting them as multiline strings as vimscript does not allow those either. Expressions can't span multiple lines without the explicit line continuation syntax. At least this way the next line is still highlighted correctly even when we fail to handle the previous line correctly (which happens at least a couple times in most real vimrc files I've looked at).

Here's a patch to remove string regions if that's preferable:

diff --git a/runtime/syntax/vi.yaml b/runtime/syntax/vi.yaml
index b45f7027..3c2df549 100644
--- a/runtime/syntax/vi.yaml
+++ b/runtime/syntax/vi.yaml
@@ -18,20 +18,10 @@ rules:
         rules: []
 
     - comment:
-        start: "(^\"|[ \t]+\" |[ \t]+\"$)"
+        start: "^[ \t]*\""
         end: "$"
         rules: []
 
-    - constant.string:
-        start: "\""
-        end: "\"|$"
-        skip: "\\\\."
-        rules:
-            - constant.specialChar: "\\\\."
-
-    - constant.string:
-        start: "'"
-        end: "'|$"
-        skip: "\\\\."
-        rules:
-            - constant.specialChar: "\\\\."
+    - constant.string: '"[^"]*"'
+    - constant.string: "'[^']*'"
+    - constant.char: "\\\\."

[JoeKar]

This ambiguity of " and ' is weird.
I agree to reduce the comment location to the start of the line (including indentation) only, because this seems to be the common approach and to handle the strings without a region.

[dmaluka]

Another option is to do nothing. Any "improvement" here would have pretty bad side effects, status quo is pretty bad too, but this is not a mission-critical functionality after all, it's just stupid syntax highlighting, it's just colors.

[dmaluka]

Anyway, if we consider the last suggestion the least of the evils, I'm probably ok with it.

+    - constant.char: "\\\\."

You probably meant constant.specialChar?