[cfe-commits] [PATCH] Add bindings for clang_getCompletionBriefComment to cindex.py

[Dmitri Gribenko]

Hello Tobias,

Thank you for the review!  Comments inline.

On Tue, Sep 11, 2012 at 1:19 AM, Tobias Grosser
<grosser at fim.uni-passau.de> wrote:
> On 09/10/2012 03:56 PM, Dmitri Gribenko wrote:
>> In order to take advantage of this functionality, we also need to
>> expose CXCodeComplete_Flags enum.  Is TranslationUnit class a good
>> place to put these constants?
>
>
> The way you implemented this mirrors exactly the way we have implemented the
> parsing flags. This means, it is definitely a reasonable solution.
>
> However, looking at it, I got the feeling that there may be a nicer way.
> What about not exposing those flags at all, but adding some optional
> parameters to the complete function:
>
> def codeComplete(self, path, line, column, unsaved_files=None,
>                  include_macros=False, include_code_patterns=False,
>                  include_brief_comments=False):
>   options = 0
>
>   if include_macros:
>     options += 1
>
>   if include_code_patterns:
>     options += 2
>
>   if include_brief_comments:
>     options += 4
>
> This seems to be a nicer interface. C not having named parameters, does not
> mean we cannot use them to enhance the python interface.

This is much better.  Changed.

>> Index: bindings/python/clang/cindex.py
>> ===================================================================
>> --- bindings/python/clang/cindex.py     (revision 163506)
>> +++ bindings/python/clang/cindex.py     (working copy)
>> @@ -1735,6 +1735,10 @@
>>           res = conf.lib.clang_getCompletionAvailability(self.obj)
>>           return availabilityKinds[res]
>>
>> +    @property
>> +    def briefComment(self):
>> +        return
>> conf.lib.clang_getCompletionBriefComment(self.obj).spelling
>
>
> Until today all methods return directly a CXString, but don't call
> '.spelling' on it. It seems to make sense to stay consistent here.

Done.

>> +    # Used to indicate that brief documentation comments should be
>> included
>> +    # into the set of code completions returned from this translation
>> unit.
>> +    INCLUDE_BRIEF_COMMENTS_IN_CODE_COMPLETION = 128
>
>
> I would call it:
>
>        PARSE_INCLUDE_BRIEF_COMMENTS_IN_CODE_COMPLETION = 128
>
> All elements of this enum are prefixed with PARSE_. For consistency, we
> should continue to do so.

Oversight on my side.  Done.

> P.S.: This would be a great opportunity to get our first code completion
> test case.

Please take a look.  Is that good enough?

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cindex-py-get-completion-brief-comment-v2.patch
Type: application/octet-stream
Size: 3845 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120914/84175829/attachment.obj>