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

flags/pflags: slice/map flags concat/merge & README update #94

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

dfinkel
Copy link
Contributor

@dfinkel dfinkel commented Jun 7, 2024

Update README

It's been a while since it was last updated, and I felt weird adding a
section explaining slice parsing without fixing the complete staleness
of the readme's examples.

  • Removed Fill() calls, which are deprecated
  • Updated ez package references to use Params
  • Removed type-asserts on View() calls
  • Removed Events() calls, and updated to the less convoluted
    callback-based change-notification mechanism.

flags/pflags: slice/map flags concat/merge

Instead of always using the last flag value referenced, merge all values
present.

Copy link
Contributor

@sergiosalvatore sergiosalvatore left a comment

Choose a reason for hiding this comment

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

Looks good -- just a couple nits.

README.md Outdated
if dialsErr != nil {
// error handling
}

// Fill will deep copy the fully-stacked configuration into its argument.
// View returns a pointer to the fully stacked configuration file
// The stacked configuration is populated from the config file, environment
// variables and commandline flags. Can alternatively use
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: dangling sentence here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I knew I saw that fragment someplace, but lost track of it 🙁

Removed.

README.md Outdated
As a result, a commandline with `"--a=b", "--a=c"` may be parsed as `[]string{b,c}`.

#### Maps
Maps are parsed like Slices, with the addition of `:` separators between keys and values. ([`strconv.Unquote`](https://pkg.go.dev/strconv#Unquote)-compatible quoting is mandatory for more complicated strins as well)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
Maps are parsed like Slices, with the addition of `:` separators between keys and values. ([`strconv.Unquote`](https://pkg.go.dev/strconv#Unquote)-compatible quoting is mandatory for more complicated strins as well)
Maps are parsed like Slices, with the addition of `:` separators between keys and values. ([`strconv.Unquote`](https://pkg.go.dev/strconv#Unquote)-compatible quoting is mandatory for more complicated strings as well)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

README.md Outdated
}
conf, serial := d.ViewVersion()

// you can get notified whenever the config changes by registering a callback
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: extra indentation

Suggested change
// you can get notified whenever the config changes by registering a callback
// You can get notified whenever the config changes by registering a callback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the capitalization.

It was actually worse than extra indentation. Somehow, I ended up with spaces instead of tabs, and a tabstop of 4 for some of the new lines. I've fixed that.

It's been a while since it was last updated, and I felt weird adding a
section explaining slice parsing without fixing the complete staleness
of the readme's examples.

 - Removed Fill() calls, which are deprecated
 - Updated ez package references to use Params
 - Removed type-asserts on View() calls
 - Removed Events() calls, and updated to the less convoluted
   callback-based change-notification mechanism.
Instead of always using the last flag value referenced, merge all values
present.
@dfinkel dfinkel force-pushed the multiple_flags_concat_slices_merge_maps branch from 50a0e2b to 839367a Compare June 7, 2024 20:17
@dfinkel dfinkel merged commit 402b482 into master Jun 7, 2024
8 checks passed
@dfinkel dfinkel deleted the multiple_flags_concat_slices_merge_maps branch June 7, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants