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

Adding formatting spec #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

emilyaherbert
Copy link

This PR adds a spec that details formatting requirements.

Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sure. Let's wait until FuelLabs/sway#265 is merged to see if @sezna has any feedback on the defaults.

@adlerjohn
Copy link
Contributor

Anything outstanding or can this be merged @emilyaherbert?

@adlerjohn
Copy link
Contributor

The Sway repository is the only one that's adopted this formatting configuration (which could possibly be a chicken-and-egg problem) in this time. I would move to close this rather than require all other repos to adopt a new formatter configuration.

@adlerjohn
Copy link
Contributor

adlerjohn commented Jun 19, 2022

cc @emilyaherbert to decide

@emilyaherbert
Copy link
Author

The Sway repository is the only one that's adopted this formatting configuration

Er, is this right? It looks like the Rust SDK has the same file https://github.com/FuelLabs/fuels-rs/blob/7fb18dfeeb12519fc7e6bd87e8da003d2e8dc9ee/rustfmt.toml

I'm open to closing this RFC (let's ignore the fact that it's been open since September haha 😅), aside from the fact that standardized formatting reduces noise and merge conflicts. As the number of outside contributors grows this will be more important, so if it needs to happen, it should be sooner rather than later IMO.

reorder_imports = true
reorder_modules = true
tab_spaces = 4
use_field_init_shorthand = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a handy feature that helps prevent clippy warnings when enabled

Suggested change
use_field_init_shorthand = false
use_field_init_shorthand = true

tab_spaces = 4
use_field_init_shorthand = false
use_small_heuristics = "Default"
use_try_shorthand = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one also makes sense to enable, no reason to keep the try! macro around

Suggested change
use_try_shorthand = false
use_try_shorthand = true

remove_nested_parens = true
reorder_imports = true
reorder_modules = true
tab_spaces = 4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This auto formats imports into a nice format for diffing (requires nightly fmt though):

Suggested change
tab_spaces = 4
imports_layout = "Vertical"
imports_granularity = "Crate"
tab_spaces = 4
use foo::{
    a,
    b
}

@Voxelot
Copy link
Member

Voxelot commented Oct 25, 2022

cc @xgreenx might have some extra suggestions :)

to be documented in a `rustfmt.toml` file in the top level of each project.
See below for an example of the default values for the file.

```toml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another nice one for organizing imports is group_imports

Suggested change
```toml
```toml
group_imports = "StdExternalCrate"

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

Successfully merging this pull request may close these issues.

4 participants