Subject: bug#32034: 26.1; [PACTH] better xref-location-marker for imperfect file locations

From: João Távora <[email-masked]>
Date: Tue, 3 Jul 2018 16:43:26 +0100

On Tue, Jul 3, 2018, 16:07 Dmitry Gutov <[email-masked]> wrote:

> On 7/3/18 5:50 PM, João Távora wrote:
>
> > Right, this should go into xref-goto-xref (or whatever it is called) or
> > xref-find-definitions.  Or scratched, if it's too much work, because
> > it's not terribly useful.
>
> I'm sure it will be useful in some situations, I'm just not sure about
> the odds.
>
> > In that special case we will do no worse than the current version,
> > wouldn't we?
>
> We would.
>
> Say, we're looking for clojure.core/cons, and the marker leads us to
> "(defn cons" (the example is largely made up). The code looks for
> clojure.core/cons, doesn't find it, and signals an error (will it?).
>

No, no error. Stays put. So no worse.

>
> Or worse, searches around and finds a verbatim reference to
> clojure.core/cons in a docstring somewhere in that file, and returns
> *that* location.
>

We could avoid that (particulary far fetched) case (and the string case) by
checking syntax.

> > At this point, I'm thinking of dismissing the whole thing, and voting to
> > deprecate xref-file-location entirely. Nobody uses it and Eglot will
> > probably use something else before this issue is solved.
>
> Err, it is used: in xref--collect-matches-1. And through it, in
> xref-collect-matches and xref-collect-references.
>

Oh, I must have misgrepped then. Excuse my ignorance, but are these
grep-like tools? If so, they shouldn't ever suffer from the "outdated"
malaise, right?

>
> > It's a shame
> > we can't share code, but if we can't give a default class any kind of
> > useful behavior, we might as well not have this class in the first place.
>
> We could. As long as we don't default to the identifier, and the backend
> has to explicitly provide the hint, we could be fine.
>

I guess a new xref-hinted-location subclass would be the way to go, if not
too much work.  But still think we should do something by default that will
do just as bad on the worst case. And if we use a new subclass, we're
guaranteed that.