Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect syntax highlighting for rf strings #186

Open
janosh opened this issue Jul 15, 2019 · 13 comments
Open

Incorrect syntax highlighting for rf strings #186

janosh opened this issue Jul 15, 2019 · 13 comments

Comments

@janosh
Copy link

@janosh janosh commented Jul 15, 2019

Environment data

  • VS Code version: 1.36.0
  • MagicPython version: 1.1.1
  • OS: macOS 10.14.5
  • Color scheme: Dark+ (default dark)
  • Python version: 3.6.7 (miniconda)

Expected behavior

If I combine raw strings with formatted string literals (a.k.a. f-strings), I'd expect the syntax highlighting to remain that of f-strings.

f-string

Actual behavior

What I'm actually seeing is the dark red color used for raw strings even for parts of the f-string that are Python code and have yet to be evaluated.

rf-string

Related issues

#6572: This issue was originally opened against the VS Code Python extension.

@lmcarreiro
Copy link

@lmcarreiro lmcarreiro commented Oct 23, 2019

Another related problem: microsoft/vscode-python#2445

image

Update: the lower case r is not wrong, it is a feature to use for regex: #114 (comment)

@vpetrovykh
Copy link
Member

@vpetrovykh vpetrovykh commented Oct 23, 2019

To reiterate: there's a feature highlighting lower-case r strings as regexp, while upper-case R as simply raw. Using upper-case R should get you the result you want. Compare:

plt.title(
    (f"{label}: " if label else "") + r"$y_\mathrm{true}$ vs $y_\mathrm{pred}$"
)

plt.title(
    (f"{label}: " if label else "") + R"$y_\mathrm{true}$ vs $y_\mathrm{pred}$"
)

plt.title(
    (Rf"{label}: " if label else "") + R"$y_\mathrm{true}$ vs $y_\mathrm{pred}$"
)

The lower-case r was chosen for the special treatment largely due to a precedent that some existing highlighters offered this convenience already.

@vors

This comment was marked as off-topic.

@kelvinwop
Copy link

@kelvinwop kelvinwop commented Jan 23, 2020

image

capital R makes it do the fstring highlighting

@janosh
Copy link
Author

@janosh janosh commented Jan 23, 2020

@vpetrovykh Thanks for this advice. Unfortunately, it's not currently usable with black formatting since black automatically lower cases r-string prefixes. I opened black#1244 to discuss this.

@elprans
Copy link
Member

@elprans elprans commented Jan 23, 2020

@janosh Use --skip-string-normalization for black to disable the lowercasing behavior.

@janosh
Copy link
Author

@janosh janosh commented Jan 23, 2020

@elprans IIUC, this has other effects besides preventing r-string prefixes from being lower cased (such as not converting single to double quotes).

Btw, just tested it and it works with auto-format on save by adding skip-string-normalization = true under [tool.black] to pyproject.toml.

@gazzar
Copy link

@gazzar gazzar commented Feb 2, 2020

I think this decision by the MagicPython devs to apply additional special meaning to a part of the Python language definition is a mistake. I've been wondering for a while why all my raw strings (which are almost never regex's) are highlighted weirdly by VSCode. It's nice to finally understand what's going on but it imposes mysterious behaviour on downstream projects. The simple question is, can this behaviour be disabled by downstream projects by some configuration option?

@MichaelBrown08
Copy link

@MichaelBrown08 MichaelBrown08 commented Feb 3, 2020

I have struggled with this issue constantly. Sometimes disable -> enable works and gets the f-strings highlighting back, but when using WSL this is no longer possible. With MagicPython installed I still don't have f-string highlighting:

image

How do I consistently, without needing to install and reinstall, get the f-string highlight?

@1st1
Copy link
Member

@1st1 1st1 commented Feb 11, 2020

I think this decision by the MagicPython devs to apply additional special meaning to a part of the Python language definition is a mistake.

FWIW I agree, that probably was my mistake. The problem is that reverting this isn't an option now -- as magicpython is used by GH to highlight Python code and by multiple IDEs. Millions of people use it daily. To be fair, millions of people used TextMate & SublimeText before that also highlighted r'strings' as regexps.

@vpetrovykh @elprans That said I think that fr'strings' shouldn't be highlighted as regexps, so perhaps we should revert that change.

I have struggled with this issue constantly. Sometimes disable -> enable works and gets the f-strings highlighting back, but when using WSL this is no longer possible. With MagicPython installed I still don't have f-string highlighting:

That seems like a not relevant issue. Perhaps your editor has some plugin that interferes with MagicPython.

@vpetrovykh
Copy link
Member

@vpetrovykh vpetrovykh commented Feb 12, 2020

Yes, looks like f-strings should be format strings first and foremost, regardless of what other flavor they have.

@elprans
Copy link
Member

@elprans elprans commented Feb 12, 2020

+1 for handling fr'foo' as f-strings. As for the r-strings, we can probably ship an alternative grammar file and have downstream make their choice.

@gazzar
Copy link

@gazzar gazzar commented Feb 12, 2020

I think this decision by the MagicPython devs to apply additional special meaning to a part of the Python language definition is a mistake.

FWIW I agree, that probably was my mistake. The problem is that reverting this isn't an option now -- as magicpython is used by GH to highlight Python code and by multiple IDEs. Millions of people use it daily. To be fair, millions of people used TextMate & SublimeText before that also highlighted r'strings' as regexps.

I think that, at least in the case of VSCODE, they might be happy if this behaviour was reverted. I'm not sure about GH or the other IDE devs' opinions; you would know better than me. Would it be possible to overload the parser to provide an alternative API that the IDEs could swap to or expose to their end users to select whether to use the regex-highlighted version or a plain-string version?

vpetrovykh added a commit that referenced this issue Jul 31, 2020
Any f-string allows variable interpolation, that includes a raw
f-string. Make all raw prefixes (`r` or `R`) behave the same way in
combination with `f` prefix.

Fixes #186
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.