Hello Tobias, Thank you for the review! Comments inline.
On Tue, Sep 11, 2012 at 1:19 AM, Tobias Grosser <[email protected]> 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 <[email protected]>*/
cindex-py-get-completion-brief-comment-v2.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
