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

C/C++ autocomplete strips internal double underscores #2059

Closed
charlesnicholson opened this issue Mar 18, 2016 · 7 comments
Closed

C/C++ autocomplete strips internal double underscores #2059

charlesnicholson opened this issue Mar 18, 2016 · 7 comments

Comments

@charlesnicholson
Copy link

It looks like YCM is stripping internal double-underscores from C and C++ identifiers.

Here's a screenshot, note that the type looks correct but the suggested completion has the __ removed:
screen shot 2016-03-18 at 2 57 32 pm
.ycm_extra_conf.py for repro here.
cpp file for repro here.
Output from ":YcmToggleLogs stderr" here.
YcmDebugInfo output here.
vim --version output here.

To reproduce, throw main.cpp and .ycm_extra_conf.py files from the above gists into a directory, open with vim (I use neovim 0.1.2 on OSX via brew), and start autocompleting variables with __ in the names / types.

Thanks in advance for reading, and also thanks for making YCM. It's wonderful software.

@micbou
Copy link
Collaborator

micbou commented Mar 19, 2016

Thanks for the detailed report. This behavior is explained by this comment in ycmd sources:

We remove any two consecutive underscores from the function definition since identifiers with them are ugly, compiler-reserved names. Functions from the standard library use parameter names like "__pos" and we want to show them as just "pos". This will never interfere with client code since ANY C++ identifier with two consecutive underscores in it is compiler-reserved.

Last sentence is not completely true for C (only identifiers starting with two consecutive underscores are reserved) so maybe we should add a special case for C.

Note that it only removes the __s in the completions menu. Inserted text will keep the __s.

@charlesnicholson
Copy link
Author

Ah, thanks for the pointer to the source. I knew about leading __'s but wasn't aware that C++ reserved internal __'s as well!

For what it's worth, my repro case was C++ but my day-to-day pain with this problem is actually in C89. Teaching YCM that internal __ is safe in C would very much help my use case.

I'm curious, though. I understand the appeal of stripping leading __'s because of visual noise and it breaks naive lexicographical sort against non-prefixed identifiers. It seems, though, that if __is used internally (not as a suffix), then it's probably a delimiter between words, right? What's advantageous about stripping it out in that case? Similarly, preserving __ suffixes seems harmless. Would it make sense to only strip the leading __'s regardless of the source language?

@charlesnicholson
Copy link
Author

Two last thoughts, just to provide information / point of view:

This will never interfere with client code since ANY C++ identifier with two consecutive underscores in it is compiler-reserved.

Sometimes we write standard libraries (I work on bare-metal firmware, for example), and it would be nice to not have the text munged! It makes YCM less powerful as an authoring tool for people implementing features that are "reserved for the implementation", though I agree that they (we) are certainly in the minority of users, and it might not be worth it.

Finally, it occurred to me that there might be good use-cases around seeing the untransformed identifier. Sometimes you do use the reserved names in a read-only capacity: #ifdef __cplusplus, for example. In those cases, it seems like it might be nice to see the name, and it might be confusing to have autocomplete kick in after __, show you cplusplus, but autocomplete to __cplusplus.

Anyway, JM2C. Thanks for reading.

@micbou
Copy link
Collaborator

micbou commented Mar 19, 2016

Sometimes we write standard libraries (I work on bare-metal firmware, for example), and it would be nice to not have the text munged! It makes YCM less powerful as an authoring tool for people implementing features that are "reserved for the implementation", though I agree that they (we) are certainly in the minority of users, and it might not be worth it.

It doesn't make YCM less powerful because only the text displayed in the completion menu is changed, not the actual completion. However, I agree that it may be confusing (at first). Once you know about this behavior, I think it is fine.

Finally, it occurred to me that there might be good use-cases around seeing the untransformed identifier. Sometimes you do use the reserved names in a read-only capacity: #ifdef __cplusplus, for example. In those cases, it seems like it might be nice to see the name, and it might be confusing to have autocomplete kick in after __, show you cplusplus, but autocomplete to __cplusplus.

If you mean that you will get four underscores, that's not the case. The first two underscores will be replaced. I don't see the issue otherwise.

@charlesnicholson
Copy link
Author

If you mean that you will get four underscores, that's not the case. The first two underscores will be replaced. I don't see the issue otherwise.

Thanks for explaining. No, sorry, in retrospect it was just an example of the "confusing (at first)" behavior that you mentioned.

What are your thoughts on simply preserving internal __ when they occur in both C and C++? They're by definition separators / delimiters, right? Even in C++ it seems like they improve readability, since removing them concatenates two words that the author intended to separate.

@Valloric
Copy link
Member

I'm leaning towards not special-casing two underscores in names at all. I think introducing this special-casing in the first place might have been a (well-meaning) mistake.

@charlesnicholson
Copy link
Author

For what it's worth, I'd always prefer to see the actual names rather than an attempt at beautification, even if the actual names are ugly and/or hard to read. JM2C though.

Valloric added a commit to ycm-core/ycmd that referenced this issue Mar 20, 2016
For C-family languages, we used to remove two consecutive underscores in
identifiers out of a misguided attempt to reduce verbosity of reserved
identifiers. This only produced user confusion.

Fixes ycm-core/YouCompleteMe#2059
homu added a commit to ycm-core/ycmd that referenced this issue Mar 22, 2016
Removing special-case handling of two underscores

For C-family languages, we used to remove two consecutive underscores in
identifiers out of a misguided attempt to reduce verbosity of reserved
identifiers. This only produced user confusion.

This logic also had no tests! Oh the shame...

Fixes ycm-core/YouCompleteMe#2059

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/437)
<!-- Reviewable:end -->
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants