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

Feature request: Customizable SHORT_ITEM_THRESHOLD #5225

Closed
cmpute opened this issue Feb 8, 2022 · 4 comments · Fixed by #5228
Closed

Feature request: Customizable SHORT_ITEM_THRESHOLD #5225

cmpute opened this issue Feb 8, 2022 · 4 comments · Fixed by #5228
Assignees
Labels
feature-request good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted

Comments

@cmpute
Copy link

cmpute commented Feb 8, 2022

This is a follow up issue to #5219. I found that I cannot format

pub const FORMAT_TEST: [u64; 5] = [
    0x0000000000000000,
    0xaaaaaaaaaaaaaaaa,
    0xbbbbbbbbbbbbbbbb,
    0xcccccccccccccccc,
    0xdddddddddddddddd,
];

to something like

pub const FORMAT_TEST: [u64; 5] = [
    0x0000000000000000, 0xaaaaaaaaaaaaaaaa, 0xbbbbbbbbbbbbbbbb,
    0xcccccccccccccccc, 0xdddddddddddddddd,
];

due to the limit set by SHORT_ITEM_THRESHOLD. I tried changing this threshold to 20, and then I managed to get the array formatted as the latter style.

Therefore I would like to request the ability to customize this limit, or at least override it for integer literals specifically. Thanks!

@calebcartwright calebcartwright added good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted labels Feb 11, 2022
@calebcartwright
Copy link
Member

I was actually going to post on #5219 to ask you to open a separate issue, so thanks for doing this! I'm definitely open to supporting this with a config option, provided the default is kept the same and someone is willing to submit a PR. It should be a relatively easy one for anyone that's interested, some relevant code sections below:

  • Add a new config option (feel free to proposal any name)

    • Probably add to the misc block, along with a test entry

      rustfmt/src/config/mod.rs

      Lines 106 to 108 in 5df8c8f

      // Misc.
      remove_nested_parens: bool, true, true, "Remove nested parens";
      combine_control_expr: bool, true, false, "Combine control expressions with function calls";

    rustfmt/src/config/mod.rs

    Lines 590 to 591 in 5df8c8f

    remove_nested_parens = true
    combine_control_expr = true

  • Drop the existing constant and replace it's usage with the new config option

    const SHORT_ITEM_THRESHOLD: usize = 10;

    rustfmt/src/overflow.rs

    Lines 758 to 761 in 0b8ffac

    fn no_long_items(list: &[ListItem]) -> bool {
    list.iter()
    .all(|item| item.inner_as_ref().len() <= SHORT_ITEM_THRESHOLD)
    }

@123vivekr
Copy link
Contributor

@calebcartwright I'm new to Rust and would like to help out. Can you assign this to me?

@ytmimi
Copy link
Contributor

ytmimi commented Feb 12, 2022

@123vivekr feel free to comment @rustbot claim to have the issue assigned to you!

@123vivekr
Copy link
Contributor

@rustbot claim

123vivekr added a commit to 123vivekr/rustfmt that referenced this issue Feb 13, 2022
Allow custom short item threshold values via config.
Closes rust-lang#5225
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants