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

add Format method #60

Merged
merged 1 commit into from
Jul 7, 2017
Merged

add Format method #60

merged 1 commit into from
Jul 7, 2017

Conversation

maddyblue
Copy link
Contributor

Supports e, E, f, g, G, s, v.

ToSci has been removed and is now just String, which wraps .Text('G').
ToStandard has been removed, but can still be produced using the 'f'
format. The precision field is not supported because that could require
rounding and we don't have a context. Instead, the format methods here
always display the full precision. Although big.Float doesn't implement
's', I think we safely can because the precision is always fully
displayed.

Fixes #19

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@maddyblue
Copy link
Contributor Author

Ping @knz.

Copy link

@knz knz left a comment

Choose a reason for hiding this comment

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

I am so sorry for picking this so late. Please accept my apologies.

Code LGTM, with a nit and a question.

decimal_test.go Outdated
g: "0e+2",
},
}
verbs := []string{"e", "E", "f", "g", "G"}
Copy link

Choose a reason for hiding this comment

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

to make linters happy, please consider using verbs := []string{"%e", "%E", ...} here and Sprintf(v, d) below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

format.go Outdated
if d.Exponent <= 0 && adj >= adjExponentLimit {
return fmtF(buf, d, digits)
}
return fmtE(buf, fmt+'e'-'g', d, digits)
Copy link

Choose a reason for hiding this comment

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

I am not familiar with this syntax in Go fmt + 'e' - 'g' what does it mean? In C that would be equivalent to fmt - 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is copied from https://github.com/golang/go/blob/fcc35147d5940d5b077586c02a97894c3edf870d/src/math/big/ftoa.go?utf8=%E2%9C%93#L148

It needs to convert the either g or G into a e or E since that's what fmtE expects. So it is indeed fmt - 2, but attempting to do that in a way that illustrates the intention.

Copy link

Choose a reason for hiding this comment

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

Thank you! Perhaps a comment would help too :)

// handle like 'G'
format = 'G'
default:
fmt.Fprintf(s, "%%!%c(*apd.Decimal=%s)", format, d.String())
Copy link

Choose a reason for hiding this comment

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

Add a test case for this format too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@knz
Copy link

knz commented Jul 7, 2017

👍 great functionality!

Supports e, E, f, g, G, s, v.

ToSci has been removed and is now just String, which wraps .Text('G').
ToStandard has been removed, but can still be produced using the 'f'
format. The precision field is not supported because that could require
rounding and we don't have a context. Instead, the format methods here
always display the full precision. Although big.Float doesn't implement
's', I think we safely can because the precision is always fully
displayed.

Fixes #19
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