[PATCH] D56429: fix python3 compability issue
serge-sans-paille added a comment. Thanks @roxma for the patch! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56429/new/ https://reviews.llvm.org/D56429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56429: fix python3 compability issue
This revision was automatically updated to reflect the committed changes. Closed by commit rL352039: Fix python3 compability issue in clang binding (authored by serge_sans_paille, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D56429?vs=180620=183276#toc Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56429/new/ https://reviews.llvm.org/D56429 Files: cfe/trunk/bindings/python/clang/cindex.py Index: cfe/trunk/bindings/python/clang/cindex.py === --- cfe/trunk/bindings/python/clang/cindex.py +++ cfe/trunk/bindings/python/clang/cindex.py @@ -2814,9 +2814,9 @@ 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, @@ -2993,17 +2993,13 @@ 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) @@ -3057,17 +3053,13 @@ 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: Index: cfe/trunk/bindings/python/clang/cindex.py === --- cfe/trunk/bindings/python/clang/cindex.py +++ cfe/trunk/bindings/python/clang/cindex.py @@ -2814,9 +2814,9 @@ 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, @@ -2993,17 +2993,13 @@ 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): -
[PATCH] D56429: fix python3 compability issue
roxma added a comment. Yes please. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56429/new/ https://reviews.llvm.org/D56429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56429: fix python3 compability issue
serge-sans-paille added a comment. @roxma: do you want me to commit this on your behalf? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56429/new/ https://reviews.llvm.org/D56429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56429: fix python3 compability issue
serge-sans-paille accepted this revision. serge-sans-paille added inline comments. This revision is now accepted and ready to land. Comment at: bindings/python/clang/cindex.py:2998 +for i,(name,contents) in enumerate(unsaved_files): +if hasattr(contents, "read"): +contents = contents.read() roxma wrote: > serge-sans-paille wrote: > > serge-sans-paille wrote: > > > Why did you remove the FIXME comment? > > @roxma LGTM except this FIXME removal. > @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. ok, I'll trust you on this. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56429/new/ https://reviews.llvm.org/D56429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56429: fix python3 compability issue
jstasiak marked an inline comment as done. jstasiak added inline comments. Comment at: bindings/python/clang/cindex.py:3001 +contents = b(contents) +unsaved_files_array[i].name = b(fspath(name)) +unsaved_files_array[i].contents = contents 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? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56429/new/ https://reviews.llvm.org/D56429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56429: fix python3 compability issue
roxma marked an inline comment as done. roxma added inline comments. Comment at: bindings/python/clang/cindex.py:2998 +for i,(name,contents) in enumerate(unsaved_files): +if hasattr(contents, "read"): +contents = contents.read() serge-sans-paille wrote: > serge-sans-paille wrote: > > Why did you remove the FIXME comment? > @roxma LGTM except this FIXME removal. @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. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56429/new/ https://reviews.llvm.org/D56429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56429: fix python3 compability issue
serge-sans-paille added inline comments. Comment at: bindings/python/clang/cindex.py:2998 +for i,(name,contents) in enumerate(unsaved_files): +if hasattr(contents, "read"): +contents = contents.read() serge-sans-paille wrote: > Why did you remove the FIXME comment? @roxma LGTM except this FIXME removal. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56429/new/ https://reviews.llvm.org/D56429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56429: fix python3 compability issue
serge-sans-paille added inline comments. Comment at: bindings/python/clang/cindex.py:2998 +for i,(name,contents) in enumerate(unsaved_files): +if hasattr(contents, "read"): +contents = contents.read() Why did you remove the FIXME comment? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56429/new/ https://reviews.llvm.org/D56429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56429: fix python3 compability issue
roxma created this revision. roxma added reviewers: ilya-biryukov, mgorny. Herald added subscribers: cfe-commits, arphaman. Herald added a reviewer: serge-sans-paille. 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 Repository: rC Clang https://reviews.llvm.org/D56429 Files: bindings/python/clang/cindex.py Index: bindings/python/clang/cindex.py === --- bindings/python/clang/cindex.py +++ bindings/python/clang/cindex.py @@ -2815,9 +2815,9 @@ 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, @@ -2994,17 +2994,13 @@ 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) @@ -3058,17 +3054,13 @@ 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: Index: bindings/python/clang/cindex.py === --- bindings/python/clang/cindex.py +++ bindings/python/clang/cindex.py @@ -2815,9 +2815,9 @@ 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, @@ -2994,17 +2994,13 @@ 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