Skip to content

Declare & implement space_around_attr_eq config. #4040

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

Merged
merged 4 commits into from
Apr 16, 2020

Conversation

thedodd
Copy link
Contributor

@thedodd thedodd commented Feb 6, 2020

This configuration setting controls the spacing around = when found
inside of attributes. Eg:

#[cfg(target_os="linux")]
// vs
#[cfg(target_os = "linux")]

The config setting space_around_attr_eq takes a bool value.

Closes #4039

todo

  • get some tests in place.

This configuration setting controls the spacing around `=` when found
inside of attributes. Eg:

```rust
 #[cfg(target_os="linux")]
 // vs
 #[cfg(target_os = "linux")]
```

The config setting `space_around_attr_eq` takes a bool value.

Closes rust-lang#4039
@thedodd thedodd force-pushed the 4039-attr-space-around-eq branch from 4f6874b to d0e471f Compare February 6, 2020 04:16
@calebcartwright
Copy link
Member

Awesome! Let me know if you have any questions/issues with the tests (or anything else).

I think there's only one existnig unit test that would need updating (test_dump_default_config), but otherwise it's just the additional tests to cover the new option

@thedodd thedodd requested review from topecongiro and nrc February 6, 2020 05:15
@thedodd
Copy link
Contributor Author

thedodd commented Feb 6, 2020

Ok, I think this PR is ready for an initial review. Tests are passing. Tried to match the other config snippets as closely as possible.

@thedodd
Copy link
Contributor Author

thedodd commented Feb 6, 2020

Looks like the test failure is due to the issue @calebcartwright mentioned. I'll knock that out here soon.

@thedodd thedodd force-pushed the 4039-attr-space-around-eq branch from 1479ff8 to 5ab22d9 Compare February 14, 2020 02:02
@thedodd thedodd force-pushed the 4039-attr-space-around-eq branch from 5ab22d9 to 881305e Compare February 14, 2020 02:28
@thedodd
Copy link
Contributor Author

thedodd commented Feb 14, 2020

Finally ugh! CI kept hitting random failures in external systems. We're green now!

@calebcartwright
Copy link
Member

@topecongiro any concerns/objections with this new config option? It seems reasonable to me and the changes seem good

@topecongiro topecongiro merged commit 9ace1de into rust-lang:master Apr 16, 2020
@topecongiro
Copy link
Contributor

Sorry for the late review. Thanks!

@thedodd thedodd deleted the 4039-attr-space-around-eq branch April 16, 2020 15:51
@thedodd
Copy link
Contributor Author

thedodd commented Apr 16, 2020

@topecongiro no problem. Thank you!

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

Successfully merging this pull request may close these issues.

Config option for spacing around = in Attributes
4 participants