From 19c83edf02aa9d6a1f19eda0df366253116e0911 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 15 Feb 2016 11:45:55 -0500 Subject: [PATCH] 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 --- cmd/guru/emacs-test.bash | 41 ++++++++++--- cmd/guru/guru.el | 127 ++++++++++++++++++++++++--------------- 2 files changed, 109 insertions(+), 59 deletions(-) diff --git a/cmd/guru/emacs-test.bash b/cmd/guru/emacs-test.bash index 5036d3d0..64a38d35 100755 --- a/cmd/guru/emacs-test.bash +++ b/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, -# load ./main.go, and describe the "fmt" import. -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 ' +# Usage: run_emacs +function run_emacs() { + 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 "$1" >$log 2>&1 || die "emacs command failed" +} + +# Usage: expect_log +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. -grep -q "fmt/print.go.*func Println" $log || die "didn't find expected lines in log; got:" +# Jump to the definition of flag.Bool. +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" diff --git a/cmd/guru/guru.el b/cmd/guru/guru.el index e9fed471..137d35e2 100644 --- a/cmd/guru/guru.el +++ b/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 -file name with a small hyperlink. Display the result." +a scope if not already set. Return the output buffer." (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) - (insert "Go Guru\n")) + (erase-buffer)) (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" - "-scope" go-guru-scope - mode - posn))) + (let* ((args (append (list "-modified" + "-scope" go-guru-scope) + flags + (list mode posn)))) ;; Log the command to *Messages*, for debugging. (message "Command: %s:" args) (message nil) ; clears/shrinks minibuffer @@ -127,44 +135,46 @@ file name with a small hyperlink. Display the result." output-buffer t) args))))) - (with-current-buffer output-buffer - (insert "\n") - (compilation-mode) - (setq compilation-error-screen-columns nil) + output-buffer)) - ;; Hide the file/line info to save space. - ;; Replace each with a little widget. - ;; compilation-mode + this loop = slooow. - ;; TODO(adonovan): have guru give us JSON - ;; and we'll do the markup directly. - (let ((buffer-read-only nil) - (p 1)) - (while (not (null p)) - (let ((np (compilation-next-single-property-change p 'compilation-message))) - (if np - (when (equal (line-number-at-pos p) (line-number-at-pos np)) - ;; Using a fixed width greatly improves readability, so - ;; if the filename is longer than 20, show ".../last/17chars.go". - ;; This usually includes the last segment of the package name. - ;; Don't show the line or column number. - (let* ((loc (buffer-substring p np)) ; "/home/foo/go/pkg/file.go:1:2-3:4" - (i (search ":" loc))) - (setq loc (cond - ((null i) "...") - ((>= i 17) (concat "..." (substring loc (- i 17) i))) - (t (substring loc 0 i)))) - ;; np is (typically) the space following ":"; consume it too. - (put-text-property p np 'display (concat loc ":"))) - (goto-char np) - (insert " ") - (incf np))) ; so we don't get stuck (e.g. on a panic stack dump) - (setq p np))) - (message nil)) - - (let ((w (display-buffer (current-buffer)))) - (balance-windows) - (shrink-window-if-larger-than-buffer w) - (set-window-point w (point-min)))))) +(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) + + ;; Hide the file/line info to save space. + ;; Replace each with a little widget. + ;; compilation-mode + this loop = slooow. + ;; TODO(adonovan): have guru give us JSON + ;; and we'll do the markup directly. + (let ((buffer-read-only nil) + (p 1)) + (while (not (null p)) + (let ((np (compilation-next-single-property-change p 'compilation-message))) + (if np + (when (equal (line-number-at-pos p) (line-number-at-pos np)) + ;; Using a fixed width greatly improves readability, so + ;; if the filename is longer than 20, show ".../last/17chars.go". + ;; This usually includes the last segment of the package name. + ;; Don't show the line or column number. + (let* ((loc (buffer-substring p np)) ; "/home/foo/go/pkg/file.go:1:2-3:4" + (i (search ":" loc))) + (setq loc (cond + ((null i) "...") + ((>= i 17) (concat "..." (substring loc (- i 17) i))) + (t (substring loc 0 i)))) + ;; np is (typically) the space following ":"; consume it too. + (put-text-property p np 'display (concat loc ":"))) + (goto-char np) + (insert " ") + (incf np))) ; so we don't get stuck (e.g. on a panic stack dump) + (setq p np))) + (message nil)) + + (let ((w (display-buffer (current-buffer)))) + (shrink-window-if-larger-than-buffer w) + (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."