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 simple ZSH completions, and updated README with install details #756

Merged
merged 3 commits into from Jan 31, 2018
Merged

Added simple ZSH completions, and updated README with install details #756

merged 3 commits into from Jan 31, 2018

Conversation

@propensive
Copy link
Contributor

@propensive propensive commented Jan 27, 2018

These completions were generated by copy/pasting details from the
version 1.0.0 '--help' output, and had a small amount of human input.
The completions are by no means perfect, and there is no intelligent
completion of parameter values (except --mode, which was trivial).

I hope that completions prove to be useful, and can form a basis that
other users of coursier can build upon, to improve the interactivity of
the command-line client.

These completions were generated by copy/pasting details from the
version 1.0.0 '--help' output, and had a small amount of human input.
The completions are by no means perfect, and there is no intelligent
completion of parameter values (except `--mode`, which was trivial).

I hope that completions prove to be useful, and can form a basis that
other users of coursier can build upon, to improve the interactivity of
the command-line client.
Copy link
Contributor

@olafurpg olafurpg left a comment

I have some (hacky) code in scalafix to automatically generate zsh and bash completions using case-app data structures https://github.com/scalacenter/scalafix/blob/425bc53a6e9ae9682a18008fbdc27b8403512f95/scalafix-cli/src/main/scala/scalafix/cli/Cli.scala#L70 it may contains some scalafix specific parts, it doesn't handle commands for example. But it might give inspiration for how to generate these files automatically for easier maintenance

@wisechengyi
Copy link
Collaborator

@wisechengyi wisechengyi commented Jan 27, 2018

Thanks @propensive! This seems a quite handy patch. However, how do you plan to maintain it? There is a lot of work going on into the cli right now, so I'd expect this will soon be out of date.

@propensive
Copy link
Contributor Author

@propensive propensive commented Jan 28, 2018

@wisechengyi I don't have a good answer to maintaining it. I see a few options:

  1. Require that any PRs which change coursier parameters also update the ZSH completions.
  2. Don't require that, but add a warning to users that the completions may be out of sync, and wait for fixes to appear separately.
  3. Move the ZSH completions to a completely different project which I would maintain.
  4. Automate the generation of the completions file from Coursier.

(1) is probably too much to expect from users, many of whom won't be using ZSH. Not realistic, I think.

(2) is reasonable, particularly if coursier releases are not too frequent: someone could ping me when the next release is feature-complete and I (or someone else) could devote an hour or two to updating the completions prior to the next release.

(3) is something I'm seriously considering, as I also have ZSH completions for Bloop and Scalafix. I'm thinking that a single project could provide support for a variety of Scala command-line tools. It would have many of the disadvantages of (2), and may require additional runtime checks on the exact version of coursier that's installed to decide which completions to display; more work.

(4) is more work, though would be a better long-term solution. Some other CLI tools offer a command which spits out the completion code. This should be a long term goal.

My feeling is that it might be better to close this PR along with the other ZSH completion PRs I have open on other projects, and go with option (3).

What do you think?

@olafurpg
Copy link
Contributor

@olafurpg olafurpg commented Jan 28, 2018

I think option (4) should not be too much work, running scalafix --bash prints out the completion script for bash

coursier launch ch.epfl.scala:scalafix-cli_2.12.3:0.5.7 --main scalafix.cli.Cli -- --bash
_scalafix()
{
    local cur prev opts
    COMPREPLY=()
    cur="${COMP_WORDS[COMP_CWORD]}"
    prev="${COMP_WORDS[COMP_CWORD-1]}"
    rules="DottyKeywords DisableSyntax NoFinalize NoValInForComprehension RemoveXmlLiterals VolatileLazyVal ProcedureSyntax ExplicitUnit DottyVolatileLazyVal DottyVarArgPattern NoInfer Sbt1 ExplicitResultTypes ExplicitReturnTypes RemoveUnusedImports RemoveUnusedTerms NoAutoTupling Disable"
    opts="--usage --help -h --version -v --verbose --config -c --config-str --sourceroot --classpath --classpath-auto-roots --tool-classpath --no-strict-semanticdb --rules -r --files -f --stdout --test --out-from --out-to --exclude --single-thread --no-sys-exit --in-place -i --working-directory --out --in --err --stack-verbosity --quiet-parse-errors --bash --zsh --non-interactive --project-id --sbt --diff --diff-base"

    case "${prev}" in
      --rules|-r )
        COMPREPLY=(   $(compgen -W "${rules}" -- ${cur}) )
        return 0
        ;;
    esac
    if [[ ${cur} == -* ]] ; then
        COMPREPLY=(   $(compgen -W "${opts}" -- ${cur}) )
        return 0
    fi
}
complete -F _scalafix scalafix
@wisechengyi
Copy link
Collaborator

@wisechengyi wisechengyi commented Jan 29, 2018

Thanks @propensive. I agree with @olafurpg, (4) sounds good, and CI would need to check for it.

I think it's also fine if you want to close and merge this one, although I'd kindly request to make it clear in the README that it may potentially be out of sync before (4) is implemented.

@propensive
Copy link
Contributor Author

@propensive propensive commented Jan 29, 2018

So, although (4) wins for being the most maintainable solution, I'm moving towards (3) because I think it gives a better experience for the intersection of Scala and ZSH users. I could bundle all the completions into a single project, and you would only have to "install" them once, and it wouldn't matter if the completions bundle supported lots of different commands which weren't used. Also, for completions developers (who typically aren't the same people contributing to Scala projects), it would be a lower barrier to make PRs. Coursier maintainers wouldn't need to know how ZSH completions work in order to review PRs.

Option (4) would still be available to improve with maintenance, but could come later, and would be independent. The completions would generally not be entirely automatically generated either: the completion of certain parameters could use special functions to run.

I'll leave this PR open for now, until I get the other project sorted out (so that, just in case I get hit by a bus, you can still go with one of the other options), and I'll replace it with a PR to the README when I'm done.

@alexarchambault
Copy link
Member

@alexarchambault alexarchambault commented Jan 29, 2018

I'd be ok with merging that, until, as @olafurpg suggested, the completions can be generated via case-app, at which point either they would be committed in this repo, or generated on-the-fly via the launcher.

About (3), managing them from the coursier repo has the advantage of allowing to install them along with the launcher via brew for example.

About generating them from case-app, a few PRs need to be merged before that (#735, then switch to the latest case-app via #748, after a - big - rebase). So I'm ok with merging this PR in a first time.

@alexarchambault
Copy link
Member

@alexarchambault alexarchambault commented Jan 29, 2018

btw, CI fails because of the changes in the README. These should be done in doc/README.md, then one should run sbt tut to update the README at the root.

@alexarchambault
Copy link
Member

@alexarchambault alexarchambault commented Jan 31, 2018

Merging that.

The risk that these become deprecated isn't too high, as we should generate the completions automatically at some point

@alexarchambault alexarchambault merged commit 359ebf9 into coursier:master Jan 31, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
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