Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Facilitate externally-processed tab-completion #555

Open
phil-s opened this issue Mar 27, 2019 · 8 comments
Open

Feature request: Facilitate externally-processed tab-completion #555

phil-s opened this issue Mar 27, 2019 · 8 comments

Comments

@phil-s
Copy link

phil-s commented Mar 27, 2019

I'd like to implement tab-completion support in a shell wrapper which doesn't use readline, and I suspect that I could work directly with the array output from Psy\TabCompletion\AutoCompleter::processCallback(), so I'm wondering if you could provide a public way of obtaining those results for a given input string?

The wrapper in question already has access to its own completion framework, but I'll need to supply it with a list of completion options which I would need to obtain from PsySH.

Experimentally, I added method Shell::getTabCompletions() in src/Shell.php:

/**
 * Get completion matches.
 *
 * @return array An array of completion matches for $input
 */
public function getTabCompletions(string $input)
{
    $ac = $this->autoCompleter;
    return $ac->processCallback('', null, ['line_buffer' => $input]);
}

Which allows me to do this:

>>> $__psysh__->getTabCompletions('wt')
=> [
     "wtf?",
     "wtf",
   ]

When the user typed TAB I would essentially then send the (as-yet-unsubmitted) input to PsySH using this new function/command, parse the output, and send the results to the local completion framework.

I feel the only thing I'd need from PsySH itself would be something along the lines of the code above. (I imagine it would work best as a PsySH command, so that simply prefixing the command name to the input would do what I wanted; but I suspect you wouldn't be so keen on this ending up in the list of standard commands, given what a niche use-case this is. I could presumably implement a custom command, however, if the basic functionality was made available?)

@bobthecow
Copy link
Owner

Yeah, this seems totally reasonable. I'd happily accept a pull request adding this.

It's probably not too niche, but you're right, it would be better not to add it to the list of standard commands. But! PsySH currently ships with a parse command, which is disabled by default. Doing the same with a completion command feels fine.

@phil-s
Copy link
Author

phil-s commented Mar 27, 2019

But! PsySH currently ships with a parse command, which is disabled by default

Ah, I see how that works. Perfect.

I'd happily accept a pull request adding this.

Thanks; I've added it to my to-do list.

@phil-s
Copy link
Author

phil-s commented Jul 24, 2020

I started working on this recently, and it's turned into a bit of a rabbit hole (I will raise another issue about that), but the fundamental change works well:

  • Add a new command completions <arg> which prints the list of possibilities for completing the given argument.

  • Make the wrapper issue that command behind the scenes when the user requests completions, consume the output of the command, and feed it to the local completion engine.

My only current question which is directly related to this issue is how should I implement the output of this command?

I concluded that the simplest output was best: A newline-separated list -- one completion per line, unadorned, un-coloured, and otherwise un-formatted. That way, there are no parsing demands placed upon the consumer (which might be anything) beyond reading lines of plain text.

My command's execute() method currently looks like this:

protected function execute(InputInterface $input, OutputInterface $output)
{
    // NOTES: All of the relevant parts of \Psy\Shell are protected or
    // private, so I've kept getTabCompletions() itself as a Shell method.
    $target = $input->getArgument('target');
    $completions = $this->getApplication()->getTabCompletions($target);
    print(\implode("\n", \array_filter($completions)) . "\n");
    return 0;
}

As you can see, I've simply used print(\implode("\n", ...)); to output the list of completions.

Initially I was expecting to follow the lead of other commands I'd looked at by using a Presenter service to handle the output, and had been trying to figure out how to tell that to use the very basic format I wanted; but then I realised print() would do that just fine.

Is that ok?

If not, what's the correct way?

@bobthecow
Copy link
Owner

For the output, newline delimited seems fine. I could see a --format option (or --json or something) to provide alternate output formats, but newlines and no decoration is a good start. And may be all we ever need :)

Yeah, Presenter is more about pretty printing values. Lots of command output doesn't use them at all.

print probably isn't ideal, because it doesn't respect any output destination besides stdout. It might be better to use the $output with no additional formatting:

$output->write($str, false, OutputInterface::OUTPUT_RAW);

@phil-s
Copy link
Author

phil-s commented Jul 24, 2020

That works perfectly, thank you.

I could see a --format option (or --json or something) to provide alternate output formats, but newlines and no decoration is a good start. And may be all we ever need :)

Exactly my thinking as well.

@phil-s
Copy link
Author

phil-s commented Jul 29, 2020

A new issue which affects the completions command is distinguishing trailing whitespace at the end of the command.

This is part of a more general issue with enabling tab-completion to recognise when the user has typed whitespace before typing TAB -- meaning that the previous non-whitespace token is actually complete, and they are requesting completion for an empty string). (In another set of changes, where the code currently strips out all whitespace tokens, I'm retaining a final empty-string token if the final token was whitespace, so that we don't end up generating completions for the preceding token.)

This is fine for readline completion, but my completions command is falling foul of trimming of the command before it's processed, meaning that I can't distinguish between "completions foo" and "completions foo ".

I traced this to \Psy\Shell::runCommand() which does this:

$input = new ShellInput(\str_replace('\\', '\\\\', \rtrim($input, " \t\n\r\0\x0B;")));

This slightly contradicts the documentation for CodeArgument, which suggests that spaces would be retained (although I understand why trailing space has never previously been a concern).

/**
 * An input argument for code.
 *
 * A CodeArgument must be the final argument of the command. Once all options
 * and other arguments are used, any remaining input until the end of the string
 * is considered part of a single CodeArgument, regardless of spaces, quoting,
 * escaping, etc.
 *
 * This means commands can support crazy input like
 *
 *     parse function() { return "wheee\n"; }
 *
 * ... without having to put the code in a quoted string and escape everything.
 */
class CodeArgument extends InputArgument

What do you think about either not trimming (does anything break?), or deferring that trimming until after the arguments have been established, so that CodeArgument can be treated differently?

The ShellInput constructor calls its tokenize method, and a cursory test suggests that trailing whitespace isn't a problem. For instance, for this (var_dumped) input:

string(16) "ls -l $x        "

It establishes the following:

array(3) {
  [0] =>
  array(2) {
    [0] =>
    string(2) "ls"
    [1] =>
    string(16) "ls -l $x        "
  }
  [1] =>
  array(2) {
    [0] =>
    string(2) "-l"
    [1] =>
    string(13) "-l $x        "
  }
  [2] =>
  array(2) {
    [0] =>
    string(2) "$x"
    [1] =>
    string(10) "$x        "
  }
}

@bobthecow
Copy link
Owner

…regardless of spaces…

… was meant to refer to interstitial space, not trailing space :)

it may get weird with newline characters—maybe with different Readline implementations—since there may be one or more of those at the end of every input. if so, trimming trailing newline characters ([\l\r\n]) before creating ShellInput, but leaving all other whitespace probably makes the most sense?

@phil-s
Copy link
Author

phil-s commented Jul 29, 2020

That sounds good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants