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

Add fish tab completion capabilities #721

Merged
merged 5 commits into from
Nov 22, 2018

Conversation

jlprat
Copy link
Contributor

@jlprat jlprat commented Nov 15, 2018

Refs: #695
Adds documentation on how to install
Adds documentation for developers when changing commands
Provides a file to generate tab completions for fish

Disclaimer on the approach: fish tab completions take a more descriptive approach than the one Bash or zsh have. The common way to write such tab completion files for fish is usually by listing all possible sub-commands and flags in such files.

I'm not sure if some other areas would need to be touched, like the installation script for example

@jlprat
Copy link
Contributor Author

jlprat commented Nov 15, 2018

This test (Test bloop.tasks.JsTestSpec) failed on CI, but I don't know how this is related to my changes.

Copy link
Contributor

@jvican jvican left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work @jlprat, nevermind the CI errors, I'll fix them myself. Thanks for this high-quality solution, generators are always the way to go 😉

@jvican jvican added cli ergonomics Any change that affects developer ergonomics and the easiness of use of bloop. community Any change or proposal that is contributed by the Open Source Community. labels Nov 16, 2018
@jlprat
Copy link
Contributor Author

jlprat commented Nov 16, 2018

You're welcome @jvican ! What about the installation scripts? Will the new file under etc/fish/bloop.fish be "magically" copied to .bloop/fish/bloop.fish?

@jvican
Copy link
Contributor

jvican commented Nov 19, 2018

You're welcome @jvican ! What about the installation scripts? Will the new file under etc/fish/bloop.fish be "magically" copied to .bloop/fish/bloop.fish?

They won't be magically copied, you need to modify two files:

  1. The python script in bin/install.py for multi-OS installation
  2. The contents of the ReleaseUtils brew installation script which we use to install bloop in OSX (https://github.com/scalacenter/bloop/blob/master/project/ReleaseUtils.scala#L98)

One question that comes to mind: can we avoid generating this file and instead let the server reply to the fish's autocompletion engine? The following SO question has two relevant answers that hint that this is possible. One reason why I prefer this approach is because it's likely that this file will go outdated.

@jlprat
Copy link
Contributor Author

jlprat commented Nov 19, 2018

Without being an expert in fish, letting the server respond will only work if the bloop server is running when the shell starts.
If we don't consider this, then it is possible to have everything scripted and never be outdated.

The main probles comes with the flag completion, where we need to have 1 complete per flag. I the server is down, the loop won't generate any entry.

I also didn't line this solution, but as it's being used already to keep the docs up, I guessed it won't be outdated that easily.

PS: you forgot the SO link 😉

@jvican
Copy link
Contributor

jvican commented Nov 19, 2018

you forgot the SO link

https://stackoverflow.com/questions/20838284/how-can-i-provide-tab-completions-to-fish-shell-from-my-own-script

Without being an expert in fish, letting the server respond will only work if the bloop server is running when the shell starts.

I would favor talking to the server to get the autocompletions directly since that's the way bash and zsh autocompletions work in bloop now. It's true that autocompletions won't work if the server is not running, but that's OK since you can do nothing when the server is down (the best way to ensure that this doesn't happen is that you have brew services/systemd enabled in your system).

@jlprat
Copy link
Contributor Author

jlprat commented Nov 19, 2018

If this is an acceptable case, then I can rewrite it to use the autocompletions directly from server.
The only problem I can image is, if somebody starts bloop manually, they would need to restart fish to get the completions working. But I guess, is an acceptable compromise.

PR update will come during next days.

@jvican
Copy link
Contributor

jvican commented Nov 20, 2018

if somebody starts bloop manually, they would need to restart fish to get the completions working.

Is this a fish-specific limitation? I've never had such issues with zsh/bash.

PR update will come during next days.

Thanks Josep!

@jlprat
Copy link
Contributor Author

jlprat commented Nov 20, 2018

Is this a fish-specific limitation? I've never had such issues with zsh/bash.

If I understand zsh completion correctly, one defines a syntax for the command with the functions that will provide the completions. This is way more flexible and fits better to bloop's way of working. On the other hand, as far as my understanding of fish goes, yes, it is a fish limitation. Fish tab completions are designed to be generated from man pages, additionally and all the examples I saw around are upfront definitions. Nothing like the zsh style.

There are conditionals on the complete command and some interesting things can be done this way, like:
complete -c bloop -f -n "__assert_args_count 0" -a '(_commands)'
But you would need a complete command for each flag and subcommand, and this is where the things get trickier.

The main problem is that in fish, one can't define multiple flags (or options in fish naming convention) in one single complete command. This, together with the fact that bloop autocomplete only works when the server is started (obviously) make it more challenging. I have a way to overcome this I think.

@@ -62,7 +62,7 @@
help="Tell the location of the ivy home.")
parser.add_argument(
'--bloop-home',
help="Tell the location of the ivy home.")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed a typo here



# Nothing has been provided yet. Complete with commands
complete -c bloop -f -n "__assert_args_count 0" -a '(_commands)'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will recover if bloop server is not running and then started.

complete -c bloop -f -n "__assert_args_count 0" -a '(_commands)'

# Only a project command has been provided
complete -c bloop -f -n "__assert_args_count 1; and __fish_seen_subcommand_from_project_commands" -a '(_projects)'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will recover if bloop server is not running and then started.

set -l flags (_flags_for_command $cmd)
for flag in $flags
set -l parsed (string split '#' $flag)
if test -n $parsed[1] -a -n $parsed[2] -a -n $parsed[3]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These will not recover if bloop server is not running and then started.

Copy link
Contributor

@jvican jvican left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jlprat Thanks! This looks great, I love that we reuse the same machinery than zsh and bash do. I've just left one comment.

Can you rebase the PR so that I can merge after the comment is addressed?

#### Fish completions

To get the command line completions with fish shell, navigate to your local completion location (e.g.: `~/.config/fish/completions`)
and create a symlink to the file provided in the Bloop distribution:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we mention here what to do if the bloop server is not running and how to get the fish completions to work after the user manually starts the server?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, makes sense to do so.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, whenever you make the change, please rebase the PR so that i can merge it right away.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change done, and rebased against master.

@jlprat jlprat force-pushed the fish-tab-completion branch from ee9feb3 to b5c8651 Compare November 22, 2018 19:37
Refs: scalacenter#695
Adds documentation on how to install
Adds documentation for developers when changing commands
Provides a file to generate tab completions for fish

Disclaimer on the approach: fish tab completions take a more descriptive approach than the one Bash or zsh have. The common way to write such tab completion files for fish is usually by listing all possible sub-commands and flags in such files.
Remove echo that was used for debugging purposes
@jlprat jlprat force-pushed the fish-tab-completion branch from b5c8651 to f100f84 Compare November 22, 2018 19:39
@jvican
Copy link
Contributor

jvican commented Nov 22, 2018

🙇

@jvican jvican merged commit 14a7989 into scalacenter:master Nov 22, 2018
@jlprat jlprat deleted the fish-tab-completion branch November 22, 2018 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli community Any change or proposal that is contributed by the Open Source Community. ergonomics Any change that affects developer ergonomics and the easiness of use of bloop.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants