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

shell tab completions #615

Closed
glennj opened this issue Jun 10, 2022 · 15 comments · Fixed by #629
Closed

shell tab completions #615

glennj opened this issue Jun 10, 2022 · 15 comments · Fixed by #629
Labels
kind: feature User-facing enhancement

Comments

@glennj
Copy link
Contributor

glennj commented Jun 10, 2022

I've been building a track, so I've been using configlet a lot. With the plethora of subcommands and options, I figured tab completion would be handy. My shell is fish, so that's where I started.

But then I thought "how to install the completions"? How about this: add an option to fetch-configlet to download a shell-specific completions file, and emit instructions on how to source it into the user's shell.

Example:

$ bin/fetch-configlet -c fish
To enable completions: source bin/configlet-completions.fish
$ bin/fetch-configlet -c bash
To enable completions: . bin/configlet-completions.bash
$ ls -1 bin
configlet
configlet-completions.bash
configlet-completions.fish
fetch-configlet
...

Do you think this would be useful?

@ErikSchierboom
Copy link
Member

I agree that shell completion would be handy, yes. What the actual implementation should look, I personally have no idea. But I like the idea.

@ee7
Copy link
Member

ee7 commented Jun 17, 2022

I'd like tab completions too, and the idea of using fetch-configlet to install them sounds good to me.

When running a plain fetch-configlet, we could also print instructions for installing the tab completions.

I'd already created issue #364 for this feature. But this issue goes into more depth, and I believe it's still not possible to move comments to another issue. So let's just use this one. No problem.

@ee7 ee7 added the kind: feature User-facing enhancement label Jun 17, 2022
@glennj
Copy link
Contributor Author

glennj commented Jun 17, 2022

Do you want to discuss here, or in the Slack #configlet channel?

@glennj
Copy link
Contributor Author

glennj commented Jun 23, 2022

I propose adding a completions/ directory to the repo, and the shell specific completion files would reside therein.

This completions directory would be added to the artifact zip/tgz file, so they are automatically downloaded and unarchived into the repo's bin/ directory:

$ bin/fetch-configlet
$ tree bin
bin
├── completions
│   ├── configlet.bash
│   ├── configlet.fish
│   └── configlet.zsh
├── configlet
├── fetch-configlet
├── fetch-configlet.ps1
...

The fetch-configlet script can add:

if command -v getent; then
    user_shell=$(getent passwd "$USER" | cut -d: -f6)
elif command -f finger; then
    user_shell=$(finger "$USER" | sed -nE '/.*Shell: / { s///; s/ .*//; p; }')
elif [[ -f /etc/passwd ]]; then
    user_shell=$(awk -F: -v u="$USER" '$1 == u {print $6}' /etc/passwd)
fi

case "$user_shell" in
    */fish) echo "Enable completions: source bin/completions/configlet.fish" ;;
    */bash) echo "Enable completions: . bin/completions/configlet.bash" ;;
    */zsh) echo "Enable completions: . bin/completions/configlet.zsh" ;;
esac

This proposal removes the need for a command-line option on the fetch-configlet script.

@glennj
Copy link
Contributor Author

glennj commented Jun 23, 2022

@glennj
Copy link
Contributor Author

glennj commented Jun 23, 2022

I don't know PowerShell: does it have a completion mechanism?

@ErikSchierboom
Copy link
Member

@glennj
Copy link
Contributor Author

glennj commented Jun 23, 2022

@kotp you posted in the Slack channel...

@glennj
Copy link
Contributor Author

glennj commented Jul 8, 2022

@ErikSchierboom since the fetch-configlet script is distributed to all tracks, would you want

  • one PR for adding the completion files to the repo and updating the build script
  • a separate PR for changes to fetch-configlet

?

@ErikSchierboom
Copy link
Member

@glennj Yes, I think separate changes would make sense.

@ee7
Copy link
Member

ee7 commented Jul 18, 2022

Sorry, I didn't have the bandwidth to think about this earlier.

@glennj @ErikSchierboom What do we think about using the approach of gh completion?

That is: embed the completions in the configlet binary, rather than writing extra files at extraction time. I think I'd prefer to avoid writing a bin/completions directory everywhere that a user runs fetch-configlet, especially since the completions will change over time.

For people that work on multiple tracks, it seems nicer to avoid the duplication, and potentially make it harder for the completions to go out of sync with the configlet binary. That said, I'm not sure if "a user sources completions in one track directory, and then runs a newer configlet in another track directory later" is more likely than "a user sources completions generated by an older configlet binary in one track directory, and run a newer configlet in another track directory later". And if people are adding configlet to their path, they should update the completions each time they update configlet either way.

We could still detect the user's shell when running fetch-configlet, but instead advise that the user run e.g.

configlet completion -s fish > ~/.config/fish/completions/configlet.fish

If we preferred that way, it doesn't block #629. I imagine I'll be able to merge that PR as-is or nearly as-is - I just haven't finished reviewing it yet (sorry).

So I'm just asking if we'd like a follow-up PR that reads and embeds the completion files at compile time, and makes configlet completion -s foo write the corresponding completion file to stdout. I'd still prefer the completion files to be separate files in this repo (so that shellcheck still runs on them during CI), just not present as separate files in the release asset.

@ErikSchierboom
Copy link
Member

I quite like that. Seems like an elegant solution.

@glennj
Copy link
Contributor Author

glennj commented Jul 19, 2022

Agreed. I'm curious how that'll be implemented.

@ee7 ee7 moved this to In Progress in Configlet roadmap Jul 23, 2022
ee7 added a commit to glennj/exercism-configlet that referenced this issue Aug 4, 2022
Instead, it looks like we'll bake the completions into the configlet
binary via a new subcommand [1].

[1] exercism#615 (comment)
@ee7 ee7 closed this as completed in #629 Aug 4, 2022
ee7 added a commit that referenced this issue Aug 4, 2022
For later:
- Add completions for zsh and PowerShell.
- Make `fetch-configlet` detect the user's shell and explain how to
  install completions.
- Make `configlet completion -s <shell>` print the corresponding
  completion script to stdout.

This commit also refactors `create-artifact` slightly.

Closes: #615

Co-authored-by: ee7 <45465154+ee7@users.noreply.github.com>
Repository owner moved this from In Progress to Done in Configlet roadmap Aug 4, 2022
@ee7
Copy link
Member

ee7 commented Aug 4, 2022

Agreed. I'm curious how that'll be implemented.

For now, I'll propose the simple approach in #631. Pretty much just staticRead and boilerplate.

The approach with one more level of abstraction is #639. That'd also be mostly staticRead + boilerplate, but adding a Nim implemention of complete behind configlet __complete.

@ee7
Copy link
Member

ee7 commented Aug 6, 2022

And to be more fancy, it's easy to embed compressed versions of the completion scripts. See #652.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: feature User-facing enhancement
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants