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

Update style for function definitions #1303

Closed
nrc opened this issue Feb 9, 2017 · 4 comments
Closed

Update style for function definitions #1303

nrc opened this issue Feb 9, 2017 · 4 comments

Comments

@nrc
Copy link
Member

nrc commented Feb 9, 2017

Following rust-lang/style-team#39. This will be mostly possible by changing default values for options, but will also require some tweaks and bug fixes.

@topecongiro
Copy link
Contributor

I am currently working on this. My work is incomplete, though it would be great if I could get any feedback. Is it possible to create a PR for that purpose?

@topecongiro
Copy link
Contributor

topecongiro commented Apr 6, 2017

Working branch

The problems are:

  1. I do not know how to rustfmt files in the tests directory. rustfmt src/lib.rs worked for files in srcs, are there similar way for formating tests?
  2. Most of the failures from cargo test just needs reformatting function definitions. However, I am having trouble handling hard tabs. Specifically,
fn foo(a: i32, a: i32, a: i32, a: i32, a: i32, a: i32, a: i32, a: i32, a: i32, a: i32, a: i32) {}

in tests/source/hard-tabs.rs is formated to

fn foo(a: i32,a: i32,a: i32,a: i32,a: i32,a: i32,a: i32,a: i32,a: i32,a: i32,a: i32,) {

@nrc nrc added the Rfc label Apr 6, 2017
@nrc
Copy link
Member Author

nrc commented Apr 6, 2017

Hi @topecongiro I think this issue is mostly resovled, if you use the rfc rustfmt.toml, then I think we match the RFC pretty well (although perhaps there are still issues in the details). I should have closed it, probably, but I didn't remember it was open.

To answer your questions

1 - you can use https://github.com/rust-lang-nursery/rustfmt/blob/master/bootstrap.sh to reformat the tests, but this only works if there are no config options needed, otherwise you just have to do it manually.

2 - The new formatting should be behind an option(s) and I'm basically not too bothered about interactions between options, except for the RFC ones with each other (or really common options). #1427 is tracking this problem.

BTW, I've noticed you've been sending a lot of PRs recently, thank you! I'm going to be away from work for the next two weeks or so and will be much less responsive here - in particular reviews will probably be slow. Apologies in advance, and please don't be discouraged, I'll get to everything as soon as I can :-)

@topecongiro
Copy link
Contributor

@nrc Thank you for you detailed explanation! Also sorry for asking questions hastily...it is embarassing that both rustfmt.toml and bootstrap.sh are in the root direcotry. I should have checked those files before asking questions or working on this issue.

I am looking forward to contributing to rustfmt. Please enjoy your holidays 😄

@nrc nrc removed the Rfc label Jun 15, 2017
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

No branches or pull requests

2 participants