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] Blank-lines surrounding curly braces #5963

Open
richardpringle opened this issue Nov 17, 2023 · 5 comments
Open

[Feature-Request] Blank-lines surrounding curly braces #5963

richardpringle opened this issue Nov 17, 2023 · 5 comments

Comments

@richardpringle
Copy link

Sorry if this is a dup, I searched for a little bit and only found issues related to blank lines inside curly braces.

I have no idea how hard this would be, but it would be really nice to have a configurable option to add blank lines surrounding anything with curly braces.

with the rule on

fn main() {
  let some_optional_value = map.get(&key); 
  
  if let Some(value) = some_optional_value {
      do_something_with_value(value);
  }
}

with the rule off

fn main() {
  let some_optional_value = map.get(&key);  
  if let Some(value) = some_optional_value {
      do_something_with_value(value);
  }
}

This would include for-loop, loop, while, etc. Any expressions with a scope defined by curly braces.

I'm thinking something akin to this rule: https://eslint.org/docs/latest/rules/padding-line-between-statements

I'm trying to have some automatic way to not get code that looks like this:

        let mut items = HashMap::new();
        for _ in 0..100 {
            let val: Vec<u8> = (0..10).map(|_| rng.borrow_mut().gen()).collect();
            items.insert(generate_key(), val);
        }
        let mut items_ordered: Vec<_> = items.clone();
        items_ordered.sort();
        items_ordered.shuffle(&mut *rng.borrow_mut());
        for (k, v) in items.into_iter() {
            other_map.insert(&k, v)?;
        }

and have the formatted add some empty lines to make the code easier to read

        let mut items = HashMap::new();

        for _ in 0..100 {
            let val: Vec<u8> = (0..10).map(|_| rng.borrow_mut().gen()).collect();
            items.insert(generate_key(), val);
        }

        let mut items_ordered: Vec<_> = items.clone();
        items_ordered.sort();
        items_ordered.shuffle(&mut *rng.borrow_mut());

        for (k, v) in items.into_iter() {
            other_map.insert(&k, v)?;
        }

Obviously, not everyone wants this, but it would be great if it were an option. I'm happy to PR, if someone can point me in the right direction.

@ytmimi
Copy link
Contributor

ytmimi commented Nov 23, 2023

Thanks for reaching out. If I understand correctly, you'd like an option to enforce a consistent number of blank lines between various control flow constructs. Let me know if I've got that right.

Reading the Contributing.md doc should give you a rough overview of how rustfmt works, how to go about adding tests, and also how to add new configuration options. I can't say how easy or difficult it would be to implement such a feature, but I can try to provide some guidance if you decide to get started.

I also want to be upfront and say that this is a fairly low priority feature request so it's unlikely that the rustfmt team would work on this directly, but PRs are welcome. Assuming the new configuration option doesn't add too much complexity to the codebase we might consider adding it.

@ytmimi ytmimi changed the title Blank-lines surrounding curly braces [Feature-Request] Blank-lines surrounding curly braces Nov 23, 2023
@richardpringle
Copy link
Author

Thanks @ytmimi, but how dare you put that p-low on this issue!

Kidding! Of course, this is low priority haha. Any chance you could give some more guidance on where to get started? My inkling is that it'll be something to do with ControlBraceStyle, but any guidance helps!

If the problem is a maze, I'm just looking for you to show me the entrance, and I'll see if I can make my way to the exit.

Thanks in advance!

@ytmimi
Copy link
Contributor

ytmimi commented Nov 27, 2023

You might want to familiarize yourself with how functions get rewritten. That starts in FmtVisitor::visit_fn. visit_block is called next, and internally that makes a call to walk_block_stmts. There's more to it then that, but I think you can follow the control flow from there. hopefully that gives you a place to start investigating.

Some advice; Be generous with println!, trace!, debug!, info! calls while you're developing. You can run rustfmt with RUSTFMT_LOG=rustfmt=DEBUG to get some debug output.

To build and run your local version of rustfmt run cargo run --bin rustfmt --check. I like to use the --check flag so I don't override the content of the file, but you could also use --emit stdout to write the formatted content to stdout. You might also want to create a file with test input for you to work on:

fn main() {
    for i in 1.. {
        println!("{i}");
    }
}

All together you can run your local rustfmt like this:

$: RUSTFMT_LOG=rustfmt=DEBUG cargo run --bin rustfmt path/to/my/file.rs

@richardpringle
Copy link
Author

Thanks @ytmimi, I have a pretty good idea of what I need to do

@richardpringle
Copy link
Author

Hey @ytmimi, think you could take a look when you get a chance?
#5969

I haven't added the docs yet, but I've gone through a first pass at this. If you like my direction, I can add docs and take it out of draft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants