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

Allow to use integer literals #19

Merged
merged 4 commits into from
Jul 24, 2020
Merged

Allow to use integer literals #19

merged 4 commits into from
Jul 24, 2020

Conversation

licaonfee
Copy link
Contributor

As says in strconv.ParseInt documentation:

"If the base argument is 0, the true base is implied by the string's prefix: 2 for "0b", 8 for "0" or "0o", 16 for "0x", and 10 otherwise. Also, for argument base 0 only, underscore characters are permitted as defined by the Go syntax for integer literals. "

This allow to use hexadecimal , octal or binary to define integer default values. With this PR is posible to use other notations accord its context without break current behaviour for integers.

Examples:

  • define file permisions in octal notation 0644 instead of 420 in decimal
  • define RGB color with 0xFFA07A as LightSalmon
  • define flags in binary like 0b1010100

Breaking changes

This PR require at least go1.13

Fixed

Potential error in 32 bit systems because int and uint are assumed as 64 bit long

@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2020

Codecov Report

Merging #19 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #19   +/-   ##
=======================================
  Coverage   98.26%   98.26%           
=======================================
  Files           2        2           
  Lines         115      115           
=======================================
  Hits          113      113           
  Misses          1        1           
  Partials        1        1           
Impacted Files Coverage Δ
defaults.go 98.21% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0dea786...edb48d0. Read the comment docs.

Copy link
Owner

@creasty creasty left a comment

Choose a reason for hiding this comment

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

Sorry for the delayed response.

defaults.go Outdated
Comment on lines 18 to 23
autoBase = 0

bitSize8 = 8
bitSize16 = 16
bitSize32 = 32
bitSize64 = 64
Copy link
Owner

Choose a reason for hiding this comment

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

I don't really see the point in defining these constants...

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 agree with bitXXSize constants , but autoBase add context to 0 value in base but it is not so significant therefore I will change that

defaults.go Outdated
@@ -58,57 +65,57 @@ func setField(field reflect.Value, defaultVal string) error {
field.Set(reflect.ValueOf(val).Convert(field.Type()))
}
case reflect.Int:
if val, err := strconv.ParseInt(defaultVal, 10, 64); err == nil {
if val, err := strconv.ParseInt(defaultVal, autoBase, strconv.IntSize); err == nil {
Copy link
Owner

Choose a reason for hiding this comment

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

strconv.IntSize

Nice 👍

Copy link
Owner

@creasty creasty left a comment

Choose a reason for hiding this comment

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

Thank you 👍

@@ -1,5 +1,5 @@
language: go
go: 1.11
go: 1.13
Copy link
Owner

Choose a reason for hiding this comment

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

Note to self:
"This PR require at least go1.13"

Per the number literal proposal, Go 1.13 supports a more uniform and modernized set of number literal prefixes.

https://golang.org/doc/go1.13#language

@creasty creasty merged commit 8240537 into creasty:master Jul 24, 2020
@creasty
Copy link
Owner

creasty commented Jul 24, 2020

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.

3 participants