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

Make identifier chars configurable #236

Closed

Conversation

JacekLach
Copy link
Contributor

Some languages allow more non-alphabetic characters in identifiers than just '_'. This pull request allows the user to configure which non-alphabetic chars should be accepted as part of identifiers.

For example for clojure development I set this to '_-/.><'.

I went for the simplest solution here, but it might be worth it to make it a regex. I can make that change if it's preferred.

@JacekLach
Copy link
Contributor Author

This doesn't do everything I hoped for, unfortunately. First, a bit of context:

The / character is the namespace separator in clojure: namespace/symbol-name is how you reference anything from a different namespace. Thus I set let g:ycm_semantic_triggers = { 'clojure' : ['(', '/'] } in my .vimrc, with the purpose of getting suggestions for all names from a namespace when I press /.

For example, take (log/ as the input. I'd like to see suggestions for all symbols in the log namespace, which may be stuff like log/warn, log/error, log/debug etc.

However, if / is not an identifier character, typing (log/ will result in YouCompleteMe calling the omnicomplete function first as omnifunc(1, '') - which correctly returns 1 as the index of the base string, but then as omnifunc(0, ''), because it has itself determined that / is not an identifier character and thus the query string must be empty. That obviously doesn't provide the right suggestions.

On the other hand, if I do add / to the list of identifier characters, YouCompleteMe seems reports no completions available - because it has cached the result for (log, which was only the string 'log', and decided that since there is nothing that starts with log/, there must be no results. Or at least this is my guess - I have not spent too much time on debugging this, other than noting the omnifunc is not called.

Pressing C-XC-O on (log/ gives me the right completions, of course.

I see a couple possible solutions, and went with the one I felt simplest:

  1. Invalidate the cache on semantic trigger characters. I have no idea what impact this could have, but then one could add namespace separators to identifier chars and it should 'just work', if my idea for why there are no results presented is correct.
  2. Present both the before and after elements around a semantic trigger to the completion engine. That is, if / is a semantic trigger, and only alphanumerics are identifier characters and I type (log/err, YCM would first select err as the query string, then notice that / is a semantic separator, and read the previous identifier as well. This results in the desired query of log/err. Again, no idea on impact on other completion engines or languages.
  3. Respect the return value from omnifunc(1, ''). This is what I chose to implement. Basically it ensures that the omnifunction gets the input string it expects.

I'm open to any improvement suggestions, but the fact that it seems to make YCM usable with a clojure setup (vim-redl, vim-fireplace and vimclojure-static) should be a benefit.

@JacekLach
Copy link
Contributor Author

Woops. The second part of the pull is broken. I'll separate it into a separate pull and fix it. I think the first change stands on its own anyway.

@JacekLach JacekLach mentioned this pull request Apr 8, 2013
@Valloric
Copy link
Member

Making identifier chars configurable is substantially more complicated than it appears. IdentifierUtils.cpp needs to be changed (the regex needs to be configurable), and so do all of the tests. I'll do this at some point in the future, don't worry about it.

Also, the cache is already invalidated on semantic trigger characters. See this line. When query is an empty string, the self.completions_cache is set to None, thus killing the cache.

@Valloric Valloric closed this Apr 10, 2013
@JacekLach
Copy link
Contributor Author

Also, the cache is already invalidated on semantic trigger characters. See this line. When query is an empty string, the self.completions_cache is set to None, thus killing the cache.

Right - as long as a semantic trigger is not an identifier :) What I was aiming for was getting both the identifier before and the identifier after into the omnifunc (i.e. log.pri would ask omnifunc for completions for the entire string, not just pri).

A better approach might be to let completers choose how much of the original text they want to replace - perhaps marking that per suggestion, allowing the completer to give different length completions? The code that replaces text with the completion on selection would have to be updated, of course.

@Valloric
Copy link
Member

Right - as long as a semantic trigger is not an identifier :) What I was aiming for was getting both the identifier before and the identifier after into the omnifunc (i.e. log.pri would ask omnifunc for completions for the entire string, not just pri).

A better approach might be to let completers choose how much of the original text they want to replace - perhaps marking that per suggestion, allowing the completer to give different length completions? The code that replaces text with the completion on selection would have to be updated, of course.

That would not respect the omnifunc spec that's in the Vim docs. See :h complete-functions.

It's just easier to let the user configure the identifier characters. This also needs to be done for other reasons as well.

@Satshabad
Copy link

is this in or not? Any progress since? Would love to use this for clojure if I could.

@alxndr
Copy link

alxndr commented Dec 5, 2013

Also would love to use this for clojure

@thinkiny
Copy link

waiting clojure work with YCM

@vitillo
Copy link

vitillo commented Aug 12, 2014

It would be really great to add support for clojure.

@Valloric
Copy link
Member

People, please note that this is a pull request; the corresponding issue you're looking for is #86.

In other words, don't comment here.

bijancn pushed a commit to bijancn/YouCompleteMe that referenced this pull request Jul 26, 2016
Fix filename of comment_strip tests

`ycmd/completers/cpp/tests/comment_strip.py` tests was not run automatically by nosetests because `_test` was missing at the end of the filename.
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

6 participants