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

[Tobias Grosser]

On 09/10/2012 03:56 PM, Dmitri Gribenko wrote:
> Hello,
>
> The attached patch adds briefComment property to CompletionString.

Nice.

> 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.

> 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.

(You are right that returning a string directly could have some 
advantages, but, if this is the case, we should fix it everywhere and 
apply such a fix in a separate patch)

> +    # 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.

> +    # Whether to include macros within the set of code completions returned.
> +    CODE_COMPLETE_INCLUDE_MACROS = 1
> +
> +    # Whether to include code patterns for language constructs within the set
> +    # of code completions, e.g., for loops.
> +    CODE_COMPLETE_INCLUDE_CODE_PATTERNS = 2
> +
> +    # Whether to include brief documentation within the set of code completions
> +    # returned.
> +    CODE_COMPLETE_INCLUDE_BRIEF_COMMENTS = 4

As mentioned above, you may consider to not expose them at all, but to 
use named parameters.

Cheers
Tobi

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