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

[Dmitry Gutov <dgutov@yandex.ru>]

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

> Still better than failing randomly.  Especially if you hint that the
> match was approximate (much like the way diff tell you about "fuzz").

The problem is it will "break" correct navigations sometimes.

> Anyway, those totally hypothetic future users of the class could well
> and polish the default behaviour if they decided it's worth it.

Suppose we use a new class. If the hint argument is mandatory, then 
there should be no problem: the backend explicitly asked for this behavior.

Similarly if xref-file-location grows a new optional field which 
defaults to nil.

>> It can certainly become outdated if the user has added or deleted a
>> couple of lines there.
> 
> But then, in that case, isn't it much better to find an actual match
> (and perhaps warn) than to land the user in nowhereland?

Yes. And in Grep results, it might be beneficial to use the new 
behavior. The code creating the locations will pass line's contents as 
the "hint" (and maybe we should make the hint a regexp, to be able to 
use the line-beginning and line-ending anchors). In that use case, 
though, it would be better to error out if the hint is not found.

I'm somewhat worried about what will happen if a hint misdetects a match 
anyway, and we're in many-automatic-edits land (such as 
xref-query-replace-in-results), but on the balance, it'll probably be 
better than the current behavior anyway.

Except for, err... lines with several matches on them. Not sure what to 
do about those. A special class, probably.

> Because I want something with some default behaviour.  Behaviour that is
> admittely half-decent, but nevertheless better than current behaviour.
> But since you showed that xref-location-marker is used by your grep
> substitute, I don't want to cause any regressions in its existing,
> broken behaviour, whatever that may be exactly.
> 
>> It might be shorter implementation-wise, too.
> 
> This is how I imagine the implementation:
> 
>     (defclass xref-hinted-location (xref-file-location)
>       ((hint :initarg :hint))))
>      
>     (cl-defmethod xref-location-marker :around ((l xref-hinted-location))
>        <...code to search around...>)
> 
> Compared to the "add optional field" approach, there would be about 1 extra line,
> the defclass one.

If we use an optional field, we could call ignore-errors around 
forward-char if that field is present (your proposal number 1).