Add suggestions support #977

saschagrunert · 2019-12-05T14:38:04Z

Motivation

The new option app.Suggest enables command and flag suggestions via the jaro-winkler distance algorithm. Flags are scoped to their appropriate commands whereas command suggestions are scoped to the current command level.

Closes #895

Release Notes

Add "Did you mean" suggestion support to flags and commands. This feature can be enabled by setting app.Suggest = true.

Changes

Added a new suggestions.go file where all the implementation details go in
Changed critical paths to show-up the suggestions
Added test cases as well

Testing

Added unit tests :)

Reviewer Guidelines

The scope of the suggestions should be correct, that we do not suggest flags from other sub-commands.

codecov · 2019-12-05T14:56:44Z

Codecov Report

Merging #977 into master will increase coverage by 0.14%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #977      +/-   ##
==========================================
+ Coverage   72.91%   73.06%   +0.14%     
==========================================
  Files          33       34       +1     
  Lines        2481     2539      +58     
==========================================
+ Hits         1809     1855      +46     
- Misses        565      573       +8     
- Partials      107      111       +4

Impacted Files	Coverage Δ
suggestions.go	`100.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b7e4e0...002bde2. Read the comment docs.

parse.go

asahasrabuddhe

LGTM

coilysiren

Love the code, and love that the suggestions stuff is mostly in it's own file 🙏

There's 2 more things I'd like to see from this PR:

for the total rest coverage in the PR to be increasing, rather than decreasing.
for suggestions to be added to the v2 manual. I'll stand up a PR for release v2.1 to support this.

AudriusButkevicius · 2019-12-08T12:30:36Z

Should this be an out of the box OnUsageError handler ? Just wondering, not proposing.

saschagrunert · 2019-12-08T12:40:23Z

Should this be an out of the box OnUsageError handler ? Just wondering, not proposing.

Seems a matter of taste to me, so I'm open for a vote :)

Code coverage seems broken for now, see #985 (comment)

saschagrunert · 2019-12-18T08:07:09Z

The coverage now looks better :)
https://codecov.io/github/urfave/cli/commit/2cb8524de32183d2eed9d355e3007417bd6708ae

coilysiren · 2019-12-18T08:28:15Z

Minor request: can you avoid force pushing branches that I've already reviewed? I can code review faster when I can use Github's "commits since last push" feature, and force pushing breaks that.

saschagrunert · 2019-12-18T08:35:16Z

Minor request: can you avoid force pushing branches that I've already reviewed? I can code review faster when I can use Github's "commits since last push" feature, and force pushing breaks that.

Sure, I can do this.

coilysiren

👀 only a few non-blocking suggestions!

suggestions.go

coilysiren · 2019-12-24T12:57:32Z

suggestions.go

+// command name
+func suggestCommand(commands []*Command, provided string) (suggestion string) {
+	distance := 0.0
+	for _, command := range commands {


⭐ (nice to have) I'm not sure about this suggestion, but: would it be a good idea to consider suggesting sub commands too? I'm imagining an example CLI that has exactly these 3 commands

docker info docker info containers docker info images

and I input this

docker containers

I would be expecting to get a suggestion like

did you mean: docker info containers?

whereas I believe this code with give me

did you mean: docker info?

is that a correct assumption?

Yes it is correct that it would only suggest docker info in that case, because can only suggest one command with the current implementation. I'm also not sure if we should add such a traversal suggestion to the users. It might be helpful but it also might result in confusing them. 😁

coilysiren · 2020-01-18T22:16:47Z

👀

saschagrunert · 2020-02-07T09:17:48Z

I rebased on top of the latest master. Do you think we can move forward with this, @lynncyrin @AudriusButkevicius @asahasrabuddhe ?

AudriusButkevicius · 2020-02-07T09:33:01Z

So my only comment is that "Suggest" is a poor name, we have UseXYZ everywhere, so the property could be more descriptive.

saschagrunert · 2020-02-07T09:54:38Z

So my only comment is that "Suggest" is a poor name, we have UseXYZ everywhere, so the property could be more descriptive.

Hm, how about EnableFlagAndCommandSuggestions ? 🤔

AudriusButkevicius · 2020-02-07T12:20:09Z

Sounds better. Perhaps others have suggestions.

coilysiren · 2020-02-09T03:08:37Z

EnableSuggestions? I'm fine with any of the options mentioned, though!

coilysiren

:seal-of-approval:

pending the tests passing ^^

The new option `app.Suggest` enables command and flag suggestions via the jaro-winkler distance algorithm. Flags are scoped to their appropriate commands whereas command suggestions are scoped to the current command level. Signed-off-by: Sascha Grunert <[email protected]>

stale · 2020-06-03T05:28:36Z

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

stale · 2020-07-03T06:40:57Z

Closing this as it has become stale.

mfinley3 · 2020-10-14T17:15:25Z

Hey, just curious what ended up happening with this feature. Looks like stale bot closed it but seems like the work was good. Getting suggestions on invalid commands would still be a nice feature to have

hojongs · 2022-03-04T05:19:22Z

Hey, Why don't you merge this feature?

Add suggestions support (#977)

saschagrunert requested a review from a team as a code owner December 5, 2019 14:38

saschagrunert self-assigned this Dec 5, 2019

saschagrunert requested review from AudriusButkevicius and coilysiren December 5, 2019 14:39

saschagrunert force-pushed the suggestions branch 2 times, most recently from f18f0b7 to 58b52b6 Compare December 5, 2019 14:49

asahasrabuddhe reviewed Dec 5, 2019

View reviewed changes

parse.go Show resolved Hide resolved

asahasrabuddhe previously approved these changes Dec 7, 2019

View reviewed changes

coilysiren requested changes Dec 7, 2019

View reviewed changes

saschagrunert dismissed asahasrabuddhe’s stale review via cb1ce55 December 8, 2019 12:17

saschagrunert force-pushed the suggestions branch from 1dbbdc7 to 576e593 Compare December 8, 2019 12:25

saschagrunert force-pushed the suggestions branch from 576e593 to da26a81 Compare December 18, 2019 08:04

coilysiren mentioned this pull request Dec 24, 2019

Add go module support antzucaro/matchr#10

Merged

coilysiren previously approved these changes Dec 24, 2019

View reviewed changes

saschagrunert dismissed coilysiren’s stale review via 07b5809 December 31, 2019 12:47

saschagrunert force-pushed the suggestions branch from ab18abf to 07b5809 Compare December 31, 2019 12:47

saschagrunert force-pushed the suggestions branch from 6d71d67 to 177d8d4 Compare February 7, 2020 09:17

coilysiren previously approved these changes Feb 9, 2020

View reviewed changes

coilysiren mentioned this pull request Feb 26, 2020

Dynamic autocompletion #855

Closed

saschagrunert force-pushed the suggestions branch from 1aa9d8a to 002bde2 Compare March 3, 2020 12:06

coilysiren self-requested a review March 3, 2020 20:21

coilysiren previously approved these changes Mar 5, 2020

View reviewed changes

coilysiren mentioned this pull request Mar 30, 2020

"Did you mean" hint on wrong command or flag #895

Closed

stale bot added the status/stale stale due to the age of it's last update label Jun 3, 2020

stale bot closed this Jul 3, 2020

meatballhat reopened this Apr 19, 2022

Merge branch 'master' into suggestions

4b238b8

meatballhat dismissed coilysiren’s stale review via 4b238b8 April 19, 2022 20:45

meatballhat removed the status/stale stale due to the age of it's last update label Apr 21, 2022

meatballhat added this to the Release 2.5.0 milestone Apr 21, 2022

meatballhat changed the base branch from master to main April 21, 2022 22:19

meatballhat added kind/feature describes a code enhancement / feature request area/v2 relates to / is being considered for v2 labels Apr 21, 2022

meatballhat modified the milestones: Release 2.5.0, Release 2.6.0, Release 2.x Apr 26, 2022

meatballhat mentioned this pull request May 7, 2022

Add suggestions support (#977) #1390

Merged

meatballhat added the status/superseded has been superseded label May 7, 2022

meatballhat added a commit that referenced this pull request May 17, 2022

Merge pull request #1390 from urfave/saschagrunert-suggestions

e770ee9

Add suggestions support (#977)

meatballhat merged commit 3d67b75 into urfave:main May 17, 2022

renovate bot mentioned this pull request Jul 16, 2023

chore(deps): update all non-major dependencies - autoclosed clok/cdocs#14

Closed

3 tasks

renovate bot mentioned this pull request Feb 19, 2024

fix(deps): update all moul/cryptoguess#5

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add suggestions support #977

Add suggestions support #977

saschagrunert commented Dec 5, 2019

codecov bot commented Dec 5, 2019 •

edited

Loading

asahasrabuddhe left a comment

coilysiren left a comment

AudriusButkevicius commented Dec 8, 2019

saschagrunert commented Dec 8, 2019

saschagrunert commented Dec 18, 2019

coilysiren commented Dec 18, 2019

saschagrunert commented Dec 18, 2019

coilysiren left a comment

coilysiren Dec 24, 2019

saschagrunert Jan 2, 2020

coilysiren Jan 2, 2020

coilysiren commented Jan 18, 2020

saschagrunert commented Feb 7, 2020

AudriusButkevicius commented Feb 7, 2020

saschagrunert commented Feb 7, 2020

AudriusButkevicius commented Feb 7, 2020

coilysiren commented Feb 9, 2020

coilysiren left a comment

stale bot commented Jun 3, 2020

stale bot commented Jul 3, 2020

mfinley3 commented Oct 14, 2020

hojongs commented Mar 4, 2022

Add suggestions support #977

Add suggestions support #977

Conversation

saschagrunert commented Dec 5, 2019

Motivation

Release Notes

Changes

Testing

Reviewer Guidelines

codecov bot commented Dec 5, 2019 • edited Loading

Codecov Report

asahasrabuddhe left a comment

Choose a reason for hiding this comment

coilysiren left a comment

Choose a reason for hiding this comment

AudriusButkevicius commented Dec 8, 2019

saschagrunert commented Dec 8, 2019

saschagrunert commented Dec 18, 2019

coilysiren commented Dec 18, 2019

saschagrunert commented Dec 18, 2019

coilysiren left a comment

Choose a reason for hiding this comment

coilysiren Dec 24, 2019

Choose a reason for hiding this comment

saschagrunert Jan 2, 2020

Choose a reason for hiding this comment

coilysiren Jan 2, 2020

Choose a reason for hiding this comment

coilysiren commented Jan 18, 2020

saschagrunert commented Feb 7, 2020

AudriusButkevicius commented Feb 7, 2020

saschagrunert commented Feb 7, 2020

AudriusButkevicius commented Feb 7, 2020

coilysiren commented Feb 9, 2020

coilysiren left a comment

Choose a reason for hiding this comment

stale bot commented Jun 3, 2020

stale bot commented Jul 3, 2020

mfinley3 commented Oct 14, 2020

hojongs commented Mar 4, 2022

codecov bot commented Dec 5, 2019 •

edited

Loading