Fix completion for subcommands of builtins

[krobelus]

This fixes #2705 - broken subcommand completion for builtins, and #5415 - completions for function.

Back when the complete builtin was introduced, builtins like
builtin, exec, command, not, if, while, or and and could
be handled by the completion engine just like any other command.
Much has changed, now we are using a grammar that contains dedicated
AST nodes for those commands.

The completion engine works on plain_statement nodes and has some
special handling for builtins like exec and builtin. However,
__fish_complete_subcommand passes subcommand tokens to the completion
engine that are based on commandline -op, which includes the
builtins.

Example: type "command git " and hit tab. Git completion does not work.
What happens is that the completion scripts assume that the first
token on the commandline ("command") is the git executable, and the next
token ("git") is the subcommand. Since "command" is ignored, the git completion is called
with garbage input.

This change uses the information provided by the AST to make completion treat the builtins like any other command. In above example the completions for "command" will be called first.
They will remove the "command" token and then call into "git" completions.

Previous approach

6e733d9

This changes commandline -p to only return the plain_statement,
without any decorators to make examples like the above one work. If
anyone needs them to return decorators, then it may be preferable to
maintain compatibility instead. However, I think it makes sense to not
print decorators, consider for example:

false; and builtin A || B     # B is not executed!
       ^

commandline -p would print and builtin A, but in fish's grammar,
and applies to the entire expression builtin A || B.
With this change, it only prints A.

If needed I can make it so it does print the simple decorators
"exec", "command", "builtin" and "not".

Note that we keep completions like share/completions/builtin.fish
around, even though they are handled in C++, because a commandline
like "builtin " is parsed as plain statement
(while "builtin a" is recognised as decorated statement).

---

TODOs:

• Tests have been added for regressions fixed
• User-visible changes noted in CHANGELOG.md

[faho]

I would expect this to be what you want for completions (because the decorator is irrelevant there) but not other uses of commandline -p like bindings.

I think a better solution would be to fake the commandline in completions, like we already do for complete -w. This is what the scripted completions tried to achieve, but the C++ was never cooperative here and I've not found the correct place to change.

[krobelus]

OK I think it's fair to maintain that behavior of commandline -p, so I think I will change the behavior of completion to match that. The machinery to do so is already here - we have completions scripts for all prefixes like and, I just need to make the completion engine not skip over those nodes. It should definitely be doable, by using parse_util_process_extent and then handling the additional node types in completer_t.

[krobelus]

I have pushed a quite different approach and changed the description.
This one uses the information in the AST to extract builtins and complete them like normal commands.

I believe this works fine.

(I was thinking that instead of the AST we could just use the tokenizer here but then we'd probably have to add some hacks that reimplement parsing. for example to handle for <TAB>)

[krobelus]

This also makes completions for command and exec use external find, which is simpler to implement, but I could work around that if needed.

[zanchey]

find is currently an optional ("preferred") dependency, for what that's worth.

[ridiculousfish]

So the idea here is to make completions forget that command, not, etc. are keywords and just pretend everything is a command. This is how fish used to work and in hindsight was better - conceptually simpler. I like this direction.

One downside of moving logic from C++ to script is that we lose autosuggestion support. For example I expect that and will now autosuggest arbitrary file paths instead of only executables. However this is probably acceptable and there's other ways to fix this.

[ridiculousfish]

I wonder if this is more complicated than necessary. Is all you hope to do collect the string-type nodes? You can do that with a stack, traversing without concern for the type:

std::vector<const parse_node_t *> result;
std::vector<const parse_node_t *> to_visit = {start};
while (! to_visit.empty()) {
           // Append to result if type is parse_token_type_string
           // Pop it off and push all of its children on
 } 

[ridiculousfish]

You can use tnode_t::descendants() for this:

auto strings = statement.descendants<grammar::tok_string>();

This gives you all the string nodes under a given node (recursively).

[krobelus]

Yes I didn't realize the same can be done with one tnode_t::descendants().
But what I am doing now is even simpler, I'm just using the tokenizer because with this change the completion does not need to use the AST, it just needs the string tokens in a process. So this seems good enough for now.

[krobelus]

One downside of moving logic from C++ to script is that we lose autosuggestion support. For example I expect that and will now autosuggest arbitrary file paths instead of only executables. However this is probably acceptable and there's other ways to fix this.

Yep, I added a small workaround for autosuggestion of builtins in completer_t::perform()