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

Flag-level Action #1337

Merged
merged 6 commits into from Sep 22, 2022
Merged

Flag-level Action #1337

merged 6 commits into from Sep 22, 2022

Conversation

xwjdsh
Copy link
Member

@xwjdsh xwjdsh commented Feb 15, 2022

What type of PR is this?

  • feature

What this PR does / why we need it:

#1219
I like the idea, but there is no comment from the maintainers, I'd like to have a simple implementation first and then hear your opinions.

Example

package main

import (
	"fmt"
	"os"

	"github.com/urfave/cli/v2"
)

func main() {
	app := &cli.App{
		Action: func(c *cli.Context) error { return nil },
		Flags: []cli.Flag{
			&cli.StringFlag{
				Name: "f",
				Action: func(c *cli.Context, s string) error {
					fmt.Println(s)
					return nil
				},
			},
		},
	}

	err := app.Run(os.Args)
	if err != nil {
		panic(err)
	}
}
## nothing if no flag
go run main.go
## print flag value
go run main.go -f test
#> test

Which issue(s) this PR fixes:

close #1219

@xwjdsh xwjdsh requested a review from a team as a code owner Feb 15, 2022
@meatballhat meatballhat added area/v2 relates to / is being considered for v2 kind/feature describes a code enhancement / feature request labels Apr 21, 2022
@meatballhat meatballhat changed the title feat: flag action Flag-level Action Apr 21, 2022
@meatballhat meatballhat added this to the Release 2.5.0 milestone Apr 21, 2022
@meatballhat meatballhat changed the base branch from master to main Apr 21, 2022
flag.go Outdated Show resolved Hide resolved
app.go Show resolved Hide resolved
Copy link
Contributor

@dearchap dearchap left a comment

Choose a reason for hiding this comment

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

Excellent work. Thanks for completeness. Just add the new interface and then you should be all set

@xwjdsh xwjdsh dismissed a stale review via decd71d Apr 30, 2022
@xwjdsh
Copy link
Member Author

xwjdsh commented Apr 30, 2022

@dearchap Thank you for your review, I updated and rebased main branch.

dearchap
dearchap previously approved these changes Apr 30, 2022
app_test.go Outdated Show resolved Hide resolved
@meatballhat
Copy link
Member

Ack, sorry for the conflicts! I'm happy to attempt to resolve them.

@meatballhat
Copy link
Member

meatballhat commented May 7, 2022

These conflicts are going to be more difficult given the flag generation code introduced in the meantime. Giving it a shot 🤞🏼

The conflicts were straightforward, but follow-up changes will be needed to re-add the Action field to every struct 😭

@meatballhat meatballhat dismissed a stale review via 239dab5 May 7, 2022
@xwjdsh
Copy link
Member Author

xwjdsh commented May 7, 2022

@meatballhat I'll fix it later, that's OK.

@meatballhat
Copy link
Member

@xwjdsh I'm happy to carry this work forward if you're not able ❤️

@meatballhat meatballhat added the status/conflicts contains merge conflicts label May 22, 2022
@xwjdsh xwjdsh dismissed a stale review via 97af685 May 22, 2022
@xwjdsh
Copy link
Member Author

xwjdsh commented May 22, 2022

Sorry for being late, I updated.

Update gopkg.in/yaml.v2 to gopkg.in/yaml.v3 cause v2 seems have an issue that commas cannot be unmarshal correctly, I switched to v3 and the issue resolved.

@meatballhat
Copy link
Member

@xwjdsh I'm sorry about this frustrating feedback loop with having to approve GitHub Actions 😩

@xwjdsh
Copy link
Member Author

xwjdsh commented May 24, 2022

@meatballhat I'm a little bit confused about the CI error, my local environment is normal...

@meatballhat
Copy link
Member

@xwjdsh I'm sorry about this. There's something up with the way that the go.mod is written that seems to be upsetting for go 1.16.x. This isn't the first time this problem has come up in CI. Here's my attempted fix: #1407. Once that's merged, can you incorporate latest main?

@dearchap
Copy link
Contributor

@xwjdsh @meatballhat Is one of you looking at the test failures and conflicts ?

@meatballhat
Copy link
Member

@xwjdsh @meatballhat Is one of you looking at the test failures and conflicts ?

Not me!

@xwjdsh xwjdsh dismissed a stale review via b331176 Sep 15, 2022
@xwjdsh
Copy link
Member Author

xwjdsh commented Sep 15, 2022

@dearchap It's OK now.

// ActionableFlag is an interface that wraps Flag interface and RunAction operation.
type ActionableFlag interface {
Flag
RunAction(*Context) error
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think RunAction might need Flag itself as an arg ?

app_test.go Show resolved Hide resolved
flag_timestamp.go Show resolved Hide resolved
@dearchap dearchap merged commit a81e201 into urfave:main Sep 22, 2022
13 checks passed
@xwjdsh xwjdsh deleted the feat/flag-action branch Sep 22, 2022
@meatballhat meatballhat mentioned this pull request Oct 2, 2022
30 tasks
another-rex added a commit to google/osv.dev that referenced this pull request Oct 6, 2022
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
|
[github.com/CycloneDX/cyclonedx-go](https://togithub.com/CycloneDX/cyclonedx-go)
| require | minor | `v0.6.0` -> `v0.7.0` |
|
[github.com/jedib0t/go-pretty/v6](https://togithub.com/jedib0t/go-pretty)
| require | minor | `v6.3.9` -> `v6.4.0` |
| [github.com/urfave/cli/v2](https://togithub.com/urfave/cli) | require
| minor | `v2.16.3` -> `v2.17.1` |
| [golang.org/x/crypto](https://togithub.com/golang/crypto) | require |
digest | `eccd636` -> `4161e89` |
| [golang.org/x/exp](https://togithub.com/golang/exp) | require | digest
| `439092d` -> `b9f4876` |
| [golang.org/x/term](https://togithub.com/golang/term) | require |
digest | `7de9c90` -> `7a66f97` |

---

### Release Notes

<details>
<summary>CycloneDX/cyclonedx-go</summary>

###
[`v0.7.0`](https://togithub.com/CycloneDX/cyclonedx-go/releases/tag/v0.7.0)

[Compare
Source](https://togithub.com/CycloneDX/cyclonedx-go/compare/v0.6.0...v0.7.0)

#### Changelog

##### Features

-
[`acb9322`](https://togithub.com/CycloneDX/cyclonedx-go/commit/acb932270c1594cb44c052ebeacfe4400c25e30b):
feat: add enum for official media types
([@&#8203;nscuro](https://togithub.com/nscuro))
-
[`2826fe2`](https://togithub.com/CycloneDX/cyclonedx-go/commit/2826fe20711931e40df00c2d9058232b6c4ec8af):
feat: add support for encoding to older spec versions
([#&#8203;51](https://togithub.com/CycloneDX/cyclonedx-go/issues/51))
([@&#8203;nscuro](https://togithub.com/nscuro))
-
[`7a2113a`](https://togithub.com/CycloneDX/cyclonedx-go/commit/7a2113a1d5cdbc27b170ce7a487cc13a108950f5):
feat: raise baseline go version to 1.17
([#&#8203;53](https://togithub.com/CycloneDX/cyclonedx-go/issues/53))
([@&#8203;nscuro](https://togithub.com/nscuro))
-
[`7415143`](https://togithub.com/CycloneDX/cyclonedx-go/commit/7415143fe9af48fafb4bd823cfd1dc1aaea9084e):
feat: return error when parsing unknown spec versions
([@&#8203;nscuro](https://togithub.com/nscuro))
-
[`1655b7d`](https://togithub.com/CycloneDX/cyclonedx-go/commit/1655b7dad8bb4e1cc7c402fac75dddf998dc5621):
feat: set `SpecVersion` when decoding from xml
([@&#8203;nscuro](https://togithub.com/nscuro))
-
[`f97e04a`](https://togithub.com/CycloneDX/cyclonedx-go/commit/f97e04a588544317e666deae16fbff4b4b1a89c5):
feat: update gitpod dockerfile
([@&#8203;nscuro](https://togithub.com/nscuro))

##### Fixes

-
[`ea0d5b7`](https://togithub.com/CycloneDX/cyclonedx-go/commit/ea0d5b79fe245884a46d7537271d0d951d46ad1a):
fix: prevent nesting of `Dependency`
([@&#8203;nscuro](https://togithub.com/nscuro))

##### Building and Packaging

-
[`f43660c`](https://togithub.com/CycloneDX/cyclonedx-go/commit/f43660c92e8aa58b574b90395330c2d423d87e54):
build(deps): bump actions/setup-go from 3.1.0 to 3.2.0
([@&#8203;dependabot](https://togithub.com/dependabot)\[bot])
-
[`2458312`](https://togithub.com/CycloneDX/cyclonedx-go/commit/245831215bceb60ad7c0b237819dadf6fb185a4e):
build(deps): bump actions/setup-go from 3.2.0 to 3.2.1
([@&#8203;dependabot](https://togithub.com/dependabot)\[bot])
-
[`760fae3`](https://togithub.com/CycloneDX/cyclonedx-go/commit/760fae3319dd04b9f95659eca5cada2dcedb885e):
build(deps): bump actions/setup-go from 3.2.1 to 3.3.0
([@&#8203;dependabot](https://togithub.com/dependabot)\[bot])
-
[`4dddf51`](https://togithub.com/CycloneDX/cyclonedx-go/commit/4dddf51ddd4be68d6c0f35adef628acd36eae0ab):
build(deps): bump apache/skywalking-eyes from 0.3.0 to 0.4.0
([@&#8203;dependabot](https://togithub.com/dependabot)\[bot])
-
[`6eb6521`](https://togithub.com/CycloneDX/cyclonedx-go/commit/6eb6521f71afc72eef65bf97033e1197a778ddab):
build(deps): bump github.com/bradleyjkemp/cupaloy/v2 from 2.7.0 to 2.8.0
([@&#8203;dependabot](https://togithub.com/dependabot)\[bot])
-
[`bff00ef`](https://togithub.com/CycloneDX/cyclonedx-go/commit/bff00ef23cf6cdcd520c179f995aabc83cc955b9):
build(deps): bump github.com/stretchr/testify from 1.7.1 to 1.7.2
([@&#8203;dependabot](https://togithub.com/dependabot)\[bot])
-
[`fc11b56`](https://togithub.com/CycloneDX/cyclonedx-go/commit/fc11b56380ce3c547d34194a39c3ef736e6c8397):
build(deps): bump github.com/stretchr/testify from 1.7.2 to 1.7.4
([@&#8203;dependabot](https://togithub.com/dependabot)\[bot])
-
[`f521d75`](https://togithub.com/CycloneDX/cyclonedx-go/commit/f521d75e187d6f2ca3ce289cfa4afbd961b04402):
build(deps): bump github.com/stretchr/testify from 1.7.4 to 1.7.5
([@&#8203;dependabot](https://togithub.com/dependabot)\[bot])
-
[`d5d1ab6`](https://togithub.com/CycloneDX/cyclonedx-go/commit/d5d1ab6ca40e8ef882d6e51e1ebcb4ce72fcb805):
build(deps): bump github.com/stretchr/testify from 1.7.5 to 1.8.0
([@&#8203;dependabot](https://togithub.com/dependabot)\[bot])
-
[`b83bbe8`](https://togithub.com/CycloneDX/cyclonedx-go/commit/b83bbe808f6545654d4e0deecc7e7806a2e49c4e):
build(deps): bump goreleaser/goreleaser-action from 2 to 3
([@&#8203;dependabot](https://togithub.com/dependabot)\[bot])

##### Documentation

-
[`8f8fadf`](https://togithub.com/CycloneDX/cyclonedx-go/commit/8f8fadfe296ad32dd78f513cd7475e81ed85e200):
docs: fix cyclonedx-go version in compatibility matrix
([@&#8203;nscuro](https://togithub.com/nscuro))
-
[`124f2be`](https://togithub.com/CycloneDX/cyclonedx-go/commit/124f2be91434d720dd5d3149d7ab04461405c207):
docs: fix typos ([@&#8203;nscuro](https://togithub.com/nscuro))

##### Others

-
[`5f10aea`](https://togithub.com/CycloneDX/cyclonedx-go/commit/5f10aea00cf46bbe3a4ce66ce2b85bd17576a35c):
refactor: refine spec version conversion to cover more cases
([@&#8203;nscuro](https://togithub.com/nscuro))
-
[`0c2ebff`](https://togithub.com/CycloneDX/cyclonedx-go/commit/0c2ebff85af58497076969010e3bb29f62f19f16):
refactor: separate custom marshalling logic from model
([@&#8203;nscuro](https://togithub.com/nscuro))

</details>

<details>
<summary>jedib0t/go-pretty</summary>

###
[`v6.4.0`](https://togithub.com/jedib0t/go-pretty/releases/tag/v6.4.0)

[Compare
Source](https://togithub.com/jedib0t/go-pretty/compare/v6.3.9...v6.4.0)

### Features

-   **progress**
- option to set Pinned Message(s) above active Trackers (thanks to
[@&#8203;iyear](https://togithub.com/iyear))

</details>

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

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

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

#### What's Changed

- Fix help results inconsistency by
[@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1499

**Full Changelog**:
urfave/cli@v2.17.0...v2.17.1

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

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

#### What's Changed

- Flag-level Action by [@&#8203;xwjdsh](https://togithub.com/xwjdsh) in
[urfave/cli#1337

#### New Contributors

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

**Full Changelog**:
urfave/cli@v2.16.6...v2.17.0

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

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

#### What's Changed

- fix: Context.Set no such flag by
[@&#8203;Torwang1](https://togithub.com/Torwang1) in
[urfave/cli#1497

#### New Contributors

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

**Full Changelog**:
urfave/cli@v2.16.5...v2.16.6

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

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

#### What's Changed

- Fix:(issue\_1197) Set destination field from altsrc for slice flags by
[@&#8203;dearchap](https://togithub.com/dearchap) in
[urfave/cli#1495

**Full Changelog**:
urfave/cli@v2.16.4...v2.16.5

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

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

#### What's Changed

- Accept the `MKDOCS_REMOTE_GITHUB_TOKEN` var as intended by
[@&#8203;meatballhat](https://togithub.com/meatballhat) in
[urfave/cli#1493

**Full Changelog**:
urfave/cli@v2.16.3...v2.16.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, click
this checkbox.

---

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.dev).

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

Co-authored-by: Rex P <106129829+another-rex@users.noreply.github.com>
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 status/conflicts contains merge conflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flag Action
3 participants