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.

Sign up for GitHub

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

Jump to bottom

Bug/fix issue 1703 #1728

Merged
merged 2 commits into from May 1, 2023
Merged

Bug/fix issue 1703 #1728

merged 2 commits into from May 1, 2023

Conversation

jojje
Copy link

@jojje jojje commented Apr 30, 2023

What type of PR is this?

  • bug

What this PR does / why we need it:

(REQUIRED)

Which issue(s) this PR fixes:

fixes #1703

Special notes for your reviewer:

Let me know if you want the solution somewhere else in the code-base.

I noticed that you try to contain corner case handling to the helpCommand.Action callback. However I'm not familiar enough with the code-base to fully understand what the right set of conditions would be by comparing various disparate but seemingly interconnected objects and fields/attributes.

Instead I went for the most intuitive solution of simply ensuring that whenever the help command is about to be rendered, instead of assuming that the caller has populated the cCtx correctly, I used defensive programming to ensure honoring the intent of using a custom template for given command, at the closest location to the template use, if the command happens to have one defined.

Testing

Two unit tests added. One for the happy path to ensure the behavior didn't change in that regard. The other for the error path which triggers the bug and which this fix addresses.

tests were performed by running make test

Release Notes

(REQUIRED)

* Custom help templates are now used when -h[elp] flag is given, even when no sub-commands are defined.

@jojje jojje requested a review from a team as a code owner April 30, 2023 16:45
@dearchap dearchap changed the base branch from main to v2-maint April 30, 2023 20:51
dearchap
dearchap reviewed Apr 30, 2023
View reviewed changes
help_test.go Outdated
{
Name: "dummy",
CustomHelpTemplate: "TEMPLATE",
HideHelpCommand: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is redundant since default is false.

@dearchap
Copy link
Contributor

@jojje Thanks for your PR. I've changed the merge from main to v2-maint.

@meatballhat meatballhat added the area/v2 relates to / is being considered for v2 label May 1, 2023
meatballhat
meatballhat reviewed May 1, 2023
View reviewed changes
help.go Outdated
Comment on lines 87 to 99
func anyFlag(candidates []string, flags []Flag) bool {
for _, flag := range flags {
for _, alias := range flag.Names() {
for _, c := range candidates {
if c == alias {
return true
}
}
}
}
return false
}

Copy link
Member

Choose a reason for hiding this comment

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

I can't find where this function is used 🤔

Copy link
Author

Choose a reason for hiding this comment

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

Oh, that should never have made it into the PR.
It was a different approach to solving the problem. Thanks for spotting this Dan!

Pushed a corrective commit that removes this function, and also the redundant test which DC pointed out.
Once you're happy, I'll squash the PR.

@meatballhat meatballhat added the kind/bug describes or fixes a bug label May 1, 2023
@dearchap dearchap merged commit dc77f3c into urfave:v2-maint May 1, 2023
10 checks passed
@jojje jojje deleted the bug/fix-issue-1703 branch May 3, 2023 19:50
another-rex pushed a commit to google/osv-scanner that referenced this pull request Jun 6, 2023
@renovate-bot
fix(deps): update osv-scanner minor (#392)
b4a32fe 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github.com/BurntSushi/toml](https://togithub.com/BurntSushi/toml) |
require | minor | `v1.2.1` -> `v1.3.0` |
| [github.com/go-git/go-git/v5](https://togithub.com/go-git/go-git) |
require | minor | `v5.6.1` -> `v5.7.0` |
| [github.com/spdx/tools-golang](https://togithub.com/spdx/tools-golang)
| require | patch | `v0.5.0` -> `v0.5.1` |
| [github.com/urfave/cli/v2](https://togithub.com/urfave/cli) | require
| patch | `v2.25.3` -> `v2.25.5` |
| golang.org/x/exp | require | digest | `dd950f8` -> `2e198f4` |
| golang.org/x/tools | require | patch | `v0.9.1` -> `v0.9.3` |

---

### Release Notes

<details>
<summary>BurntSushi/toml</summary>

### [`v1.3.0`](https://togithub.com/BurntSushi/toml/releases/tag/v1.3.0)

[Compare
Source](https://togithub.com/BurntSushi/toml/compare/v1.2.1...v1.3.0)

New features:

-   Support upcoming TOML 1.1

While it looks like TOML 1.1 is mostly stable and I don't expect any
further major changes, there are *NO* compatibility guarantees as it is
*NOT* yet released and *anything can still change*.

To use it, set the `BURNTSUSHI_TOML_110` environment variable to any
value, which can be done either with `os.SetEnv()` or by the user
running a program.

A full list is changes is available in the [TOML ChangeLog]; the two
most notable ones are that newlines and trailing commas are now allowed
in inline tables, and Unicode in bare keys can now be used – this is now
a valid document:

        lëttërs = {
          ä = "a with diaeresis",
          è = "e with accent grave",
        }

[TOML ChangeLog]:
https://togithub.com/toml-lang/toml/blob/main/CHANGELOG.md

- Allow MarshalTOML and MarshalText to be used on the document type
itself, instead of only fields
([#&#8203;383](https://togithub.com/BurntSushi/toml/issues/383)).

Bufixes:

- `\` escapes at the end of line weren't processed correctly in
multiline strings
([#&#8203;372](https://togithub.com/BurntSushi/toml/issues/372)).

- Read over UTF-8 BOM
([#&#8203;381](https://togithub.com/BurntSushi/toml/issues/381)).

- `omitempty` struct tag did not work for pointer values
([#&#8203;371](https://togithub.com/BurntSushi/toml/issues/371)).

- Fix encoding anonymous structs on 32bit systems
([#&#8203;374](https://togithub.com/BurntSushi/toml/issues/374)).

</details>

<details>
<summary>go-git/go-git</summary>

### [`v5.7.0`](https://togithub.com/go-git/go-git/releases/tag/v5.7.0)

[Compare
Source](https://togithub.com/go-git/go-git/compare/v5.6.1...v5.7.0)

#### What's Changed

- \*: Add support for initializing SHA256 repositories by
[@&#8203;pjbgf](https://togithub.com/pjbgf) in
[go-git/go-git#707
- git: add mirror clone option by
[@&#8203;aymanbagabas](https://togithub.com/aymanbagabas) in
[go-git/go-git#735
- git: Add support to ls-remote with peeled references. Fixes
[#&#8203;749](https://togithub.com/go-git/go-git/issues/749) by
[@&#8203;pjbgf](https://togithub.com/pjbgf) in
[go-git/go-git#750
- git: fix cloning with branch name by
[@&#8203;AriehSchneier](https://togithub.com/AriehSchneier) in
[go-git/go-git#755
- git: Worktree, add check to see if file already checked in. Fixes
[#&#8203;718](https://togithub.com/go-git/go-git/issues/718) by
[@&#8203;cbbm142](https://togithub.com/cbbm142) in
[go-git/go-git#719
- git: Worktree, git grep bare repositories by
[@&#8203;aymanbagabas](https://togithub.com/aymanbagabas) in
[go-git/go-git#728
- git: Add Depth to SubmoduleUpdateOptions by
[@&#8203;matejrisek](https://togithub.com/matejrisek) in
[go-git/go-git#754
- git: Testing, Fix tests not cleaning temp folders by
[@&#8203;AriehSchneier](https://togithub.com/AriehSchneier) in
[go-git/go-git#769
- git: remote, add support for a configurable timeout. by
[@&#8203;andrewpollock](https://togithub.com/andrewpollock) in
[go-git/go-git#753
- git: Allow Initial Branch to be configurable by
[@&#8203;techknowlogick](https://togithub.com/techknowlogick) in
[go-git/go-git#764
- storage: filesystem/dotgit, Improve load packed-refs by
[@&#8203;fcharlie](https://togithub.com/fcharlie) in
[go-git/go-git#743
- storage: filesystem, Populate index before use. Fixes
[#&#8203;148](https://togithub.com/go-git/go-git/issues/148) by
[@&#8203;AriehSchneier](https://togithub.com/AriehSchneier) in
[go-git/go-git#722
- plumbing: resolve non-external delta references by
[@&#8203;ZauberNerd](https://togithub.com/ZauberNerd) in
[go-git/go-git#485
- plumbing/transport: fix regression in scp-like match by
[@&#8203;jotadrilo](https://togithub.com/jotadrilo) in
[go-git/go-git#715
- plumbing/transport: Add support for custom proxy settings by
[@&#8203;aryan9600](https://togithub.com/aryan9600) in
[go-git/go-git#744
- \*: small fixes across the codebase by
[@&#8203;pjbgf](https://togithub.com/pjbgf) in
[go-git/go-git#770
- \*: bump github.com/cloudflare/circl from 1.1.0 to 1.3.3 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[go-git/go-git#776
- \*: bump dependencies by [@&#8203;pjbgf](https://togithub.com/pjbgf)
in
[go-git/go-git#748
- \*: bump Go version to 1.18 on go.mod by
[@&#8203;pjbgf](https://togithub.com/pjbgf) in
[go-git/go-git#774
- \*: add Codeql workflow and bump dependencies by
[@&#8203;pjbgf](https://togithub.com/pjbgf) in
[go-git/go-git#775
- ci: fix upstream git build for master branch by
[@&#8203;pjbgf](https://togithub.com/pjbgf) in
[go-git/go-git#739

#### New Contributors

- [@&#8203;ZauberNerd](https://togithub.com/ZauberNerd) made their first
contribution in
[go-git/go-git#485
- [@&#8203;jotadrilo](https://togithub.com/jotadrilo) made their first
contribution in
[go-git/go-git#715
- [@&#8203;fcharlie](https://togithub.com/fcharlie) made their first
contribution in
[go-git/go-git#743
- [@&#8203;AriehSchneier](https://togithub.com/AriehSchneier) made their
first contribution in
[go-git/go-git#755
- [@&#8203;cbbm142](https://togithub.com/cbbm142) made their first
contribution in
[go-git/go-git#719
- [@&#8203;aryan9600](https://togithub.com/aryan9600) made their first
contribution in
[go-git/go-git#744
- [@&#8203;matejrisek](https://togithub.com/matejrisek) made their first
contribution in
[go-git/go-git#754
- [@&#8203;andrewpollock](https://togithub.com/andrewpollock) made their
first contribution in
[go-git/go-git#753
- [@&#8203;techknowlogick](https://togithub.com/techknowlogick) made
their first contribution in
[go-git/go-git#764

**Full Changelog**:
go-git/go-git@v5.6.1...v5.7.0

</details>

<details>
<summary>spdx/tools-golang</summary>

###
[`v0.5.1`](https://togithub.com/spdx/tools-golang/releases/tag/v0.5.1)

[Compare
Source](https://togithub.com/spdx/tools-golang/compare/v0.5.0...v0.5.1)

#### What's Changed

- Add ability to specify JSON output options by
[@&#8203;DmitriyLewen](https://togithub.com/DmitriyLewen) in
[spdx/tools-golang#213
- Fix some optional params: `copyrightText`, `licenseListVersion`,
`packageVerificationCode` by
[@&#8203;lumjjb](https://togithub.com/lumjjb) in
[spdx/tools-golang#215
- Properly output and read the `filesAnalyzed` field in JSON/YAML by
[@&#8203;kzantow](https://togithub.com/kzantow) in
[spdx/tools-golang#210
- Ensure no duplicates in relationships when shortcut fields are used.
by [@&#8203;lumjjb](https://togithub.com/lumjjb) in
[spdx/tools-golang#218

#### New Contributors

- [@&#8203;testwill](https://togithub.com/testwill) made their first
contribution in
[spdx/tools-golang#212
- [@&#8203;DmitriyLewen](https://togithub.com/DmitriyLewen) made their
first contribution in
[spdx/tools-golang#213

**Full Changelog**:
spdx/tools-golang@v0.5.0...v0.5.1

</details>

<details>
<summary>urfave/cli</summary>

### [`v2.25.5`](https://togithub.com/urfave/cli/releases/tag/v2.25.5)

[Compare
Source](https://togithub.com/urfave/cli/compare/v2.25.4...v2.25.5)

#### What's Changed

- Fix:(issue\_1737) Set bool count by taking care of num of aliases by
[@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1740

**Full Changelog**:
urfave/cli@v2.25.4...v2.25.5

### [`v2.25.4`](https://togithub.com/urfave/cli/releases/tag/v2.25.4)

[Compare
Source](https://togithub.com/urfave/cli/compare/v2.25.3...v2.25.4)

#### What's Changed

- Bug/fix issue 1703 by [@&#8203;jojje](https://togithub.com/jojje) in
[urfave/cli#1728
- Fix:(issue\_1734) Show categories for subcommands by
[@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1735
- Fix:(issue\_1610). Keep RunAsSubcommand behaviour as before by
[@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1736
- Fix:(issue\_1731) Add fix for checking if aliases are set by
[@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1732
- Fix func name referenced in doc comment by
[@&#8203;meatballhat](https://togithub.com/meatballhat) in
[urfave/cli#1738

#### New Contributors

- [@&#8203;jojje](https://togithub.com/jojje) made their first
contribution in
[urfave/cli#1728

**Full Changelog**:
urfave/cli@v2.25.3...v2.25.4

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 6am on monday" in timezone
Australia/Sydney, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/google/osv-scanner).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS40OC4yIiwidXBkYXRlZEluVmVyIjoiMzUuMTAyLjEwIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
kyubisation pushed a commit to angular-static-server/angular-static-server that referenced this pull request Aug 20, 2023
@renovate
fix(deps): update module github.com/urfave/cli/v2 to v2.25.7 (#4)
b5bbf2f 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github.com/urfave/cli/v2](https://togithub.com/urfave/cli) | require
| patch | `v2.25.3` -> `v2.25.7` |

---

### Release Notes

<details>
<summary>urfave/cli (github.com/urfave/cli/v2)</summary>

### [`v2.25.7`](https://togithub.com/urfave/cli/releases/tag/v2.25.7)

[Compare
Source](https://togithub.com/urfave/cli/compare/v2.25.6...v2.25.7)

#### What's Changed

- Fix: fix v2 broken tests by
[@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1757
- Fix:(issue\_1755) Ensure that timestamp flag destination is set
correctly by [@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1756

**Full Changelog**:
urfave/cli@v2.25.6...v2.25.7

### [`v2.25.6`](https://togithub.com/urfave/cli/releases/tag/v2.25.6)

[Compare
Source](https://togithub.com/urfave/cli/compare/v2.25.5...v2.25.6)

#### What's Changed

- Fix:(issue\_1668) Add test case for sub command of sub command
completion by [@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1747
- Update dependencies for v2 by
[@&#8203;meatballhat](https://togithub.com/meatballhat) in
[urfave/cli#1749
- Document slice flags as part of examples (v2) by
[@&#8203;carhartl](https://togithub.com/carhartl) in
[urfave/cli#1751

**Full Changelog**:
urfave/cli@v2.25.5...v2.25.6

### [`v2.25.5`](https://togithub.com/urfave/cli/releases/tag/v2.25.5)

[Compare
Source](https://togithub.com/urfave/cli/compare/v2.25.4...v2.25.5)

#### What's Changed

- Fix:(issue\_1737) Set bool count by taking care of num of aliases by
[@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1740

**Full Changelog**:
urfave/cli@v2.25.4...v2.25.5

### [`v2.25.4`](https://togithub.com/urfave/cli/releases/tag/v2.25.4)

[Compare
Source](https://togithub.com/urfave/cli/compare/v2.25.3...v2.25.4)

#### What's Changed

- Bug/fix issue 1703 by [@&#8203;jojje](https://togithub.com/jojje) in
[urfave/cli#1728
- Fix:(issue\_1734) Show categories for subcommands by
[@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1735
- Fix:(issue\_1610). Keep RunAsSubcommand behaviour as before by
[@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1736
- Fix:(issue\_1731) Add fix for checking if aliases are set by
[@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1732
- Fix func name referenced in doc comment by
[@&#8203;meatballhat](https://togithub.com/meatballhat) in
[urfave/cli#1738

#### New Contributors

- [@&#8203;jojje](https://togithub.com/jojje) made their first
contribution in
[urfave/cli#1728

**Full Changelog**:
urfave/cli@v2.25.3...v2.25.4

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/angular-static-server/angular-static-server).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi40My4yIiwidXBkYXRlZEluVmVyIjoiMzYuNDMuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meatballhat meatballhat meatballhat left review comments

@dearchap dearchap dearchap approved these changes

Assignees
No one assigned
Labels
area/v2 relates to / is being considered for v2 kind/bug describes or fixes a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CustomHelpTemplate is ignored with --help flag
3 participants
@jojje @dearchap @meatballhat