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

[joaotavora@gmail.com (João Távora)]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: joaotavora@gmail.com (João Távora)
>> Cc: 32034@debbugs.gnu.org,  dgutov@yandex.ru
>> Date: Mon, 02 Jul 2018 16:28:53 +0100
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >>            (beginning-of-line line)
>> >> -          (forward-char column)
>> >> +          (ignore-errors (forward-char column))
>> >> +          (unless (and (= (1+ (current-line)) line)
>> >> +                       (= (current-column) column))
>> >> +            (message "Intended xref location was line=%d, column=%d"
>> >> +                     line column))
>> >>            (point-marker))))))
>> >
>> > What will the last hunk do when a file changed, but the identifier was
>> > still found?
>> 
>> The function is completely oblivious to the identifier being found or
>> not.  So it would do what it does now: jump to the wrong location
>
> And display the message, right?

Not necessarily, if that line and column still exists.

> If so, the message would be an annoyance if displayed unconditionally,
> because at least the etags back-end can cope with these calamities,
> and users rely on that (it lets one re-generate TAGS only once in a
> blue moon).

That's what I wrote in the original message.  I added it under the
assumption that it would be less of an annoyance than silently jumping
to the wrong spot int he file.  The feature you want to add to the
relevant xref-location-marker method makes a lot of sense, but it's a
new feature nevertheless.

> Or maybe I misunderstand the context in which this code runs, in
> which case ignore me.

No, I think your understanding is fine.  The current code is really
quite dumb.  The reason you don't come across it often is that the elisp
and etags backend use their own "location" class, xref-etags-location
and xref-elisp-location.  But eglot uses the xref-file-location class
bundled with xref (it could also use its own, but then what's the point
of having a built-in class for this?  perhaps none, in this case I move
to delete it)

>> > AFAIR, the etags back-end is capable of doing that
>> > (because it searches the file in the vicinity of the line/column if
>> > not found at the exact location), and it's a valuable feature, for
>> > obvious reasons.
>> 
>> IIUC, that's what I'm proposing in my point 3: add a "hint" field to
>> xref-file-location and default that hint to the identifier being looked
>> for.  If etags has code for that already, then great, we could try to
>> share it
>
> I don't think you can share that code, because it relies on the
> contents of the TAGS file, where the 'etags' utility records not only
> the position of the identifier, but also a string to search for it.

I meant the code that searches for the string then, if it isn't too
trivial.

When you can, please comment on the possibility of fixing this in
emacs-26 or master.

João