code review 4276059: misc/emacs: gofmt: don't clobber the current buffer on ...

misc/emacs/go-mode.el

 ;;; go-mode.el --- Major mode for the Go programming language
 
 ;;; Commentary:
 
 ;; For installation instructions, see go-mode-load.el
 
 ;;; To do:
 
 ;; * Indentation is *almost* identical to gofmt
 ;; ** We think struct literal keys are labels and outdent them
@@ skipping 480 matching lines @@
 
 (defun go-mode-reload ()
   "Reload go-mode.el and put the current buffer into Go mode.
 Useful for development work."
 
   (interactive)
   (unload-feature 'go-mode)
   (require 'go-mode)
   (go-mode))
 
-(provide 'go-mode)
-
-(defun gofmt-buffer (outbuf replace errbuf)
-  ;; for some reason save-excursion isn't working
-  ;; probably because shell-command-on-region deletes the contents of the
-  ;; region before filling in the new values
-  ;; so we will save the point/mark by hand
-  ;; similarly we can't use push-mark/pop-mark
-  (let ((old-mark (mark t)) (old-point (point)))
-    (save-restriction
-      (let (deactivate-mark)
-        (widen)
-        (setq exit-status (shell-command-on-region (point-min) (point-max) "gofmt"
-                                                   outbuf replace errbuf))))
-    (goto-char (min old-point (point-max)))
-    (if old-mark (set-mark (min old-mark (point-max)))))
-  exit-status)
-
+;;;###autoload
 (defun gofmt ()
-  "Pipe the current buffer through the external tool `gofmt`."
-··
-  (interactive)
-  (gofmt-buffer t t shell-command-default-error-buffer))
-
-(defun get-empty-buffer (name)
-  "Finds or creates the buffer named name, erases it, and returns it."
-
-  (let ((buf (get-buffer-create name)))
-    (save-current-buffer
-      (set-buffer buf)
-      (erase-buffer))
-    buf))
+ "Pipe the current buffer through the external tool `gofmt`.
+Replace the current buffer on success; display errors on failure."
+
+ (interactive)
+ (let ((srcbuf (current-buffer)))
+   (with-temp-buffer
+     (let ((outbuf (current-buffer))
+           (errbuf (get-buffer-create "*Gofmt Errors*")))
+       (with-current-buffer errbuf (erase-buffer))
+       (with-current-buffer srcbuf
+         (save-restriction
+           (let (deactivate-mark)
+             (widen)
+             (if (= 0 (shell-command-on-region (point-min) (point-max) "gofmt"
+                                               outbuf nil errbuf))
+                 ;; gofmt succeeded: replace the current buffer with outbuf,
+                 ;; restore the mark and point, and discard errbuf.
+                 (let ((old-mark (mark t)) (old-point (point)))
+                   (erase-buffer)
+                   (insert-buffer-substring outbuf)
+                   (goto-char (min old-point (point-max)))
+                   (if old-mark (set-mark (min old-mark (point-max))))
+                   (kill-buffer errbuf))
+
+               ;; gofmt failed: display the errors
+               (display-buffer errbuf)))))
+
+       ;; Collapse any window opened on outbuf if shell-command-on-region
+       ;; displayed it.
+       (delete-windows-on outbuf)))))
 
 ;;;###autoload
 (defun gofmt-before-save ()
-  "Add this to .emacs to run gofmt on the current buffer when saving:
-  (add-hook 'before-save-hook 'gofmt-before-save)"
-
-  (interactive)
-  (when (eq major-mode 'go-mode)

[Sameer (personal), 2011/03/19 02:01:28]

This when guard is the only bit that's specific to the hook behavior (since
before-save-hook runs in all modes, and we only want to run the rest of this on
Go files).  So we could just change all of this to:
  (defun gofmt ...the code below...)
  (defun gofmt-before-save (when (eq major-mode 'go-mode) gofmt))
Provided we're willing to change how M-x gofmt works for people now.

If we want to retain the current behavior of M-x gofmt, another option is to do:
  (defun gofmt-noclobber ...the code below...)
  (defun gofmt-before-save (when (eq major-mode 'go-mode) gofmt-noclobber))

-    (let ((outbuf (get-empty-buffer "Gofmt Output"))
-          (errbuf (get-empty-buffer "Gofmt Errors")))
-      (if (= 0 (gofmt-buffer outbuf nil errbuf))
-
-          ;; gofmt succeeded: copy outbuf into the current buffer,
-          ;; preserving the current mark and point
-          (let ((old-mark (mark t)) (old-point (point)))
-            (erase-buffer)
-            (insert-buffer-substring outbuf)
-            (goto-char (min old-point (point-max)))
-            (if old-mark (set-mark (min old-mark (point-max))))
-            (kill-buffer errbuf))
-
-        ;; gofmt failed: display the errors
-        (display-buffer errbuf))
-
-      ;; Kill outbuf, collapsing its window first if
-      ;; shell-command-on-region displayed it.
-      (delete-windows-on outbuf)
-      (kill-buffer outbuf))))
+ "Add this to .emacs to run gofmt on the current buffer when saving:
+ (add-hook 'before-save-hook #'gofmt-before-save)"
+
+ (interactive)
+ (when (eq major-mode 'go-mode) (gofmt)))
+
+(provide 'go-mode)