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

[Dmitry Gutov <dgutov@yandex.ru>]

Hi guys,

I've read up on the discussion.

On 7/2/18 4:46 PM, João Távora wrote:

> 1. xref-location-marker should check for `file-readable-p' before trying
> to open the file, and if that fails issues an error ("File %s can't be
> found.").

I'm fine with this, naturally.

> 2. if the file is found, xref-location-marker should detect if the
> location is indeed available in the file, and if it isn't, issue a
> message.  In that case it should return a marker to the nearest possible
> location.

When xref-location-marker is inaccurate, it may lead to problems, like 
xref-query-replace-in-results sometimes performing replacements at the 
"auto-corrected", wrong positions.

Maybe we can add a laxer version of this function that is only used when 
we know the user will be looking at the result directly (e.g. from 
xref-find-definitions, but not from xref-query-replace-in-results). I'm 
on the fence about xref-fined-references regarding this, because it also 
supports automatic replacement.

> 3. Number 2. could turn out to be brittle and annoying if we have
> changed the file in the meantime (but probably not more so than jumping
> to a wrong location).  So we could have a "hint" field in
> xref-file-location (or a xref-hinted-file-location) that helps in
> looking around the landing point for, say, a regexp, and puts point
> there.  Historically, this technique is successfully used in SLIME.  We
> could also reasonably default that field to the identifier being looked
> for.

I'm not sure this is a good idea. Certainly not the "defaulting to the 
identifier" bit. Because the identifier could e.g. look like 
namespace-name/symbol-name, where only "symbol-name" appears verbatim in 
the definition. I don't have much experience with LSP, but I imagine 
this could happen there, too (unless it only supports navigation to 
unqualified identifiers).

Now, if hint is optional (and disabled by default), and extracting the 
relevant code from Etags is natural, I say go for it.

But overall, I think individual backends that want "smarter" behavior 
should create their own "location" class, like Elisp does.