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

[clangd] Support square bracket escaping in Annotations #69379

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

Conversation

fefaleksey
Copy link

The Annotations class is not supporting C++ attributes [[...]] because the attributes are parsed as a selection. The proposed improvement allows [, ] characters to be escaped (example: \[\[nodiscard\]\]), allowing attributes to be used.

@fefaleksey
Copy link
Author

@sam-mccall @kadircet Hello! Could you please review this PR?

@sam-mccall
Copy link
Collaborator

The usual workaround for this is to use the digraph <: in place of [. Does this work for your case?

I'd prefer not to add a second escaping mechanism if the existing one does work, maybe we should document it though.

@fefaleksey fefaleksey closed this Oct 18, 2023
@fefaleksey fefaleksey reopened this Oct 18, 2023
@fefaleksey
Copy link
Author

The usual workaround for this is to use the digraph <: in place of [. Does this work for your case?

I'd prefer not to add a second escaping mechanism if the existing one does work, maybe we should document it though.

Yes, this works well, but in this case we will not be able to test the [[ ... ]] attributes. This may be important, for example, for testing a tweak for moving functions (with its attributes). What do you think about it?

@fefaleksey
Copy link
Author

@sam-mccall friendly reminder)

@sam-mccall
Copy link
Collaborator

Yes, this works well, but in this case we will not be able to test the [[ ... ]] attributes.

Can you explain a bit further?
These can be spelled [<: ... :>] or <:<: ... :>:> in the code, which should have the same effect.

Code that moves attributes around would ideally not be too sensitive to the char-by-char spelling, as these are often hidden behind macros etc.

Is the concern that the code being tested might work with digraphs but not with literal [ characters?
(It is unfortunate if a test isn't realistic, but this doesn't seem terribly likely)

@fefaleksey
Copy link
Author

Yes, this works well, but in this case we will not be able to test the [[ ... ]] attributes.

Can you explain a bit further? These can be spelled [<: ... :>] or <:<: ... :>:> in the code, which should have the same effect.

Code that moves attributes around would ideally not be too sensitive to the char-by-char spelling, as these are often hidden behind macros etc.

Is the concern that the code being tested might work with digraphs but not with literal [ characters? (It is unfortunate if a test isn't realistic, but this doesn't seem terribly likely)

Actually yes, I had concern that the code being tested might work with digraphs but not with literal [ characters, because <: has 2 symbols, but [ is only one symbol. But maybe you are right and the probability is really low.
If you think that this is not necessary, then I think we can close the PR.

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.

None yet

2 participants