fix python3 compability issue

[roxma]

Summary

The file contents could be of str type. Should use byte length instead
of str length, otherwise utf-8 encoded files may not get properly parsed
for completion.

Source issue: https://github.com/ncm2/ncm2-pyclang#2

[serge-sans-paille]

bindings/python/clang/cindex.py

2998 ↗

(On Diff #180620)

Why did you remove the FIXME comment?

[serge-sans-paille]

bindings/python/clang/cindex.py

2998 ↗

(On Diff #180620)

@roxma LGTM except this FIXME removal.

[roxma]

bindings/python/clang/cindex.py

2998 ↗

(On Diff #180620)

@serge-sans-paille

the contents.read() looks almost the same as line 2817.

It is better to keep the code consistent. The FIXME doesn't seem to be helpful.

[jstasiak]

bindings/python/clang/cindex.py

3001 ↗

(On Diff #180620)

Nitpicking: the description only mentions changes related to file contents but this modification (fspath(name) -> b(fspath(name))) likely fixes a different issue, it may be worth mentioning this in the commit message. Unless this extra b() call is not strictly necessary here but added for consistency?

[serge-sans-paille]

bindings/python/clang/cindex.py

2998 ↗

(On Diff #180620)

ok, I'll trust you on this.

[serge-sans-paille]

@roxma: do you want me to commit this on your behalf?

[roxma]

Yes please.

[serge-sans-paille]

Thanks @roxma for the patch!

cfe/trunk/bindings/python/clang/cindex.py

def from_source(cls, filename, args=None, unsaved_files=None, options=0,
             args_array = (c_char_p * len(args))(*[b(x) for x in args])
 
         unsaved_array = None
         if len(unsaved_files) > 0:
             unsaved_array = (_CXUnsavedFile * len(unsaved_files))()
             for i, (name, contents) in enumerate(unsaved_files):
                 if hasattr(contents, "read"):
                     contents = contents.read()
-
+                contents = b(contents)
                 unsaved_array[i].name = b(fspath(name))
-                unsaved_array[i].contents = b(contents)
+                unsaved_array[i].contents = contents
                 unsaved_array[i].length = len(contents)
 
         ptr = conf.lib.clang_parseTranslationUnit(index,
                                     fspath(filename) if filename is not None else None,
                                     args_array,
                                     len(args), unsaved_array,
                                     len(unsaved_files), options)
 
def reparse(self, unsaved_files=None, options=0):
         file. The contents may be passed as strings or file objects.
         """
         if unsaved_files is None:
             unsaved_files = []
 
         unsaved_files_array = 0
         if len(unsaved_files):
             unsaved_files_array = (_CXUnsavedFile * len(unsaved_files))()
-            for i,(name,value) in enumerate(unsaved_files):
-                if not isinstance(value, str):
-                    # FIXME: It would be great to support an efficient version
-                    # of this, one day.
-                    value = value.read()
-                    print(value)
-                if not isinstance(value, str):
-                    raise TypeError('Unexpected unsaved file contents.')
-                unsaved_files_array[i].name = fspath(name)
-                unsaved_files_array[i].contents = value
-                unsaved_files_array[i].length = len(value)
+            for i,(name,contents) in enumerate(unsaved_files):
+                if hasattr(contents, "read"):
+                    contents = contents.read()
+                contents = b(contents)
+                unsaved_files_array[i].name = b(fspath(name))
+                unsaved_files_array[i].contents = contents
+                unsaved_files_array[i].length = len(contents)
         ptr = conf.lib.clang_reparseTranslationUnit(self, len(unsaved_files),
                 unsaved_files_array, options)
 
     def save(self, filename):
         """Saves the TranslationUnit to a file.
 
         This is equivalent to passing -emit-ast to the clang frontend. The
         saved file can be loaded back into a TranslationUnit. Or, if it
def codeComplete(self, path, line, column, unsaved_files=None,
             options += 4
 
         if unsaved_files is None:
             unsaved_files = []
 
         unsaved_files_array = 0
         if len(unsaved_files):
             unsaved_files_array = (_CXUnsavedFile * len(unsaved_files))()
-            for i,(name,value) in enumerate(unsaved_files):
-                if not isinstance(value, str):
-                    # FIXME: It would be great to support an efficient version
-                    # of this, one day.
-                    value = value.read()
-                    print(value)
-                if not isinstance(value, str):
-                    raise TypeError('Unexpected unsaved file contents.')
+            for i,(name,contents) in enumerate(unsaved_files):
+                if hasattr(contents, "read"):
+                    contents = contents.read()
+                contents = b(contents)
                 unsaved_files_array[i].name = b(fspath(name))
-                unsaved_files_array[i].contents = b(value)
-                unsaved_files_array[i].length = len(value)
+                unsaved_files_array[i].contents = contents
+                unsaved_files_array[i].length = len(contents)
         ptr = conf.lib.clang_codeCompleteAt(self, fspath(path), line, column,
                 unsaved_files_array, len(unsaved_files), options)
         if ptr:
             return CodeCompletionResults(ptr)
         return None
 
     def get_tokens(self, locations=None, extent=None):
         """Obtain tokens in this translation unit.