[PATCH] D22087: [clang-rename] add basic vim integration

Benjamin Kramer via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 7 05:02:12 PDT 2016


bkramer added inline comments.

================
Comment at: clang-rename/tool/ClangRename.cpp:39
@@ -38,3 +38,2 @@
 #include "llvm/Support/Host.h"
-#include <cstdlib>
 #include <string>
----------------
This looks unrelated.

================
Comment at: clang-rename/tool/clang-rename.py:14
@@ +13,3 @@
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cf'. You will be promted a new name if the
----------------
The comment should say that you have to save the file before running the tool.

================
Comment at: clang-rename/tool/clang-rename.py:40
@@ +39,3 @@
+    command = [binary,
+               filename,
+               '-i',
----------------
Maybe add a FIXME to input the unsaved file via stdin like we do for clang-format?

================
Comment at: clang-rename/tool/clang-rename.py:47
@@ +46,3 @@
+                         stderr=subprocess.PIPE,
+                         stdin=subprocess.PIPE)
+    stdout, stderr = p.communicate()
----------------
stdin is pipe but you never write to it? Is that intentional?

================
Comment at: clang-rename/tool/clang-rename.py:48
@@ +47,3 @@
+                         stdin=subprocess.PIPE)
+    stdout, stderr = p.communicate()
+
----------------
Should we output stderr in case the tool fails?


http://reviews.llvm.org/D22087





More information about the cfe-commits mailing list