8000 Add "string" to cppRawDelimiter group name by rburny · Pull Request #32 · vim-jp/vim-cpp · GitHub
[go: up one dir, main page]

Skip to content

Add "string" to cppRawDelimiter group name #32

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

Merged
merged 1 commit into from
Apr 22, 2015
Merged

Add "string" to cppRawDelimiter group name #32

merged 1 commit into from
Apr 22, 2015

Conversation

rburny
Copy link
Contributor
@rburny rburny commented Apr 21, 2015

This change fixes handling of raw strings by matchparen - a standard plugin that highlights matching parentheses. The bug that I'm fixing manifests in the following code:

foo(bar(
  R"( )"
) );

If you move over parens in this code in Vim, you'll see that wrong ones are highlighted. This is because matchparen uses syntax highlighting to find (and skip) string literals. To be precise, it assumes that all string literals are highlighted with a syntax group whose name contains "string". Because we used "cppRawDelimiter", which does not contain "string", parens in raw string delimiters were not handled properly.

This change fixes handling of raw strings in matchparen - a standard plugin that highlights matching parentheses.
The bug that I'm fixing manifests in the following code:

foo(bar(
  R"( )"
) );

If you move over parens in this code in Vim, you'll see that wrong ones are highlighted. This is because matchparen uses syntax highlighting to find (and skip) string literals. To be precise, it assumes that all string literals are highlighted with a syntax group whose name contains "string". Because we used "cppRawDelimiter", which does not contain "string" in it, parens in raw string delimiters confused the matchparen algorithm.
@mattn
Copy link
Member
mattn commented Apr 21, 2015

Sorry, I don't still understand what you fix.

https://github.com/rburny/vim-cpp/commit/0911af4c4c2ba8b54fe1a90a36dd0a3f1341da83?diff=split

AFAICS, you change only spelling of s/cppRawDelimiter/cppRawStringDelimiter/g.

@rburny
Copy link
Contributor Author
rburny commented Apr 21, 2015

Yes, that's right - I'm only changing the name. However, matchparen parses this name, so changing it can change its rehavior! It is very tricky, though - so let me explain.

First of all, matchparen is the standard plugin that highlights matching braces. E.g. if you write foo(bar), then move cursor over opening paren, the closing paren is highlighted. This sounds easy, but it becomes more tricky when you add string literals or comments. Consider the following invocation:

foo(" ) ");

How does matchparen algorithm know it should highlight the closing paren, not the one inside string? It does not have any language-specific rules - instead, it relies on syntax highlighting. The exact rule, described in help matchparen, is:

The syntax highlighting attributes are used.  When the cursor currently is not
in a string or comment syntax item, then matches inside string and comment
syntax items are ignored.  Any syntax items with "string" or "comment"
somewhere in their name are considered string or comment items.

So this heuristics only works if each syntax highlighting group is named properly. Unfortunately, "cppRawDelimiter" doesn't have the required "string" in its name, causing bad interactions with matchparen. The new name contains "string", and matchparen works well.

@mattn
Copy link
Member
mattn commented Apr 22, 2015

Ah, you are right. I didn't know it.

mattn added a commit that referenced this pull request Apr 22, 2015
Add "string" to cppRawDelimiter group name
@mattn mattn merged commit 98eb6f6 into vim-jp:master Apr 22, 2015
@mattn
Copy link
Member
mattn commented Apr 22, 2015

Thank you

@MarkLodato
Copy link

Thank you! This has been bugging me for a long time!

@rburny rburny deleted the patch-1 branch May 8, 2015 19:51
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
0