Re: [Emacs-diffs] master d7df36e: Rewrite elisp--xref-find-definitions to handle many more cases; add tests.

[Stephen Leake]

Dmitry Gutov <address@hidden> writes:

> On 08/12/2015 02:30 AM, Stephen Leake wrote:
>
>>> How come you saw fit to undo the tweaks that I've added over time?
>>
>> Because they got in the way of some of my uses of xref-find-definitions.
>
> At least a question would've been nice.
>
>> In general, rather than using similar heuristics in the low level code,
>> we should be adding hints from the source being searched, and/or the
>> user.
>
> In general, elisp-xref doesn't open the source files. When used with
> xref-find-apropos, that was rather slow.
>
>> For example, one heuristic returned only the function when there are
>> both function and feature with the same name. But there are times when I
>> want to see both, or one or the other.
>
> If the feature and the function have the same name, idiomatically,
> they will always be in the same file. 

I was going to say "not guarranteed", but that is a style we strongly
encourage, so I think it is reasonable to rely on it.

> Why wouldn't you want to jump to
> the function? 

> I see you're also jumping to the (provide 'xxx) forms. What's the use
> in that?

I think that's because I was trying to find the closest equivalent to
what Ada mode does. In Ada, every file has clear syntax indicating "the
start of the file code", and M-. on "with foo;" jumps to that point in
foo.ads.

(provide 'foo) is the closest thing in elisp syntax. We could jump
to the ;;; Code: comment.

For a better reason, sometimes it does matter what happens in the file
after (provide 'foo), so that's sometimes a useful place to go.

> If it's not right, M-< is not too far away (or M->, as per below).

and M-. to jump to the function is also not too far away.

But even better is the source code info discussed below; if that works,
this point is moot.

>> So if I'm searching for the
>> identifier at point, in code like this:
>>
>>          (dvc-log-edit ...) ;; point on '-l'; show defun
>>          (require 'dvc-log-edit) ;; point on '-l'; show feature
>>
>> In both cases, we can easily tell from the source near point what the
>> user is searching for.
>
> Right, it's a good default behavior. But then we'll have to decide how
> the user could indicate whether elisp-xref should *not* look at the
> context. Currently, C-u only forces the prompt to appear, but the
> default value will still use the text properties from the context (if
> xref-default-identifier-at-point includes them).

C-u should strip the text properties.

>> Yes. But I thought the heuristic the previous code used was: "if there
>> is both a variable and a function by the same name, _assume_ they are
>> located in the same place, so only return the function". That assumption
>> is broken for some of my code, and I assume in many others as well.
>>
>> However, you point out later that you used (memq sym minor-mode-list) to
>> determine if this is from define-minor-mode.
>>
>> I didn't realize why that was there when reading the code.
>>
>> So I'll put that back, with a comment this time :).
>
> There was a comment: "Don't show minor modes twice". I thought it
> explained the purpose of (not (and (fboundp sym) (memq sym
> minor-mode-list))) clearly enough.

Not for me, obviously.

I will ask first about code I don't understand in the future.

Did you find my comments clear enough?

-- 
-- Stephe