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

rustfmt configuration and canonicalization #133

Closed
toastronics opened this issue Oct 5, 2018 · 2 comments
Closed

rustfmt configuration and canonicalization #133

toastronics opened this issue Oct 5, 2018 · 2 comments

Comments

@toastronics
Copy link
Contributor

I'm interested in contributing to sprs, but immediately ran into a contribution blocker in the form of the outdated rustfmt.toml file.

ideal_width has been renamed comment_width:

rust-lang/rustfmt#1321 (comment)

where_indent has been removed. There is not an obvious replacement which behaves in the same way as where_indent = "Inherit".

It's hard now to find information about what where_indent = "Inherit" is supposed to do. The discussion here may be relevant to why this option is gone, if the intent of "Inherit" was to preserve formatting:

rust-lang/rustfmt#2555

The existing option indent_style seems to be related to this, with choices "Block" or "Visual". Neither is a perfect fit for the existing style in sprs, but "Visual" seems to somewhat more closely reflect the existing style.

Both comment_width and indent_style are unstable options, requiring cargo +nightly rustfmt. My suggestion/preference would be to stick with stable options only; this both minimizes the possibility of incompatible changes in rustfmt.toml options as well as allows for naming a 'canonical' rustfmt which does not change too often (until there is a non-preview version of rustfmt, a good choice for the canonical version is probably the latest stable Rust version of rustfmt-preview).

@vbarrielle
Copy link
Collaborator

Hello,

First thanks for your interest in sprs. I must admit that the current rustfmt.toml file is quite old, I tried it some time ago but was not satisfied with the result (too weird output for function declarations with lots of generic parameters). This was quite a while ago, so I guess it probably has improved.

I do not have strong pre-conceptions regarding what the formatting should be, except for the line length, where I would like to see the 80 characters limit enforced as best as possible.

However I do think having the code in a consistent, rustfmt enforced style would be beneficial, so I should maybe try rustfmt again. Of course, if you have more experience than I do with rustfmt and want to propose a config file, please do.

I agree with you that targeting the stable version of rustfmt-preview should be the way to go.

On a side note: which area of sprs would you like to contribute to? I do not currently have much time to spend improving sprs by coding myself, but I would gladly help if you have some interesting improvements or want to tackle existing issues.

@toastronics
Copy link
Contributor Author

toastronics commented Oct 6, 2018

Hi @vbarrielle, thanks for your response. Other than enforcing your preference for 80-character line width, there aren't any changes from the default rustfmt settings that I would choose from the stable options. Please see the pull request I have opened updating the rustfmt.toml and applying rustfmt.

I'm interested in starting with some of the E-easy issues - #26 and #93 sound interesting to look at. Longer term I'm not sure where my focus would be but I'm considering tackling #117.

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