cmd/guru: in Emacs, change 'definition' to jump directly

go-guru--run has been split up to separate running the tool
from turning its output into compilation-mode form.
The definition command uses only the first part,
and parses its output in JSON form.

Added test, factoring the test script.

Change-Id: I4c3e4a51a1346551a3703a5e3137c878d7b2d95f
Reviewed-on: https://go-review.googlesource.com/19499
Reviewed-by: Michael Matloob <matloob@golang.org>

cmd/guru/emacs-test.bash

@@ -26,12 +26,22 @@ mv -f $GOPATH/bin/guru $GOROOT/bin/
 $GOROOT/bin/guru >$log 2>&1 || true # (prints usage and exits 1)
 grep -q "Run.*help" $log || die "$GOROOT/bin/guru not installed"
 
-# Run Emacs, set the scope to the guru tool itself,
+# Usage: run_emacs <elisp>
-# load ./main.go, and describe the "fmt" import.
+function run_emacs() {
-emacs --batch --no-splash --no-window-system --no-init \
+    emacs --batch --no-splash --no-window-system --no-init \
 	  --load $GOPATH/src/github.com/dominikh/go-mode.el/go-mode.el \
 	  --load $thisdir/guru.el \
-    --eval '
+	  --eval "$1" >$log 2>&1 || die "emacs command failed"
+}
+
+# Usage: expect_log <regex>
+function expect_log() {
+    grep -q "$1" $log || die "didn't find expected lines in log; got:"
+}
+
+# Load main.go and describe the "fmt" import.
+# Check that Println is mentioned.
+run_emacs '
 (progn
   (princ (emacs-version)) ; requires Emacs v23
   (find-file "'$thisdir'/main.go")
@@ -42,9 +52,22 @@ emacs --batch --no-splash --no-window-system --no-init \
   (princ (with-current-buffer "*go-guru*"
                               (buffer-substring-no-properties (point-min) (point-max))))
   (kill-emacs 0))
-' main.go >$log 2>&1 || die "emacs command failed"
+'
+expect_log "fmt/print.go.*func  Println"
 
-# Check that Println is mentioned.
+# Jump to the definition of flag.Bool.
-grep -q "fmt/print.go.*func  Println" $log || die "didn't find expected lines in log; got:"
+run_emacs '
+(progn
+  (find-file "'$thisdir'/main.go")
+  (search-forward "flag.Bool")
+  (backward-char)
+  (go-guru-definition)
+  (message "file: %s" (buffer-file-name))
+  (message "line: %s" (buffer-substring (line-beginning-position)
+                                        (line-end-position)))
+  (kill-emacs 0))
+'
+expect_log "^file: .*flag.go"
+expect_log "^line: func Bool"
 
 echo "PASS"

cmd/guru/guru.el

@@ -11,6 +11,7 @@
 
 (require 'compile)
 (require 'go-mode)
+(require 'json)
 (require 'simple)
 (require 'cl)
 
@@ -61,10 +62,10 @@ previous scope.
 
 The scope specifies a set of arguments, separated by spaces.
 It may be:
-1) a set of packages whose main() functions will be analyzed.
+1) a set of packages whose main functions will be analyzed.
 2) a list of *.go filenames; they will treated like as a single
    package (see #3).
-3) a single package whose main() function and/or Test* functions
+3) a single package whose main function and/or Test* functions
    will be analyzed.
 
 In the common case, this is similar to the argument(s) you would
@@ -80,10 +81,18 @@ specify to 'go build'."
     (setq go-guru-scope scope)))
 
 (defun go-guru--run (mode &optional need-scope)
-  "Run the Go guru in the specified MODE, passing it the
+  "Run the Go guru in the specified MODE, passing it the selected
+region of the current buffer.  If NEED-SCOPE, prompt for a scope
+if not already set.  Mark up the output using `compilation-node`,
+replacing each file name with a small hyperlink, and display the
+result."
+  (with-current-buffer (go-guru--exec mode need-scope)
+    (go-guru--compilation-markup)))
+
+(defun go-guru--exec (mode &optional need-scope flags)
+  "Execute the Go guru in the specified MODE, passing it the
 selected region of the current buffer.  If NEED-SCOPE, prompt for
-a scope if not already set.  Process the output to replace each
+a scope if not already set.  Return the output buffer."
-file name with a small hyperlink.  Display the result."
   (if (not buffer-file-name)
       (error "Cannot use guru on a buffer without a file name"))
   (and need-scope
@@ -104,16 +113,15 @@ file name with a small hyperlink.  Display the result."
 	 (output-buffer (get-buffer-create "*go-guru*")))
     (with-current-buffer output-buffer
       (setq buffer-read-only nil)
-      (erase-buffer)
+      (erase-buffer))
-      (insert "Go Guru\n"))
     (with-current-buffer (get-buffer-create "*go-guru-input*")
       (setq buffer-read-only nil)
       (erase-buffer)
       (go-guru--insert-modified-files)
-      (let* ((args (list "-modified"
+      (let* ((args (append (list "-modified"
-			 "-scope" go-guru-scope
+				 "-scope" go-guru-scope)
-			 mode
+			   flags
-			 posn)))
+			   (list mode posn))))
 	;; Log the command to *Messages*, for debugging.
 	(message "Command: %s:" args)
 	(message nil) ; clears/shrinks minibuffer
@@ -127,7 +135,10 @@ file name with a small hyperlink.  Display the result."
 						     output-buffer
 						     t)
 					       args)))))
-    (with-current-buffer output-buffer
+    output-buffer))
+
+(defun go-guru--compilation-markup ()
+  "Present guru output in the current buffer using `compilation-mode'."
   (insert "\n")
   (compilation-mode)
   (setq compilation-error-screen-columns nil)
@@ -162,9 +173,8 @@ file name with a small hyperlink.  Display the result."
     (message nil))
   
   (let ((w (display-buffer (current-buffer))))
-        (balance-windows)
     (shrink-window-if-larger-than-buffer w)
-        (set-window-point w (point-min))))))
+    (set-window-point w (point-min))))
 
 (defun go-guru--insert-modified-files ()
   "Insert the contents of each modified Go buffer into the
@@ -180,6 +190,16 @@ current buffer in the format specified by guru's -modified flag."
 		   (insert-buffer-substring b))))
 	(buffer-list)))
 
+(defun go-guru--goto-pos (posn)
+  "Find the file containing the position POSN (of the form `file:line:col')
+set the point to it, switching the current buffer."
+  (let ((file-line-pos (split-string posn ":")))
+    (find-file (car file-line-pos))
+    (goto-char (point-min))
+    ;; NB: go/token's column offsets are byte- not rune-based.
+    (forward-line (1- (string-to-number (cadr file-line-pos))))
+    (forward-char (1- (string-to-number (caddr file-line-pos))))))
+
 (defun go-guru-callees ()
   "Show possible callees of the function call at the current point."
   (interactive)
@@ -202,9 +222,16 @@ function containing the current point."
   (go-guru--run "callstack" t))
 
 (defun go-guru-definition ()
-  "Show the definition of the selected identifier."
+  "Jump to the definition of the selected identifier."
   (interactive)
-  (go-guru--run "definition"))
+  ;; TODO(adonovan): use -format=sexpr when available to avoid a
+  ;; dependency and to simplify parsing.
+  (let* ((res (with-current-buffer (go-guru--exec "definition" nil '("-format=json"))
+	       (goto-char (point-min))
+	       (cdr (car (json-read)))))
+	 (desc (cdr (assoc 'desc res))))
+    (go-guru--goto-pos (cdr (assoc 'objpos res)))
+    (message "%s" desc)))
 
 (defun go-guru-describe ()
   "Describe the selected syntax, its kind, type and methods."