Overhaul clang-rename.el.

[phst]

Summary

• Use lexical binding, as recommended for new libraries.
• Fix customization variable (set correct group and type).
• Create a new customization group for the library.
• Autoload the main clang-rename command so that users don't have to explicitly load the library.
• Correctly translate between file and buffer positions using bufferpos-to-filepos (if available) or a fallback.
• Don't invoke the shell, it's not necessary and adds complexity.
• Save clang-rename output in a buffer and display that on failure.
• Save all buffers before calling clang-rename. This is required anyway and prevents data loss when the buffer is later reverted.
• In revert-buffer, use keywords instead of t for Boolean arguments to improve readability.
• Don't reset buffer modes when reverting.
• Emacs treats the character after a symbol as part of the symbol, while clang-rename doesn't; resolve this inconsistency.
• Formatting.

[omtcyfz]

This is really great! Thank you so much for working on this!

Do you have commit access or should I land the patch?

[phst]

I don't have commit access, would be great if you could commit it.

[omtcyfz]

In D25156#558506, @phst wrote:

I don't have commit access, would be great if you could commit it.

Great; will do. Thank you very much once more!

clang-tools-extra/trunk/clang-rename/tool/clang-rename.el

-;;; clang-rename.el --- Renames every occurrence of a symbol found at <offset>.
+;;; clang-rename.el --- Renames every occurrence of a symbol found at <offset>.  -*- lexical-binding: t; -*-
 
 ;; Keywords: tools, c
 
 ;;; Commentary:
 
 ;; To install clang-rename.el make sure the directory of this file is in your
-;; 'load-path' and add
+;; `load-path' and add
 ;;
 ;;   (require 'clang-rename)
 ;;
 ;; to your .emacs configuration.
 
 ;;; Code:
 
+(defgroup clang-rename nil
+  "Integration with clang-rename"
+  :group 'c)
+
 (defcustom clang-rename-binary "clang-rename"
   "Path to clang-rename executable."
-  :type 'hook
-  :options '(turn-on-auto-fill flyspell-mode)
-  :group 'wp)
+  :type '(file :must-match t)
+  :group 'clang-rename)
 
+;;;###autoload
 (defun clang-rename (new-name)
-  "Rename all instances of the symbol at the point using clang-rename"
+  "Rename all instances of the symbol at point to NEW-NAME using clang-rename."
   (interactive "sEnter a new name: ")
-  (let (;; Emacs offset is 1-based.
-        (offset (- (point) 1))
-        (orig-buf (current-buffer))
-        (file-name (buffer-file-name)))
-
-    (let ((rename-command
-          (format "bash -f -c '%s -offset=%s -new-name=%s -i %s'"
-                               clang-rename-binary offset new-name file-name)))
-          (message (format "Running clang-rename command %s" rename-command))
-          ;; Run clang-rename via bash.
-          (shell-command rename-command)
-          ;; Reload buffer.
-          (revert-buffer t t)
-    )
-  )
-)
+  (save-some-buffers :all)
+  ;; clang-rename should not be combined with other operations when undoing.
+  (undo-boundary)
+  (let ((output-buffer (get-buffer-create "*clang-rename*")))
+    (with-current-buffer output-buffer (erase-buffer))
+    (let ((exit-code (call-process
+                      clang-rename-binary nil output-buffer nil
+                      (format "-offset=%d"
+                              ;; clang-rename wants file (byte) offsets, not
+                              ;; buffer (character) positions.
+                              (clang-rename--bufferpos-to-filepos
+                               ;; Emacs treats one character after a symbol as
+                               ;; part of the symbol, but clang-rename doesn’t.
+                               ;; Use the beginning of the current symbol, if
+                               ;; available, to resolve the inconsistency.
+                               (or (car (bounds-of-thing-at-point 'symbol))
+                                   (point))
+                               'exact))
+                      (format "-new-name=%s" new-name)
+                      "-i" (buffer-file-name))))
+      (if (and (integerp exit-code) (zerop exit-code))
+          ;; Success; revert current buffer so it gets the modifications.
+          (progn
+            (kill-buffer output-buffer)
+            (revert-buffer :ignore-auto :noconfirm :preserve-modes))
+        ;; Failure; append exit code to output buffer and display it.
+        (let ((message (format-message
+                        "clang-rename failed with %s %s"
+                        (if (integerp exit-code) "exit status" "signal")
+                        exit-code)))
+          (with-current-buffer output-buffer
+            (insert ?\n message ?\n))
+          (message "%s" message)
+          (display-buffer output-buffer))))))
+
+(defalias 'clang-rename--bufferpos-to-filepos
+  (if (fboundp 'bufferpos-to-filepos)
+      'bufferpos-to-filepos
+    ;; Emacs 24 doesn’t have ‘bufferpos-to-filepos’, simulate it using
+    ;; ‘position-bytes’.
+    (lambda (position &optional _quality _coding-system)
+      (1- (position-bytes position)))))
 
 (provide 'clang-rename)
 
 ;;; clang-rename.el ends here