[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Generalizing find-definition
From: |
Stefan Monnier |
Subject: |
Re: Generalizing find-definition |
Date: |
Tue, 18 Nov 2014 11:31:29 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux) |
> - find-definition does not store locations in the tag ring yet
> (ran out of time and wanted to post the initial patch for feedback)
That needs to be written before it can be installed.
> - The whole "find uses" interface
That can be written later.
> - etags.el integration
That needs to be written before it can be installed.
> - emacs-lisp-mode integration
That can be written later. It should probably obsolete
find-definition-noselect.
> This also does define a minor mode instead of changing the global key
> bindings. I think in the final version it should replace the key
> binding definitions done in etags.el. Is this correct?
Yes, that's correct.
> Do I need to hook it up elsewhere in the build process?
I don't think so.
Thanks, it looks pretty good. See further comments below.
Stefan
> +(defcustom find-definition-marker-ring-length 16
> + "Length of marker rings `find-definition-marker-ring'."
> + :group 'find-definition
> + :type 'integer)
The :group is redundant.
> +(defvar find-definition-function nil
> + "The function `find-definition' calls to find the definition.
> +
> +Will be called with no arguments with point at the location of
> +the thing to find the definition for. It should return a list
> +with each element being a list of one to three elements. The
> +first element should be the file name, the second the
> +line (defaulting to 1) and the third the column (defaulting to
> +0).")
Please use two spaces between sentences. Also we usually prefer to
describe things using "patterns", as in:
Will be called with no arguments with point at the location of
the thing to find the definition for. It should return a list
of elements of the form (FILE LINE COL) where LINE and COL can be
omitted.")
> +(defvar find-definition-identifier-function nil
I suggest you collapse those two function variables into one: if called
with a nil value (or without argument), then just find the definitions
of "thing at point" and if called with a string, then find the
definitions of that identifier.
> + (let ((map (make-sparse-keymap)))
> + (define-key map (kbd "M-.") 'find-definition)
> + (define-key map (kbd "C-x 4 .") 'find-definition-other-window)
> + (define-key map (kbd "C-x 5 .") 'find-definition-other-frame)
> + ;; (define-key map (kbd "M-_") 'find-definition-uses)
> + (define-key map (kbd "M-,") 'find-definition-goto-last-position)
We should probably keep a M-* binding for now.
> +;;;###autoload
> +(define-minor-mode find-definition-mode
> + "Minor mode to provide some key bindings to find definitions.
> +\\{find-definition-mode-map}"
> + :keymap 'find-definition-mode)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I don't think this really does what you want. Better just completely
remove it [ Not that it matters since find-definition-mode should
simply disappear. ].
> +;;;###autoload
> +(defun find-definition (&optional ask)
> + "Go to the definition of the thing at point.
> +If the definition can not be found, or with a prefix argument,
> +prompt for a symbol to use."
> + (interactive "P")
> + (switch-to-buffer (find-definition--noselect ask)))
Rather than ask from the body of the function, I think the prompting
would be better done in the interactive form. I.e.
(defun find-definition (&optional identifier)
"Go to the definition of the thing at point.
If the definition can not be found, or with a prefix argument,
prompt for a symbol to use."
(interactive
(if current-prefix-arg (list (find-definition--read-identifier))))
(switch-to-buffer (find-definition--noselect identifier)))
This of course also suggests we should not do the "if the definition can
not be found, prompt for a symbol to use".
> + ;; Exactly one definition
Please punctuate your comments.
> + (with-current-buffer outbuf
> + (erase-buffer)
> + (setq default-directory dir)
> + (compilation-mode)
> + (dolist (location locations)
> + (let* ((filename (elt location 0))
> + (line (or (elt location 1)
> + 1))
> + (col (or (elt location 2)
> + 0))
> + (buffer (find-buffer-visiting filename))
> + (line-string
> + (when buffer
> + (with-current-buffer buffer
> + (save-excursion
> + (save-restriction
> + (widen)
> + (goto-char (point-min))
> + (forward-line (- line 1))
> + (buffer-substring (line-beginning-position)
> + (line-end-position))))))))
> + (insert (format "%s:%s:%s:%s\n"
> + filename line col
> + (or line-string
> + "")))))
We can probably use a new mode that derives from compilation-mode.
That will let us use a more efficient and reliable regexp to match the
entries we put in there.
Also, I think that when the LINE&COL are absent, we should not move to
LINE=1, but instead just leave point alone in that buffer (and in the
list, we should not explicitly say "FILE:1:0" but just "FILE:" or
something like that).
One remaining issue is with the filenames themselves: the
find-definition-function may very well (and maybe should usually) return
absolute file names, so we should "prettify" them (make them relative)
before displaying them in the *Definitions* buffer.
Stefan
- Re: Generalizing find-definition, (continued)
- Re: Generalizing find-definition, Jorgen Schaefer, 2014/11/17
- Re: Generalizing find-definition, Helmut Eller, 2014/11/18
- Re: Generalizing find-definition, Stefan Monnier, 2014/11/19
- Re: Generalizing find-definition, Ivan Shmakov, 2014/11/19
- Re: Generalizing find-definition, Stefan Monnier, 2014/11/19
- Re: Generalizing find-definition, Stephen Leake, 2014/11/19
- Re: Generalizing find-definition, Stefan Monnier, 2014/11/19
- Re: Generalizing find-definition,
Stefan Monnier <=
- Re: Generalizing find-definition, Stephen Leake, 2014/11/19
- Re: Generalizing find-definition, Stefan Monnier, 2014/11/19
- Re: Generalizing find-definition, Jorgen Schaefer, 2014/11/20
- Re: Generalizing find-definition, Helmut Eller, 2014/11/20
- Re: Generalizing find-definition, Jorgen Schaefer, 2014/11/20
- Re: Generalizing find-definition, Helmut Eller, 2014/11/20
- Re: Generalizing find-definition, Stefan Monnier, 2014/11/20
- Re: Generalizing find-definition, Jorgen Schaefer, 2014/11/20
- Re: Generalizing find-definition, Stefan Monnier, 2014/11/20
- Re: Generalizing find-definition, martin rudalics, 2014/11/21