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 completion for rules description in zsh. #639

Merged
merged 2 commits into from
Feb 28, 2018

Conversation

tanishiking
Copy link
Contributor

issue: #260

This PR enable zsh to show rules description on tab completion like this gif.

scalafix-zsh-completion

I couldn't find the way to show description on tab completion (without autocompleting whole element including description) in bash and sbt. 😢

ScalafixRules.allNamesDescriptions
.map {
case (name, description) =>
s""""$name[${StringEscapeUtils.escapeXSI(description)}]""""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is necessary to quote characters like | & ; ... (see this doc).

I think it is reasonable to use org.apache.commons.text.StringEscapeUtils.escapeXSI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good 👍 If we want to slim down the jars we can always inline the implentation, doesn't look too bad https://github.com/apache/commons-text/blob/ddb47903dcab034e7dd33eadfe2658eb3aad0739/src/main/java/org/apache/commons/text/StringEscapeUtils.java#L234-L261

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also escape cli flag descriptions? I've noticed that scalafix --<TAB> can act a bit weirdly sometimes.

@tanishiking
Copy link
Contributor Author

Thank you! I'll try to implement it in sbt.

Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

Very cool @tanishiking !

LGTM 👍 The sbt completions can be added separately, this is just perfect 💯

One question about escaping help descriptions, otherwise ready to merge.

ScalafixRules.allNamesDescriptions
.map {
case (name, description) =>
s""""$name[${StringEscapeUtils.escapeXSI(description)}]""""
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good 👍 If we want to slim down the jars we can always inline the implentation, doesn't look too bad https://github.com/apache/commons-text/blob/ddb47903dcab034e7dd33eadfe2658eb3aad0739/src/main/java/org/apache/commons/text/StringEscapeUtils.java#L234-L261

ScalafixRules.allNamesDescriptions
.map {
case (name, description) =>
s""""$name[${StringEscapeUtils.escapeXSI(description)}]""""
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also escape cli flag descriptions? I've noticed that scalafix --<TAB> can act a bit weirdly sometimes.

@tanishiking
Copy link
Contributor Author

tanishiking commented Feb 28, 2018

Should we also escape cli flag descriptions? I've noticed that scalafix -- can act a bit weirdly sometimes.

Hmm... cli flag descriptions are escaped by hands and I couldn't reproduce its weird acts, but we use characters like ' or " in cli descriptions flag, it would act weiredly.
Now we can use org.apache.commons.text.StringEscapeUtils, I think it's better to use StringEscapeUtils.escapeXSI()

@tanishiking
Copy link
Contributor Author

tanishiking commented Feb 28, 2018

I escaped cli description flags using StringEscapeUtils 99c478e .

Output scalafix_opts and screenshot
scalafix_opts=(
  "--usage[Print\ usage\ and\ exit]" \
   "--help[Print\ help\ message\ and\ exit]" \
   "-h[Print\ help\ message\ and\ exit]" \
   "--version[Print\ version\ number\ and\ exit]" \
   "-v[Print\ version\ number\ and\ exit]" \
   "--verbose[If\ set,\ print\ out\ debugging\ inforation\ to\ stderr.]" \
   "--config[File\ path\ to\ a\ .scalafix.conf\ configuration\ file.]" \
   "-c[File\ path\ to\ a\ .scalafix.conf\ configuration\ file.]" \
   "--config-str[String\ representing\ scalafix\ configuration]" \
   "-c[String\ representing\ scalafix\ configuration]" \
   "--sourceroot[Absolute\ path\ passed\ to\ semanticdb\ with\ -P:semanticdb:sourceroot:\<path\>.\ Relative\ filenames\ persisted\ in\ the\ Semantic\ DB\ are\ absolutized\ by\ the\ sourceroot.\ Defaults\ to\ current\ working\ directory\ if\ not\ provided.]" \
   "--classpath[java.io.File.pathSeparator\ separated\ list\ of\ directories\ or\ jars\ containing\ \'.semanticdb\'\ files.\ The\ \'semanticdb\'\ files\ are\ emitted\ by\ the\ semanticdb-scalac\ compiler\ plugin\ and\ are\ necessary\ for\ semantic\ rules\ like\ ExplicitResultTypes\ to\ function.]" \
   "--classpath-auto-roots[Automatically\ infer\ --classpath\ starting\ from\ these\ directories.\ Ignored\ if\ --classpath\ is\ provided.]" \
   "--tool-classpath[Additional\ classpath\ to\ use\ when\ classloading/compiling\ rules]" \
   "--no-strict-semanticdb[Disable\ validation\ when\ loading\ semanticdb\ files.]" \
   "*--rules=[Scalafix\ rules\ to\ run.]:rule:_rule_names" \
   "*-r=[Scalafix\ rules\ to\ run.]:rule:_rule_names" \
   "--stdout[If\ set,\ print\ fix\ to\ stdout\ instead\ of\ writing\ to\ file.]" \
   "--test[Exit\ non-zero\ code\ if\ files\ have\ not\ been\ fixed.\ Won\'t\ write\ to\ files.]" \
   "--out-from[Regex\ that\ is\ passed\ as\ first\ argument\ to\ fileToFix.replaceAll\(outFrom,\ outTo\)]" \
   "--out-to[Replacement\ string\ that\ is\ passed\ as\ second\ argument\ to\ fileToFix.replaceAll\(outFrom,\ outTo\)]" \
   "--exclude[Space\ separated\ list\ of\ regexes\ to\ exclude\ which\ files\ to\ fix.\ If\ a\ file\ match\ one\ of\ the\ exclude\ regexes,\ then\ it\ will\ not\ get\ fixed.\ Defaults\ to\ excluding\ no\ files.]" \
   "--single-thread[If\ true,\ run\ on\ single\ thread.\ If\ false\ \(default\),\ use\ all\ available\ cores]" \
   "--no-sys-exit[If\ true,\ does\ not\ sys.exit\ at\ the\ end.\ Useful\ for\ example\ in\ sbt-scalafix]" \
   "--quiet-parse-errors[Don\'t\ report\ parse\ errors\ for\ non-explictly\ passed\ filepaths.]" \
   "--bash[Print\ out\ bash\ completion\ file\ for\ scalafix.\ To\ install\ on\ scalafix\ --bash\ \>\ /usr/local/etc/bash_completion.d/scalafix\ \#\ Mac\ scalafix\ --bash\ \>\ /etc/bash_completion.d/scalafix\ \#\ Linux]" \
   "--zsh[Print\ out\ zsh\ completion\ file\ for\ scalafix.\ To\ install:\ scalafix\ --zsh\ \>\ /usr/local/share/zsh/site-functions/_scalafix]" \
   "--non-interactive[Don\'t\ use\ fancy\ progress\ bar.]" \
   "--project-id[String\ ID\ to\ prefix\ reported\ messages\ with]" \
   "--diff[If\ set,\ only\ apply\ scalafix\ to\ added\ and\ edited\ files\ in\ git\ diff\ against\ master.]" \
   "--diff-base[If\ set,\ only\ apply\ scalafix\ to\ added\ and\ edited\ files\ in\ git\ diff\ against\ a\ provided\ branch,\ commit\ or\ tag.\ \(defaults\ to\ master\)]" \
   "--format"
)

2018-02-28 13 32 33


LGTM 👍 The sbt completions can be added separately, this is just perfect 💯
One question about escaping help descriptions, otherwise ready to merge.

OK! Thanks.

@tanishiking tanishiking force-pushed the zsh-completion-description branch from c9f052b to 99c478e Compare February 28, 2018 04:33
Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

Just tried locally and it's beautiful 😍

screen shot 2018-02-28 at 11 41 06

Default flag completions also seem more robust now, can cycle through them by holding tab without problem. Thanks a lot @tanishiking !

@olafurpg olafurpg merged commit 0239fb7 into scalacenter:master Feb 28, 2018
@tanishiking tanishiking deleted the zsh-completion-description branch March 15, 2018 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants