bug#53644: 29.0.50; xref-search-program breaks if programm not installed on a remote host

[Philip Kaludercic]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 14.02.2022 19:32, Philip Kaludercic wrote:
>> Dmitry Gutov <dgutov@yandex.ru> writes:
>> 
>>> On 14.02.2022 15:57, Philip Kaludercic wrote:
>>>>> And use a simpler test: (only when the host is remote) write some text
>>>>> to a file in the temp dir, then search through it. Only doing it once,
>>>>> of course, when the connection-local value is initialized.
>>>> I am afraid I don't understand what you mean here, specifically "some
>>>> text".
>>>>
>>>
>>> Well, we need some file to search and some knowledge about its
>>> contents in advance (so the search can succeed).
>> I guess this is what confuses me, what about the contents is
>> relevant to
>> the query?  `xref-matches-in-files' is already passed a list of files
>> that can be concatenated into the input for xargs.  The current version
>> already works and is reasonably fast, so I don't recognise the
>> improvement.
>
> Sorry, I guess I wasn't clear enough.
>
> When I said "extract the detection logic", I meant implement something
> similar to 'grep-compute-defaults' where there is a bunch of "probing"
> code which detects which commands work on the given system (and which
> arguments, etc). But a shorter function, of course, since it would
> only need to choose between two alternatives -- and return it.
>
> And it seems to be it would be simpler (conceptually) if the said
> function didn't itself depend on xref-matches-in-files (the
> implementation or the return type). Though it's certainly possible to
> use it as well.
>
> ...so that function (let's call it xref--choose-search-program,
> perhaps) would write to a file in the temporary directory on the
> remote system, and then search in it using the configured search
> program, and then fall back to the default one if the first one fails.
>
> WDYT?

The current design (subsuming probing into executing by speculatively
starting a process) was intentional, mainly to avoid the overhead of
executable-find calls.  I guess one could argue that this is only
necessary once, and after that can be stored as a connection local
variable.  Setting this aside, I see little difference to the
xref--choose-search-program approach, unless there is some other context
that might want to use this function.

>>> Since we don't know anything about the remote system, we probably have
>>> to create the file ourselves. Put something like "aaa\nbbb\nccc" in
>>> it, and then search for "bbb".
>> My apologies, I feel stupid for not understanding, but what would
>> aaa,
>> bbb and ccc be?
>
> Those are characters. "aaa\nbbb\nccc" would be the temporary file's
> contents, verbatim.
>
> To clarify, I think your code quality is just fine, but the way the
> main function gets two responsibilities intertwined (both program
> detection and the actual search) seems a bit too much for me,
> clarity-wise.

I am biased, but I do prefer the current state of the patch.  If you
think it would help, I could comment it out more?

-- 
        Philip Kaludercic