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

powershell recipe autocomplete implementation #651

Merged
merged 6 commits into from
Oct 6, 2020
Merged

powershell recipe autocomplete implementation #651

merged 6 commits into from
Oct 6, 2020

Conversation

Insomniak47
Copy link
Contributor

Adds powershell recipe autocomplete as mentioned in: #573

Because it's executing in a scriptblock I have to include the function doing it as a local function. I've also set it up so that if you specify a justfile with --justfile it will use that for the completions.

ex. 
justfile
	recipes:
		- A
		- B
		- C

justfileB:
	recipes:
		- X
		- Y
		- Z

just <tab> would get you the results of just --list in order.
just --justfile ".\justfileB" <tab>
will complete X, Y, Z. You can test it out from outside of the just repo. I've exercised this a bunch locally but not with any exceptional cases. As well the default vscode powershell extension formatter took over and formatted the file. I'm definitely fine with reverting it but you'll likely run into it again.

@casey
Copy link
Owner

casey commented Jun 23, 2020

Thanks for this! Completions are generated in a somewhat weird way. First, completions are generated by Clap, the command-line argument parser that Just uses. Then, they're modified in code to add recipe name completions.

There are tests which check that the completions on disk match the completions that are output by the code, so that they don't get out of sync.

In order to make the tests pass, the modifications to the PowerShell completions should be done here. If you're not comfortable with Rust, I can move the changes for you.

Either way, the whitespace/formatting changes should be reverted, to match what is output by Clap and keep the diff minimal.

@Insomniak47
Copy link
Contributor Author

I'll take a look at getting that into the generator

@casey
Copy link
Owner

casey commented Jun 23, 2020

Definitely let me know if you need any help!

@casey
Copy link
Owner

casey commented Oct 6, 2020

@Insomniak47 I just pushed a diff that moves the changes into src/subcommand.rs, which should make the tests pass. However, I noticed that the code used --list, instead of --summary. --summary prints a space-separated list of recipe names, and doesn't print comments or arguments, so it's more suitable to be processed by scripts.

Since I changed it to --summary, the recipe splitting logic is probably wrong now. Would you mind taking a look? The modification should be made to the replacement strings in POWERSHELL_COMPLETION_REPLACEMENTS in src/subcommand.rs.

Also, I don't have powershell installed locally, so if you could test that it works, that would be much appreciated.

@Insomniak47
Copy link
Contributor Author

Insomniak47 commented Oct 6, 2020

Hey, I had completely forgotten about this. I've taken a look and made a fix to work with your changes. It looks like it simplifies it a little bit. I've tested it locally.

cargo run -- --completions powershell > .\test.ps1
. .\test.ps1
just b<tab> --> just build
just s<tab> --> just sloc<tab> --> just spam
cd .. 
just <tab> --> just --choose<tab> just --chooser --> etc etc
just b<tab> --> just b (no change)
just --justfile .\just\justfile b<tab> --> just [..] build
# Same as if we're pointing at that justfile!
just --justfile .\just\.editorconfig b<tab> --> (no change, as expected)

It seems like it's working relatively well. There might be a few edge cases but this seems to be reasonably well. I've only tested it on windows in pwsh/Powershell and not on pwsh on linux

@casey casey merged commit fbda8dd into casey:master Oct 6, 2020
@casey
Copy link
Owner

casey commented Oct 6, 2020

Excellent! Thank you for the PR, this is a great quality of life improvement.

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

Successfully merging this pull request may close these issues.

2 participants