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 suggestions support (#977) #1390

Merged
merged 10 commits into from May 17, 2022
Merged

Add suggestions support (#977) #1390

merged 10 commits into from May 17, 2022

Conversation

meatballhat
Copy link
Member

@meatballhat meatballhat commented May 7, 2022

What type of PR is this?

  • feature

What this PR does / why we need it:

See #977

Which issue(s) this PR fixes:

Closes #895

Special notes for your reviewer:

This is intended to carry forward the work in #977 with the addition of build tags to allow opting out of the suggestions dependency at build time as with recently-added work for markdown/man doc dependencies (cc @kolyshkin).

saschagrunert and others added 5 commits Mar 3, 2020
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 <sgrunert@suse.com>
@meatballhat meatballhat added kind/feature describes a code enhancement / feature request area/v2 relates to / is being considered for v2 labels May 7, 2022
@meatballhat meatballhat requested a review from a team as a code owner May 7, 2022
@meatballhat meatballhat added this to the Release 2.x milestone May 8, 2022
@kolyshkin
Copy link
Contributor

kolyshkin commented May 12, 2022

@meatballhat what is the size difference with and without suggestions?

suggestions.go Show resolved Hide resolved
@meatballhat
Copy link
Member Author

meatballhat commented May 14, 2022

@kolyshkin regarding size difference

$ go run ./internal/build/build.go check-binary-size
# ---> /usr/sbin/git rev-parse --show-toplevel
# ---> /home/me/.gimme/versions/go1.18.1.linux.amd64/bin/go build -tags  -o ./internal/example-cli/built-example -ldflags -s -w ./internal/example-cli/example-cli.go
# ---> /home/me/.gimme/versions/go1.18.1.linux.amd64/bin/go build -tags  -o ./internal/example-hello-world/built-example -ldflags -s -w ./internal/example-hello-world/example-hello-world.go

1.7MB is the current binary size
  ✅ 1.7MB is the target min size
  ✅ 2.2MB is the target max size
$ go run ./internal/build/build.go --tags urfave_cli_no_suggest check-binary-size
# ---> /usr/sbin/git rev-parse --show-toplevel
# ---> /home/me/.gimme/versions/go1.18.1.linux.amd64/bin/go build -tags urfave_cli_no_suggest -o ./internal/example-cli/built-example -ldflags -s -w ./internal/example-cli/example-cli.go
# ---> /home/me/.gimme/versions/go1.18.1.linux.amd64/bin/go build -tags urfave_cli_no_suggest -o ./internal/example-hello-world/built-example -ldflags -s -w ./internal/example-hello-world/example-hello-world.go

1.7MB is the current binary size
  ✅ 1.7MB is the target min size
  ✅ 2.2MB is the target max size

(small enough that it's not showing up?)

$ du internal/example-cli/built-example*
2852    internal/example-cli/built-example
2844    internal/example-cli/built-example-no-suggest

meatballhat added a commit that referenced this pull request May 14, 2022
@kolyshkin
Copy link
Contributor

kolyshkin commented May 16, 2022

(small enough that it's not showing up?)

The size difference seems to be about 0.1% (a few kilobytes), so it probably does not make sense to add a build tag for this.

@meatballhat meatballhat merged commit e770ee9 into main May 17, 2022
13 checks passed
@meatballhat meatballhat deleted the saschagrunert-suggestions branch May 17, 2022
meatballhat added a commit that referenced this pull request May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v2 relates to / is being considered for v2 kind/feature describes a code enhancement / feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Did you mean" hint on wrong command or flag
4 participants