Shell completion

[chairmank]

I would like a command that writes a completion script for any supported $SHELL to stdout.

For example, to enable completion for klog commands in Bash, one would source the output of klog completion bash:

echo '. <(klog completion bash)' >>~/.bashrc

If there is interest, I can implement. I think that Cobra automatically generates shell completion.

[jotaen]

I had briefly looked into that topic a while ago, but never got around to it. In general, I think that would be a nice feature to have. If you’d be willing to set something up, that would be cool 👍

Only caveat is that the klog CLI currently uses kong as CLI framework, and for now I wouldn’t want to switch away from that. kong doesn’t support completions out of the box, but there is a separate library called kongplete which provides support, and which seems like the “recommended” way to go with kong.

[chairmank]

I will try implementing this feature with kongplete and share my progress.

[chairmank]

I have a branch with rudimentary implementation of this feature:

https://github.com/chairmank/klog/tree/shell-completion

There is no test coverage :( I have manually tested a few commands, and only with bash shell.

There are no completions for filter options (e.g. --period, --this-***). I would like feedback on my approach before I try to implement these trickier completion generators.

[jotaen]

🎉 Awesome, thanks for figuring this out! I’ve tried this on both bash and zsh (macOS), and it seems to work well:

• On bash (3.2), it worked out of the box.
• On zsh (5.8), there is one caveat, which is that autoload -Uz compinit && compinit needs to be enabled via .zshrc. I suppose, though, that zsh users know that, and it can additionally be mentioned on the docs website.
• kongplete doesn’t support Windows (Powershell). That’s alright, though, and maybe someday a keen Windows user could submit a patch for that to kongplete. (Cobra supports Powershell, btw., so maybe the code can be taken over from there.)

The code structure you suggested seems to fit in very nicely. It makes sense to me how the predictors are separated into the cli/lib/ folder. (diff link for convenience.)

Regarding tests: my feeling is that as long as the predictors are as straightforward as the PredictBookmarks one, no unit tests would be needed.

To me, the current functionality would already be useful enough for it to be merged in. Feel free to add more completion predictors as you like, and open a PR at any point.

[chairmank]

Thanks for reviewing my change.

kongplete doesn’t support Windows (Powershell). That’s alright, though, and maybe someday a keen Windows user could submit a patch for that to kongplete. (Cobra supports Powershell, btw., so maybe the code can be taken over from there.)

kongplete uses complete, which is specifically for bash completion by design. kongplete supports zsh and fish with thin wrappers around bash completion, so it is unable to leverage other shell-specific completion capabilities. For example, in fish shell, available commands are displayed with helpful descriptions, but kongplete can not implement this behavior because bash does not have an equivalent capability.

There exists a standalone wrapper for bash completions in PowerShell. I have not investigated whether this wrapper could be used by kongplete to add support for PowerShell. Even if it is feasible, it would lack the richness of "native" PowerShell completions.

Cobra has a different design that involves implementing a function for each shell to leverage shell-specific completion capabilities. I think that using Cobra instead of kong would provide superior ergonomics. Of course, I understand that this is a major change and you are not interested in doing it now.

Anyway, this implementation with kongplete adds useful functionality. I will continue working on this branch and open a pull request.

[chairmank]

I noticed a difficulty with the --this-*** and --last-*** filter flags. These are dummy flags that are included for brevity in the help output, while the flags for the actual underlying filters (--this-week, --this-month, etc.) are hidden. Unfortunately, kongplete does not automatically generate completions for the hidden flags, and instead only generates completions for the dummy flags.

I have not determined whether I can override this behavior in kongplete. If it not possible to override, what do you think about removing the dummy filter flags and simply exposing the actual filter flags?

[chairmank]

Another problem that I noticed is that the complete.PredictFiles function does not expand the user's home directory (~/some/path, ~user/some/path) or parent directories (../../some/path). This is an unacceptable limitation. I can try writing my own.

[jotaen]

kongplete uses complete, which is specifically for bash completion by design. kongplete supports zsh and fish with thin wrappers around bash completion, so it is unable to leverage other shell-specific completion capabilities.

Ah okay, thanks for all that background info, I wasn’t aware of that.

I’m generally not married to kong in any way, I was only reluctant to invest all the work to migrate to Cobra, when kong is working well overall currently. (Well, except for the limited completion capabilities, that is.)

If it not possible to override, what do you think about removing the dummy filter flags and simply exposing the actual filter flags?

What I like about the dummy flags is how they make the help output concise, and also how they resemble the logical pattern of how the flags work.

However, I’ve tried it out again on your branch, and seeing how it completes to --last-*** doesn’t feel acceptable. If it’s not possible (or too complicated) to override or workaround, then I’d think it’s viable to make the help output explicit. I’d suggest then to only expose the dash’ed flags (--last-week) and keep the non-dash’ed ones hidden (--lastweek), since these were only meant as an implicit fallback for convenience.

Filter (shortcuts)
  --today        Records at today’s date
  --yesterday    Records at yesterday’s date
  --tomorrow     Records at tomorrow’s date
  --this-***     Records of the current week/quarter/month/year (e.g. --this-year)
  --last-***     Records of the previous week/quarter/month/year (e.g. --last-month)

vs.

Filter (shortcuts)
  --today           Records at today’s date
  --yesterday       Records at yesterday’s date
  --tomorrow        Records at tomorrow’s date
  --this-week
  --last-week
  --this-month
  --last-month
  --this-quarter
  --last-quarter
  --this-year
  --last-year

Another problem that I noticed is that the complete.PredictFiles function does not expand the user's home directory (~/some/path, ~user/some/path) or parent directories (../../some/path). This is an unacceptable limitation. I can try writing my own.

Ah, bummer. There is this stale PR to add support for ~ in “complete”. I then also noticed that it cannot handle ../.

I agree with you that flawless file path completion would be critical.

Would it possible to fallback to the shell’s default completion for file paths? (E.g. via compgen -o default. Unfortunately, the “complete” library doesn’t seem to support such mechanism natively, though.) My thinking was, why jump through the loops to re-build something that the shell itself would handle perfectly already. Since processing file paths is no core business of klog, I’d be a bit worried that a custom solution could turn into a source of bugs and follow-up issues down the line.

[chairmank]

Would it possible to fallback to the shell’s default completion for file paths? (E.g. via compgen -o default. Unfortunately, the “complete” library posener/complete#131, though.) My thinking was, why jump through the loops to re-build something that the shell itself would handle perfectly already. Since processing file paths is no core business of klog, I’d be a bit worried that a custom solution could turn into a source of bugs and follow-up issues down the line.

I verified that defining the completion specification in bash as

complete -o default -C /path/to/klog klog

enables default shell expansions for file paths. It would be easy to change the kongplete.InstallCompletions Kong command to define the completion specification in this way. I assume (but have not verified) that similar solutions exist for zsh and fish. If I verify that it works, I will create an issue in kongplete and a pull request with a simple change to the kongplete.installCompletion private function.

[jotaen]

I assume (but have not verified) that similar solutions exist for zsh and fish. If I verify that it works, I will create an issue in kongplete and a pull request with a simple change to the kongplete.installCompletion private function.

Oh terrific, that sounds like a good solution!

I have confirmed on zsh that your fix works! (complete -o default -C /path/to/klog klog)

[jotaen]

Too bad that the kongplete maintainers don’t respond in the issue that you opened. Do you happen to know how complicated it is to fix the kongplete code? Because I think I would just fork the kongplete project and switch the dependency to my fork, in order to move forward with this issue.

[jotaen]

Quick update: I created a fork of kongplete and added a patch for the problem based on your suggestion. With that, it seems to work smoothly. I haven’t looked into fish shell yet, though.

The fork can be dropped in relatively easily via the replace directive (see full diff).

I’ll try to do some more testing throughout the next weeks. It might be nice to file a PR with kongplete (once fish is fixed and tested), but it’s good to know that we would be able to move forward independently of kongplete.

[chairmank]

Thanks for the update! I am sorry that I have not done anything on this issue recently.

What do you think about reimplementing the command-line interface with Cobra? I understand that forking kongplete is easier and a more immediate solution, but Cobra is already maintained and has more features than kongplete. Would you accept a switch to Cobra if it preserves the current behavior of the CLI? If you are okay with the idea, I will explore it and report what I learn.

[jotaen]

Oh, don’t worry! You helped tremendously by figuring out all the groundwork, so thanks heaps for that. As far as I see, the remaining todos are:

• Test fish shell and (if need be) fix the file path issue (in the fork: https://github.com/jotaen/kongplete)
• Take care of the magic --this-*** flags
  • Either by adding a custom predictor, or if that doesn’t work, by abandoning the --this-*** pseudo flag and just enumerate all shortcut flags in the help output

I don’t mean to take this away from you, so let me know if you want to keep working on it. I’m otherwise also happy to collaborate, or take over, depending on your capacity. There is also no rush with this. (Though I have to say the completions are a really cool usability improvement – I’ve started to use them locally now.)

Regarding Cobra, I’ve opened a discussion here, so that we have a dedicated place to discuss: #205.

[chairmank]

Thanks!

I recently started using fish shell and can test the file path issue with your fork. I will do this soon, within a week.

I am also interested in implementing completion for the various --this-*** flags, but it is somewhat complicated to do so while keeping the abbreviated help output. If you or someone else has time to work on it now, please proceed.

Thanks for opening the discussion of Cobra. I don't have time to pursue it right now, but I will be sure to report any progress that I make in the discussion.

[jotaen]

I also played around with fish this weekend, just because I was curious and wanted to check it out in general. While on it, I was able to confirm that the generated completions code seems to work fine 🎉

So I think it’s all good to go now: #209

[chairmank]

I looked at the changes in #209 and everything looks good to me. Thanks for implementing this feature!