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

Added tab-completions for ZSH. #201

Merged
merged 6 commits into from Jan 29, 2018
Merged

Added tab-completions for ZSH. #201

merged 6 commits into from Jan 29, 2018

Conversation

@propensive
Copy link
Contributor

@propensive propensive commented Jan 26, 2018

The installation instructions in the README file are experimental. They
worked for me, though I don't know how special my own configuration is.

The will also be some maintenance overhead involved with this. As
parameters change, the zsh-completions file will need to change with
them. In the long-term, maybe we could auto-generate this file somehow
(for example, by parsing the output from bloop <subcommand> --help),
but for now we should probalbly merge early and get feedback sooner.

The definitions for the ZSH completions involve really ugly code. Any
improvements would be very welcome.

You may want to make an additional change after merging this. Having
bloop, bloop-server and bloop-shell all on the PATH at the same
time means that (assuming there are no other commands beginnig with
blo), typing blo<tab> will autocomplete to bloop without a space
after it, because of the bloop-server and bloop-shell completions.

If bloop-server and bloop-shell were to be renamed to, say,
blp-server and blp-shell respectively, then blo<tab> would take
you straight to the subcommands list, inevitably saving you valuable
microseconds. # Please enter the commit message for your changes. Lines
starting.

The installation instructions in the README file are experimental. They
worked for me, though I don't know how special my own configuration is.

The will also be some maintenance overhead involved with this. As
parameters change, the zsh-completions file will need to change with
them. In the long-term, maybe we could auto-generate this file somehow
(for example, by parsing the output from `bloop <subcommand> --help`),
but for now we should probalbly merge early and get feedback sooner.

You may want to make an additional change after merging this. Having
`bloop`, `bloop-server` and `bloop-shell` all on the PATH at the same
time means that (assuming there are no other commands beginnig with
`blo`), typing `blo<tab>` will autocomplete to `bloop` *without a space*
after it, because of the `bloop-server` and `bloop-shell` completions.

If `bloop-server` and `bloop-shell` were to be renamed to, say,
`blp-server` and `blp-shell` respectively, then `bloop<tab>` would take
you straight to the subcommands list, inevitably saving you valuable
microseconds.  # Please enter the commit message for your changes. Lines
starting.
@propensive propensive requested review from Duhemm and jvican Jan 26, 2018
@propensive
Copy link
Contributor Author

@propensive propensive commented Jan 26, 2018

Renaming bloop-server and bloop-shell to blp-server and blp-shell in the obvious way (search and replace) worked, so I'm including this in the PR, too.

propensive added 2 commits Jan 26, 2018
This reads the configuration files to find all binary output
directories, then uses `find` to get all class files in them, and
reformats the filenames to look like fully-qualified classnames.

It's ugly, but it seems to work.
Copy link
Member

@Duhemm Duhemm left a comment

Thanks, that's really great!

Renaming bloop-server and bloop-shell (this one is going away anyway) is probably the way to go given how much it'll improve the shell completion.

It'd also be great to be able to generate the completion. I imagine it'd be doable to inspect bloop.cli.Commands via reflection and generate the completions. Or add that directly in CaseApp? If it sounds too complicated, I think that we can leave generation of completions for another PR. I don't expect that we'll be adding that many more commands and options in the near future... It'd definitely be nice to have though.

Finally, it'd be awesome if the brew formula took care of installing the completions. There's support for that in Homebrew, see this example:
https://github.com/Homebrew/homebrew-core/blob/ee727e0f8985027fc889b0c35d7eaacdef4c0d02/Formula/kerl.rb#L10-L14

I'm happy to take care of that if you don't have a Mac 😄

case $words[1] in
about)
_arguments -C \
{-c --config-dir}'[File path to the bloop config directory]:directory:_files -/' \

This comment has been minimized.

@Duhemm

Duhemm Jan 29, 2018
Member

Why did you remove the comma here? (between -c and --config-dir)

This comment has been minimized.

@propensive

propensive Jan 29, 2018
Author Contributor

That was a mistake, actually! Though it seems to work anyway... ;)


_projects() {
local projects; projects=( $(ls .bloop-config 2> /dev/null | sed 's/.config$//g') )
_describe 'values' projects

This comment has been minimized.

@Duhemm

Duhemm Jan 29, 2018
Member

👍 👍 👍

@jvican
Copy link
Member

@jvican jvican commented Jan 29, 2018

This is something that was on the todo list, so thank you for submitting a PR. I echo Martin's idea of autogenerating the autocompletions rather than making them explicit. This should not take a lot of time, Scalafix has already done the hard work. For a caseapp integration, could you emulate what Scalafix does @propensive https://github.com/scalacenter/scalafix/blob/master/scalafix-cli/src/main/scala/scalafix/cli/Cli.scala#L70-L159 ?

@olafurpg
Copy link
Member

@olafurpg olafurpg commented Jan 29, 2018

I would recommend to merge the manual static completions for now since they already improve the state of the art. Then open a ticket to discuss how to how to automatically keep the completions up to do date, case-app provides all the necessary information to generate completion files.

Another interesting topic worth discussing is that bloop is always hot via nailgun, so I think you can also complete dynamic information such as project IDs 🔥

@jvican
jvican approved these changes Jan 29, 2018
@jvican
Copy link
Member

@jvican jvican commented Jan 29, 2018

Another interesting topic worth discussing is that bloop is always hot via nailgun, so I think you can also complete dynamic information such as project IDs

Unfortunately, I don't this is possible without rewriting the nailgun protocol... Would be cool to experiment with it when bloop is stable, but writing my own custom nailgun protocol is something I'd like to avoid 🤔.

@propensive I defer merging this to @Duhemm, this looks good to me! Let's tackle the scalable solution in another PR.

@propensive
Copy link
Contributor Author

@propensive propensive commented Jan 29, 2018

I'm actually tending towards the answer I've just given on coursier completions.

I agree that autogeneration is the best long-term solution, though I'd like to treat it separately for now. I think it needs a bit more work than what Olaf's already done for the same support currently in these completions, but that shouldn't stop anything right now.

So, in case it's not clear, I'm going to propose:

  1. move this to a "completions-for-scala-tools" project
  2. leave the PR open for now
  3. look towards autogenerating the completions in the future

Regarding (3), this would typically work by providing a (probably a bit secret) subcommand, such as,

bloop completions --zsh

or

bloop completions --bash

which would spit out the completions commands relating to that version of bloop. Meanwhile, the bloop script in the ZSH-completions project would be written to simply call bloop completions --zsh, and it could then use them.

So, leave this PR unmerged for now, and I'll get to work on a more general ZSH bundle for Scala users.

@jvican
Copy link
Member

@jvican jvican commented Jan 29, 2018

Hey Jon, are you sure you don't wanna merge this? It looks good as it is.

@propensive
Copy link
Contributor Author

@propensive propensive commented Jan 29, 2018

If you think it's worth including now, go for it. :) I intend to look into better long-term solutions, but it probably makes sense to merge this now, and remove it if and when those long-term solutions materialize.

propensive and others added 2 commits Jan 29, 2018
@Duhemm
Copy link
Member

@Duhemm Duhemm commented Jan 29, 2018

Awesome, @jvican brought the CI back from its vacation :)

LGTM!

Just a side-note when generating the completion: It'd be better if the generation was a command from the server rather than from the Bloop CLI. This way, it's easier to generate while installation with, say, homebrew.

@Duhemm
Duhemm approved these changes Jan 29, 2018
@Duhemm Duhemm merged commit 06cb55d into master Jan 29, 2018
2 checks passed
2 checks passed
continuous-integration/drone/pr the build was successful
Details
continuous-integration/drone/push the build was successful
Details
@propensive propensive deleted the zsh-completions branch Jan 29, 2018
@propensive
Copy link
Contributor Author

@propensive propensive commented Jan 29, 2018

@Duhemm Thank you!

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

Successfully merging this pull request may close these issues.

None yet

4 participants