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

Adds rustStringIdentifier region #476

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sanpii
Copy link

@sanpii sanpii commented May 25, 2022

The RFC implicit named arguments for formatting macros is merged since rust 1.59, I think it’s more readable to highlight the identifier.

Before:
2022-05-25-210840_1676x963_scrot
After:
2022-05-25-210819_1676x981_scrot

It’s my first vim syntax contribution, it’s probably perfectible.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @chris-morgan (or someone else) soon.

Please see the contribution instructions for more information.

@chris-morgan
Copy link
Member

I’ve thought about this sort of thing before (prototyped it a few years ago), but overall I don’t like it because it’s too faulty: the use of curly braces don’t mean that it’s a format string. If this were applied only to certain whitelisted macros (e.g. format, print, eprintln, panic, todo, assert’s second argument which is not terribly easy to express in Vim syntax highlighting, and in practice not even worth trying), there would be cause to do it, but more generally I don’t think it’s a good idea.

On the specific highlighting, I’d want at least { and } to be something that defaulted to Special, and very probably the format specifier from the colon onwards; {{ and }} would also need to be Special, as an escape sequence. And I don’t think Identifier is the right group to use at all. Special is probably the least bad choice for a default highlight link.

(Incidentally, I’d say RFC 2795 is a red herring here.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants