This is an archive of the discontinued LLVM Phabricator instance.

[libclang] Optionally add code completion results for arrow instead of dot
ClosedPublic

Authored by yvvan on May 14 2018, 11:13 PM.

Diff Detail

Repository
rC Clang

Event Timeline

yvvan created this revision.May 14 2018, 11:13 PM
yvvan updated this revision to Diff 147042.May 16 2018, 3:53 AM

The base revision has changed - some minor changes were also required here.

yvvan retitled this revision from Optionally add code completion results for arrow instead of dot (libclang) to [libclang] Optionally add code completion results for arrow instead of dot.May 17 2018, 1:52 AM
yvvan updated this revision to Diff 148177.May 23 2018, 2:46 AM

Output range of token being replaced instead of it's text

As a side-effect introduce a call to get a Token starting from specified location.

yvvan updated this revision to Diff 148559.May 25 2018, 1:00 AM

Update according to c++ part changes

nik requested changes to this revision.May 29 2018, 2:54 AM

Generates build errors here:

In file included from /d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:4:0:
/d2/llvm/qtc/source/tools/clang/include/clang-c/Index.h:1332:62: warning: comma at end of enumerator list [-Wpedantic]
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c: In function 'tokens_spelling_at_range':
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:2263:3: error: 'for' loop initial declarations are only allowed in C99 or C11 mode
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:2263:3: note: use option -std=c99, -std=gnu99, -std=c11 or -std=gnu11 to compile your code
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c: In function 'print_completion_result':
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:2365:3: error: 'for' loop initial declarations are only allowed in C99 or C11 mode
At top level:
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:2257:13: warning: 'tokens_spelling_at_range' defined but not used [-Wunused-function]

include/clang-c/Index.h
85

s/string/index

88

Currently there is not more, so I would avoid the "etc.".

92

s/tokens/token

93
  • Please use proper documentation style, e.g. make use of "\brief", "\param", "\returns". Currently one has to somewhat guess that the returned string is related to the replacement_range parameter.
  • Is this a behavior change now? What happens to old code not aware of this function?
95

Please avoid double negation.

97

s/editors/clients/

98

Do you mean replacement_range instead of RemoveRange?

113

The vec_ptr-> example should go to its own line.

test/Index/complete-arrow-dot.cpp
21

That's pretty verbose, consider to use wildcards/regexes instead just checking for "requires fix-it:".

tools/c-index-test/c-index-test.c
41

Keep terminology of the API - s/CORRECTIONS/FIXITS

This revision now requires changes to proceed.May 29 2018, 2:54 AM
yvvan updated this revision to Diff 149049.May 29 2018, 11:31 PM
yvvan marked 6 inline comments as done.

Address review comments

include/clang-c/Index.h
88

It might change without Index.h being updated.

93

'\brief' is not used anymore.

Is this a behavior change now?

No, without CXCodeComplete_IncludeFixIts set you won't event have extra completions. And existing ones won't get any fix-its (because why would they?)

98

Yes, it's repalcement_range here

113

I've fixed it the same way it's done in CodeCompleteConsumer.h

nik requested changes to this revision.Jun 6 2018, 3:28 AM
nik added inline comments.
include/clang-c/Index.h
85

Indicate that calling this makes only sense if CXCodeComplete_IncludeFixIts was set.

86

"Tokenize the source code" sounds as if more than one token will be handled.

Suggestion: Reduce to "Get the raw lexical token starting with the given location".

90

Suggestion: CXCodeComplete_IncludeCompletionsWithFixIts

93

Still \param and \returns are useful since there the range is an output argument.

95

Clarify that you refer to clang_codeCompleteAt here.

Suggestion: By default, clang_codeCompleteAt()....

97

Add empty line before "For the client..." for readability.

98

...of the cursor after applying the fixits

106

new line

110

new line

112

"one of the ...", same below.

This revision now requires changes to proceed.Jun 6 2018, 3:28 AM
nik added a comment.Jun 6 2018, 3:32 AM
  • Sometimes you refer to "fixits", sometimes "fix-its" and some times "FixIts". Unify to what is already there.
  • The term "completion items" is new so far. Use "completions" for consistency.
yvvan updated this revision to Diff 150291.Jun 7 2018, 3:57 AM
yvvan marked 8 inline comments as done.

Address review comments

nik added inline comments.Jun 13 2018, 3:05 AM
include/clang-c/Index.h
87

s/completion items/completions/g

126

Please document the parameters. It's more important here than for any other function that was introduced.

yvvan updated this revision to Diff 151123.Jun 13 2018, 4:36 AM
yvvan marked 2 inline comments as done.

Address comments about Index.h new functions parameters

yvvan updated this revision to Diff 151124.Jun 13 2018, 4:41 AM

Diff with unix line endings

nik accepted this revision.Jun 13 2018, 4:48 AM
This revision is now accepted and ready to land.Jun 13 2018, 4:48 AM
This revision was automatically updated to reflect the committed changes.