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

Add support for extracting doc from comment #74

Merged
merged 1 commit into from
Jan 2, 2015

Conversation

ptrv
Copy link
Contributor

@ptrv ptrv commented Dec 30, 2014

No description provided.

ptrv added a commit to ptrv/emacs-ycmd that referenced this pull request Dec 30, 2014
@ptrv ptrv force-pushed the support-brief-comment branch 2 times, most recently from a77e930 to 4f3a2c5 Compare December 31, 2014 11:42
ptrv added a commit to ptrv/emacs-ycmd that referenced this pull request Dec 31, 2014
@Valloric
Copy link
Member

Thanks for the PR!

You need to sign the Google CLA and state that you have (see CONTRIBUTING.md).

@@ -21,6 +21,13 @@
#include <clang-c/Index.h>
#include <string>

#if defined(CINDEX_VERSION_MAJOR) && defined(CINDEX_VERSION_MINOR) && \
(CINDEX_VERSION_MAJOR > 0 || CINDEX_VERSION_MINOR >= 6)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ycmd requires libclang version >= 3.5 which is pretty recent. If that has CINDEX_VERSION_MINOR >= 6, then you don't need this check and can just use this API without concern. A lot of the other code becomes simpler too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in libclang 3.5 the CINDEX_VERSION_MINOR is 27. I will remove the checks.

@ptrv
Copy link
Contributor Author

ptrv commented Jan 1, 2015

Ok, I have signed the Google CLA.

@ptrv ptrv force-pushed the support-brief-comment branch 2 times, most recently from a48fcf8 to 4680c19 Compare January 1, 2015 02:19
ptrv added a commit to ptrv/emacs-ycmd that referenced this pull request Jan 1, 2015
ptrv added a commit to ptrv/emacs-ycmd that referenced this pull request Jan 1, 2015
ptrv added a commit to ptrv/emacs-ycmd that referenced this pull request Jan 1, 2015
ptrv added a commit to ptrv/emacs-ycmd that referenced this pull request Jan 1, 2015
ptrv added a commit to ptrv/emacs-ycmd that referenced this pull request Jan 1, 2015
@@ -271,7 +271,8 @@ def ConvertCompletionData( completion_data ):
menu_text = completion_data.MainCompletionText(),
extra_menu_info = completion_data.ExtraMenuInfo(),
kind = completion_data.kind_.name,
detailed_info = completion_data.DetailedInfoForPreviewWindow() )
detailed_info = completion_data.DetailedInfoForPreviewWindow(),
extra_data = { 'doc_string': completion_data.DocString() } )
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Valloric Now we have always extra_data even if completion_data.DocString() returns an empty string. Would this be better?

extra_data = { 'doc_string': completion_data.DocString() } if completion_data.DocString() else None

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, go with that.

ptrv added a commit to ptrv/emacs-ycmd that referenced this pull request Jan 1, 2015
@Valloric
Copy link
Member

Valloric commented Jan 2, 2015

Thanks for the PR! For future reference, github doesn't send notifications to repo owners when you push new commits to a PR branch (longstanding github issue), so if you address my comments with new commits, you need to add some sort of issue comment for me to notice there have been updates.

Valloric added a commit that referenced this pull request Jan 2, 2015
Add support for extracting doc from comment
@Valloric Valloric merged commit ce7ee17 into ycm-core:master Jan 2, 2015
@ptrv
Copy link
Contributor Author

ptrv commented Jan 2, 2015

Thanks for merging and also for the hint about github not sending notification when PR branch has new commits!

@ptrv ptrv deleted the support-brief-comment branch January 2, 2015 22:55
@Valloric
Copy link
Member

Just realized this PR forgot to bump up the API version in versioning.cpp. Fixing it now.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f5fd319 on ptrv:support-brief-comment into * on Valloric:master*.

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

3 participants