-
Notifications
You must be signed in to change notification settings - Fork 311
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
Comments
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 |
Ah, I see how that works. Perfect.
Thanks; I've added it to my to-do list. |
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:
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
As you can see, I've simply used 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? |
For the output, newline delimited seems fine. I could see a Yeah, Presenter is more about pretty printing values. Lots of command output doesn't use them at all.
|
That works perfectly, thank you.
Exactly my thinking as well. |
A new issue which affects the 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 I traced this to $input = new ShellInput(\str_replace('\\', '\\\\', \rtrim($input, " \t\n\r\0\x0B;"))); This slightly contradicts the documentation for
What do you think about either not trimming (does anything break?), or deferring that trimming until after the arguments have been established, so that The
It establishes the following:
|
… 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 ( |
That sounds good to me. |
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 fromPsy\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()
insrc/Shell.php
:Which allows me to do this:
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?)
The text was updated successfully, but these errors were encountered: