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

Formatting big numbers with thousand separator #3949

Closed
thor opened this issue Dec 2, 2019 · 2 comments
Closed

Formatting big numbers with thousand separator #3949

thor opened this issue Dec 2, 2019 · 2 comments
Labels
feature-request good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted wont-fix

Comments

@thor
Copy link

thor commented Dec 2, 2019

After using rustfmt v1.3.0 on the following one line and finding that it didn't change anything,

Utc::now() + Duration::seconds(1000000000)

I then realised later I should have written 1_000_000_000 for readability such as in

Utc::now() + Duration::seconds(1_000_000_000)

Is adding the underscore as a thousand separator something rustfmt would be interested in supporting?

@calebcartwright
Copy link
Member

calebcartwright commented Dec 6, 2019

Sounds reasonable to me, though IMO it'd probably need to be user-controllable via a new config option (to support users that don't want the visual separator).

For anyone interested in implementing, I believe that would be done in rewrite_literal within src/expr.rs, presumably with a new match arm to handle ast::LitKind::Int

rustfmt/src/expr.rs

Lines 1218 to 1231 in 7713d05

pub(crate) fn rewrite_literal(
context: &RewriteContext<'_>,
l: &ast::Lit,
shape: Shape,
) -> Option<String> {
match l.kind {
ast::LitKind::Str(_, ast::StrStyle::Cooked) => rewrite_string_lit(context, l.span, shape),
_ => wrap_str(
context.snippet(l.span).to_owned(),
context.config.max_width(),
shape,
),
}
}

@calebcartwright calebcartwright added good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted labels May 10, 2020
@topecongiro
Copy link
Contributor

I think that this feature is a bit too niche to add support in rustfmt and has too many scenarios where it doesn't work. A similar feature was once added to black and removed - see psf/black#549 for the discussion.

The constant number normalization does not seem to work well in practice as the number tends to have its meaning. This is especially true in the context of the low-level system programming.

As such, I am going to close this as won't fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted wont-fix
Projects
None yet
Development

No branches or pull requests

4 participants