[vim/vim] Incorrect syntax highlighting of Python f-strings (Issue #10734)
Adam J. Stewart
Steps to reproduce
Vim syntax highlighting is incorrect for Python f-strings (introduced in Python 3.6) and combinations of r-strings and u-strings. For example, the following file:
# Correct a = '\nfoo\n' b = r'\nfoo\n' c = u'foo' d = ur'\nfoo\n' # Incorrect e = f'{c} bar' f = fr'{c}\n' g = rf'{c}\n' h = ru'\nfoo\n' i = uf'{c}'
renders as:
Expected behaviour
I would expect the string marker (f, r, u) to always be the same color, even if it is used in combination with another marker.
Version of Vim
8.2
Environment
OS: macOS 12.4
Terminal: Terminal.app
$TERM: xterm-256color
Shell: bash 5.1.8
Logs and stack traces
No response
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
lacygoill
On my Ubuntu 20.04 machine, which comes with Python 3.8.10, the d, h, and i assignments are not syntactically correct:
$ python
>>> d = ur'\nfoo\n'
File "<stdin>", line 1
d = ur'\nfoo\n'
^
SyntaxError: invalid syntax
>>> h = ru'\nfoo\n'
File "<stdin>", line 1
h = ru'\nfoo\n'
^
SyntaxError: invalid syntax
>>> i = uf'{c}'
File "<stdin>", line 1
i = uf'{c}'
^
SyntaxError: invalid syntax
Not sure it's the correct fix (I don't know Python), but you could try this patch:
diff --git a/runtime/syntax/python.vim b/runtime/syntax/python.vim index 3af54cc4c..c2d5737f3 100644 --- a/runtime/syntax/python.vim +++ b/runtime/syntax/python.vim @@ -123,13 +123,13 @@ syn keyword pythonTodo FIXME NOTE NOTES TODO XXX contained " Triple-quoted strings can contain doctests. syn region pythonString matchgroup=pythonQuotes - \ start=+[uU]\=\z(['"]\)+ end="\z1" skip="\\\\\|\\\z1" + \ start=+[fFuU]\{,2}\z(['"]\)+ end="\z1" skip="\\\\\|\\\z1" \ contains=pythonEscape,@Spell syn region pythonString matchgroup=pythonTripleQuotes \ start=+[uU]\=\z('''\|"""\)+ end="\z1" keepend \ contains=pythonEscape,pythonSpaceError,pythonDoctest,@Spell syn region pythonRawString matchgroup=pythonQuotes - \ start=+[uU]\=[rR]\z(['"]\)+ end="\z1" skip="\\\\\|\\\z1" + \ start=+\%([fFuU]\{,2}[rR]\|[rR][fFuU]\{,2}\)\z(['"]\)+ end="\z1" skip="\\\\\|\\\z1" \ contains=@Spell syn region pythonRawString matchgroup=pythonTripleQuotes \ start=+[uU]\=[rR]\z('''\|"""\)+ end="\z1" keepend
To get it merged, you could try to contact the maintainer of the Python syntax plugin, whose email address is given at the top of file:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
lacygoill
Not sure it's the correct fix (I don't know Python), but you could try this patch:
It doesn't take into account the possibility that the r flag (or whatever it's called) is in the middle. New try:
diff --git a/runtime/syntax/python.vim b/runtime/syntax/python.vim
index 3af54cc4c..eff587c6b 100644
--- a/runtime/syntax/python.vim +++ b/runtime/syntax/python.vim @@ -123,13 +123,13 @@ syn keyword pythonTodo FIXME NOTE NOTES TODO XXX contained " Triple-quoted strings can contain doctests. syn region pythonString matchgroup=pythonQuotes - \ start=+[uU]\=\z(['"]\)+ end="\z1" skip="\\\\\|\\\z1" + \ start=+[fFuU]\{,2}\z(['"]\)+ end="\z1" skip="\\\\\|\\\z1" \ contains=pythonEscape,@Spell syn region pythonString matchgroup=pythonTripleQuotes \ start=+[uU]\=\z('''\|"""\)+ end="\z1" keepend \ contains=pythonEscape,pythonSpaceError,pythonDoctest,@Spell syn region pythonRawString matchgroup=pythonQuotes - \ start=+[uU]\=[rR]\z(['"]\)+ end="\z1" skip="\\\\\|\\\z1"
+ \ start=+[fFuU]\{,2}[rR][fFuU]\{,2}\z(['"]\)+ end="\z1" skip="\\\\\|\\\z1"
\ contains=@Spell
syn region pythonRawString matchgroup=pythonTripleQuotes
\ start=+[uU]\=[rR]\z('''\|"""\)+ end="\z1" keepend—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Adam J. Stewart
Interesting, it's weird that some combinations work and others don't.
Your latest regex changes look perfect.
Any idea if Zvezdan Petkovic has a GitHub?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
lacygoill
cc @zvezdan
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Adam J. Stewart
Interesting, it's weird that some combinations work and others don't.
Found the official BNF grammar definition: https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
lacygoill
Found the official BNF grammar definition: https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals
IIUC the spec, this regex should match a string prefix:
[rRuUfF]\|[fF][rR]\|[rR][fF]\|[bB][rR]\=\|[rR][bB]
Since the syntax plugin needs to make a distinction between the raw strings and the other types of strings, we need one regex for a string prefix in front of a raw string:
[rR]\|[fF][rR]\|[rR][fF]\|[bB][rR]\|[rR][bB]
And one for other types of strings:
[uUfF]\|[bB]
Which gives this patch:
diff --git a/runtime/syntax/python.vim b/runtime/syntax/python.vim index 3af54cc4c..43e46e5d2 100644 --- a/runtime/syntax/python.vim +++ b/runtime/syntax/python.vim @@ -123,16 +123,16 @@ syn keyword pythonTodo FIXME NOTE NOTES TODO XXX contained " Triple-quoted strings can contain doctests. syn region pythonString matchgroup=pythonQuotes
- \ start=+[uU]\=\z(['"]\)+ end="\z1" skip="\\\\\|\\\z1"
+ \ start=+\%([uUfF]\|[bB]\)\=\z(['"]\)+ end="\z1" skip="\\\\\|\\\z1"
\ contains=pythonEscape,@Spell
syn region pythonString matchgroup=pythonTripleQuotes
- \ start=+[uU]\=\z('''\|"""\)+ end="\z1" keepend + \ start=+\%([uUfF]\|[bB]\)\=\z('''\|"""\)+ end="\z1" keepend
\ contains=pythonEscape,pythonSpaceError,pythonDoctest,@Spell
syn region pythonRawString matchgroup=pythonQuotes
- \ start=+[uU]\=[rR]\z(['"]\)+ end="\z1" skip="\\\\\|\\\z1"
+ \ start=+\%([rR]\|[fF][rR]\|[rR][fF]\|[bB][rR]\|[rR][bB]\)\=\z(['"]\)+ end="\z1" skip="\\\\\|\\\z1"
\ contains=@Spell
syn region pythonRawString matchgroup=pythonTripleQuotes
- \ start=+[uU]\=[rR]\z('''\|"""\)+ end="\z1" keepend + \ start=+\%([rR]\|[fF][rR]\|[rR][fF]\|[bB][rR]\|[rR][bB]\)\=\z('''\|"""\)+ end="\z1" keepend \ contains=pythonSpaceError,pythonDoctest,@Spell syn match pythonEscape +\\[abfnrtv'"\\]+ contained
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
@zvezdan - what do you think of the suggested patch?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Zvezdan Petkovic
@brammool, @lacygoill, and @adamjstewart
I'll review the regex carefully this weekend. Assuming everything is right if would add the highlighting to f or F part as well as the rest of the string. However, that's not much benefit compared to other proposals ...
I've received email questions/requests about showing f-strings in a richer format with the {name} parts highlighted differently. So, I was looking into using a different set of entries for such regions that would allow for contained code parts to be highlighted, similar to pythonDoctest.
If there's no big hurry, I can estimate the effort for that implementation and perhaps we could pragmatically start with this change and proceed with more complex one later, or I can offer the complete change sooner if the effort is not too large. I'm aware of f-strings (and some other newer features) missing, but just didn't get around implementing them yet.
Opinions?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Ben Lewis
Seeing as it's been 18 months, I'd love to see the pragmatic solution deployed asap because the current syntax highlighting is completely broken (if the code uses a few fstrings, you can end up with large swathes of code treated as strings, reducing readability perhaps worse than no highlighting).
Of course the richer f-string highlighting would be nice too, but the syntax is quite complicated (e.g. strings can be nested inside the code segments in f-strings), and the impact of lacking that feature (rendering the entire f-string as a string) would be more contained.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Zvezdan Petkovic
@benjimin I have several changes in the pipeline, but was stuck on PR #8033 not being merged for over a year. When that PR was finally closed, I updated my clone of Vim source code, and my changes were still not merged. So, I waited a little longer before asking again for the merge. Somehow, I missed when it was actually done and didn't check again in quite a while because of Bram's passing. :-(
I'll collect all pending changes as I find some free time over the month of December and submit PRs for them. I agree that even a simple change to treat f-strings correctly as strings would be better than any more complex handling. I'll separate the changes in that order. A simple fix for correctness first followed by richer handling later if possible.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
K.Takata
but was stuck on PR #8033 not being merged for over a year.
Sorry, but could you create a new PR for this? (Or, reopen it?)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Zvezdan Petkovic
Sorry, but could you create a new PR for this? (Or, reopen it?)
@k-takata It was eventually merged. I did say above ...
Somehow, I missed when it was actually done ...
So, there's no need for another PR, I think. :-)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
Unless I am missing something, the f-string syntax for python strings is not in current master:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Zvezdan Petkovic
Unless I am missing something, the f-string syntax for python strings is not in current master:
The conversation was about PR #8033 which seems merged now.
I was just explaining that it was sitting unmerged for over two years, from March 2021 until April 2023. When the PR was closed it was still unmerged -- I did update my clone of the code at the time and confirmed that it was still missing. Turns out that it was merged later, but I somehow missed that. Hence, I did not send any other changes waiting in my local checkout.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
yeah that is fine. I'll try to be a bit quicker with merging runtime file updates, but I cannot make guarantees, especially not if there is a lot of discussions on how to move forward.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Mikhail f. Shiryaev
Is there any progress on the issue? It's very painful to have the f-strings without {} highlighted
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
dkearns
You can try #14057.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Mikhail f. Shiryaev
Thank you, I downloaded it as following, and it fixed the f-strings:
curl -s https://raw.githubusercontent.com/vim/vim/451559280c96bba3f30e7c6ab380395b7d9ed647/runtime/syntax/python.vim | sed 's/b:current_syntax/b:fixed_syntax/' > ~/.vim/after/syntax/python.vim
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Zvezdan Petkovic
I previously stated what I feel should be changed / improved in the comment on another issue.
Several people sent patches and I had a few candidates in my local changes for the syntax file.
I'll try to send a sequence of incremental improvements to the syntax file to bring it up-to-date.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
Closed #10734 as completed via b7fc24d.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Mikhail f. Shiryaev
Felixoid left a comment (vim/vim#10734)I am pretty sure it shouldn't be closed ¯\_(ツ)_/¯
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#10734)still not? I thought commit b7fc24d would fix this issue finally?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Mikhail f. Shiryaev
Felixoid left a comment (vim/vim#10734)Checked it as follows, nothing like on the screenshot above:
mkdir -p "$HOME/.vim/after/syntax"
curl -s https://raw.githubusercontent.com/vim/vim/b7fc24d3a3d21ccf1461c703eb7ff07ef3994c54/runtime/syntax/python.vim \
| sed 's/b:current_syntax/b:fixed_syntax/' > "$HOME/.vim/after/syntax/python.vim"
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Rob B
f4nb0y left a comment (vim/vim#10734)The next PR handles f-string replacement fields.
Working on it now :-)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Rob B
f4nb0y left a comment (vim/vim#10734)Opened draft PR #17784
Please confirm that it matches fields and delimiters in all cases, then we can discuss highlighting inside fields.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Rob B
f4nb0y left a comment (vim/vim#10734)Hi Folks,
#17962 is the final PR for this issue. Please confirm that it highlights all of your examples inside f-strings, and does not regress highlighting anywhere else.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.