[PATCH] D23006: [clang-rename] add basic Emacs integration

[Haojian Wu via cfe-commits]

hokein added inline comments.

================
Comment at: clang-rename/tool/clang-rename.el:16
@@ +15,3 @@
+
+(defvar clang-rename-binary "clang-rename")
+
----------------
I think we should make `clang-rename` binary path configurable by making it a custom variable (using `defcustom`).

================
Comment at: clang-rename/tool/clang-rename.el:20
@@ +19,3 @@
+  "Rename all instances of the symbol at the point using clang-rename"
+  (interactive "sEnter a new name: ")
+  (let (;; Emacs offset is 1-based.
----------------
`s` is an extra character here?

================
Comment at: clang-rename/tool/clang-rename.el:27
@@ +26,3 @@
+    (let ((rename-command
+          (format "bash -f -c '%s -offset=%s -new-name=%s -i %s'"
+                               clang-rename-binary offset new-name file-name)))
----------------
Any reason why not use `call-process-region`?

================
Comment at: docs/clang-rename.rst:106
@@ +105,3 @@
+clang-rename Emacs integration
+============================
+
----------------
missing two "=".

================
Comment at: docs/clang-rename.rst:114
@@ +113,3 @@
+Once installed, you can point your cursor to symbols you want to rename, press
+`M-X`, print `clang-rename` and new desired name.
+
----------------
`print` doesn't make sense here? I think user should type `clang-rename` and new name.


https://reviews.llvm.org/D23006