add Integer type casting when loading yaml file #1669

kjeom · 2023-01-27T08:05:36Z

What type of PR is this?

(REQUIRED)

bug

What this PR does / why we need it:

(REQUIRED)

Latest v2 release (2.24.1) resolved this There are two problems when loading yaml (slice, Uint64 flags) #1592
But also we cannot load the Uint64 flag because the casting error
The integer from yaml file is unmarshaled to the int about lower than 9223372036854775807 regardless of the flag type
This PR can accept even the integer that can be casted to the flag type
IsSet()
- The IsSet() returns true after loaded from yaml about all the flags except the slice flags
- This PR can return true about slice flag type also

Which issue(s) this PR fixes:

(REQUIRED)

There are two problems when loading yaml (slice, Uint64 flags) #1592

Special notes for your reviewer:

(fill-in or delete this section)

Testing

(fill-in or delete this section)

Release Notes

(REQUIRED)

…TypeCasting

abitrolly · 2023-01-27T09:13:19Z

@urfave/cli I am not watching it closely, but is there an API to type cast and report errors for just any type? The specificity of int64 sounds like something that should not come out of the box for generic command line handling library.

dearchap · 2023-01-27T13:43:29Z

@kjeom Can you add some tests in altsrc for this condition ?

kjeom · 2023-01-31T15:56:19Z

@urfave/cli I am not watching it closely, but is there an API to type cast and report errors for just any type? The specificity of int64 sounds like something that should not come out of the box for generic command line handling library.

I didn't find the API.

altsrc/map_input_source.go

dearchap · 2023-02-01T14:26:45Z

altsrc/map_input_source.go

+	case reflect.Uint64:
+		uint64Value := v.(uint64)
+		if uint64Value <= math.MaxInt64 {
+			int64Value = int64(uint64Value)


Test to update a test case to cover this

All the patched code is covered.

altsrc/map_input_source.go

kjeom · 2023-02-21T00:48:36Z

@dearchap please finish your review

altsrc/yaml_file_loader_test.go

…o integerTypeCasting

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/go-git/go-git/v5](https://togithub.com/go-git/go-git) | require | minor | `v5.5.2` -> `v5.6.0` | | [github.com/jedib0t/go-pretty/v6](https://togithub.com/jedib0t/go-pretty) | require | patch | `v6.4.4` -> `v6.4.6` | | [github.com/urfave/cli/v2](https://togithub.com/urfave/cli) | require | minor | `v2.24.4` -> `v2.25.0` | | [golang.org/x/exp](https://togithub.com/golang/exp) | require | digest | `c95f2b4` -> `9ff063c` | | [golang.org/x/mod](https://togithub.com/golang/mod) | require | minor | `v0.8.0` -> `v0.9.0` | | [golang.org/x/term](https://togithub.com/golang/term) | require | minor | `v0.5.0` -> `v0.6.0` | | [golang.org/x/tools](https://togithub.com/golang/tools) | require | minor | `v0.5.1-0.20230117180257-8aba49bb5ea2` -> `v0.6.0` | | [golang.org/x/vuln](https://togithub.com/golang/vuln) | require | digest | `4ec8867` -> `d3042fe` | --- ### Release Notes <details> <summary>go-git/go-git</summary> ### [`v5.6.0`](https://togithub.com/go-git/go-git/releases/tag/v5.6.0) [Compare Source](https://togithub.com/go-git/go-git/compare/v5.5.2...v5.6.0) #### What's Changed - Worktree, check for empty parent dirs during Reset (Fixes [#670](https://togithub.com/go-git/go-git/issues/670)) by [@mbohy](https://togithub.com/mbohy) in [go-git/go-git#671 - \*: remove need to build with CGO by [@pjbgf](https://togithub.com/pjbgf) in [go-git/go-git#688 - plumbing: support SSH/X509 signed tags by [@hiddeco](https://togithub.com/hiddeco) in [go-git/go-git#690 **Full Changelog**: go-git/go-git@v5.5.2...v5.6.0 </details> <details> <summary>jedib0t/go-pretty</summary> ### [`v6.4.6`](https://togithub.com/jedib0t/go-pretty/releases/tag/v6.4.6) [Compare Source](https://togithub.com/jedib0t/go-pretty/compare/v6.4.5...v6.4.6) ### Features - **table** - honor border color overrides for title row ([#260](https://togithub.com/jedib0t/go-pretty/issues/260)) ### [`v6.4.5`](https://togithub.com/jedib0t/go-pretty/releases/tag/v6.4.5) [Compare Source](https://togithub.com/jedib0t/go-pretty/compare/v6.4.4...v6.4.5) ### Features - **table** - option to custom color borders/separators ([#259](https://togithub.com/jedib0t/go-pretty/issues/259)) - `table.Style().Color.Border` - `table.Style().Color.Separator` - **text** - handle escape sequences that embed a URL ([#256](https://togithub.com/jedib0t/go-pretty/issues/256)) // thanks [@vsemichev](https://togithub.com/vsemichev) </details> <details> <summary>urfave/cli</summary> ### [`v2.25.0`](https://togithub.com/urfave/cli/releases/tag/v2.25.0) [Compare Source](https://togithub.com/urfave/cli/compare/v2.24.4...v2.25.0) #### What's Changed - Drop support for Go versions before 1.18 by [@meatballhat](https://togithub.com/meatballhat) in [urfave/cli#1667 - add Integer type casting when loading yaml file by [@kjeom](https://togithub.com/kjeom) in [urfave/cli#1669 #### New Contributors - [@kjeom](https://togithub.com/kjeom) made their first contribution in [urfave/cli#1669 **Full Changelog**: urfave/cli@v2.24.4...v2.25.0 </details> <details> <summary>golang/mod</summary> ### [`v0.9.0`](https://togithub.com/golang/mod/compare/v0.8.0...v0.9.0) [Compare Source](https://togithub.com/golang/mod/compare/v0.8.0...v0.9.0) </details> <details> <summary>golang/term</summary> ### [`v0.6.0`](https://togithub.com/golang/term/compare/v0.5.0...v0.6.0) [Compare Source](https://togithub.com/golang/term/compare/v0.5.0...v0.6.0) </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. --- - [ ] 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).

jack.jin added 2 commits January 27, 2023 16:38

added type casting functions

c487ec1

Merge branch 'v2-maint' of https://github.com/urfave/cli into integer…

87f3a50

…TypeCasting

kjeom requested a review from a team as a code owner January 27, 2023 08:05

jack.jin added 2 commits January 27, 2023 17:18

added control for flag value, nil

551ae56

resolve uint testcase fail

285a533

jack.jin added 5 commits January 31, 2023 23:06

added missed casting function

f164c98

added flag set about slice flags for IsSet()

1de7495

added testcases for yaml file loading

6337546

added testcases for coverage

ac2ce9e

fix testcase error

c9c2f90

dearchap reviewed Feb 1, 2023

View reviewed changes

altsrc/map_input_source.go Outdated Show resolved Hide resolved

dearchap reviewed Feb 1, 2023

View reviewed changes

altsrc/map_input_source.go Outdated Show resolved Hide resolved

dearchap reviewed Feb 1, 2023

View reviewed changes

altsrc/map_input_source.go Show resolved Hide resolved

dearchap reviewed Feb 1, 2023

View reviewed changes

altsrc/map_input_source.go Outdated Show resolved Hide resolved

refactor the integer casting code

fb6fcd2

dearchap reviewed Feb 5, 2023

View reviewed changes

altsrc/map_input_source.go Show resolved Hide resolved

dearchap reviewed Feb 5, 2023

View reviewed changes

altsrc/map_input_source.go Show resolved Hide resolved

jack.jin added 5 commits February 7, 2023 15:46

added testcase

4423ae4

resolve testcase error

588cd92

fix testcase

bebf9b7

make casting function private

a105e80

Merge branch 'v2-maint' into integerTypeCasting

afcb85a

kjeom mentioned this pull request Feb 7, 2023

yaml configuration for booting the each nodes klaytn/klaytn#1684

Merged

9 tasks

dearchap approved these changes Feb 21, 2023

View reviewed changes

dearchap reviewed Feb 21, 2023

View reviewed changes

altsrc/yaml_file_loader_test.go Outdated Show resolved Hide resolved

jack.jin added 2 commits February 23, 2023 23:59

rollback package name from altsrc to altsrc_test

f8fe5e8

Merge branch 'integerTypeCasting' of https://github.com/kjeom/cli int…

963fab7

…o integerTypeCasting

dearchap approved these changes Feb 26, 2023

View reviewed changes

dearchap merged commit ce617c1 into urfave:v2-maint Feb 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add Integer type casting when loading yaml file #1669

add Integer type casting when loading yaml file #1669

kjeom commented Jan 27, 2023 •

edited

abitrolly commented Jan 27, 2023 •

edited

dearchap commented Jan 27, 2023

kjeom commented Jan 31, 2023

dearchap Feb 1, 2023

kjeom Feb 15, 2023

kjeom commented Feb 21, 2023

add Integer type casting when loading yaml file #1669

add Integer type casting when loading yaml file #1669

Conversation

kjeom commented Jan 27, 2023 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Testing

Release Notes

abitrolly commented Jan 27, 2023 • edited

dearchap commented Jan 27, 2023

kjeom commented Jan 31, 2023

dearchap Feb 1, 2023

Choose a reason for hiding this comment

kjeom Feb 15, 2023

Choose a reason for hiding this comment

kjeom commented Feb 21, 2023

kjeom commented Jan 27, 2023 •

edited

abitrolly commented Jan 27, 2023 •

edited