Skip to content

Conversation

@ishai1
Copy link

@ishai1 ishai1 commented Dec 30, 2024

My attempt at a function which will create a default rustfmt file in the current project - following up on #68.

@CeleritasCelery
Copy link
Contributor

Thanks for creating this PR. Why did you pick these particular settings for the rustfmt file?

@ishai1
Copy link
Author

ishai1 commented Dec 31, 2024

The default from copilot. At a quick glance it seemed reasonable but I don't have good insight in to what makes a good rustfmt config so won't pretend it's better than any other config.

@CeleritasCelery
Copy link
Contributor

I don't think we would want to specify rustfmt settings for the user unless there is a de facto standard that most people would want. I would assume that the default settings (i.e. no rustfmt file) would be the most common.

@ishai1
Copy link
Author

ishai1 commented Dec 31, 2024

Fair enough, I think it could stil be nice to provide users with a basic rustfmt file. I changed to create a file with a single entry for edition which is defined to its default value of "2015" - https://rust-lang.github.io/rustfmt/?version=master&search=#edition.

I'll close the PR if it still doesn't seem appropriate to you 🙂.

@CeleritasCelery
Copy link
Contributor

From the docs you linked:

Rustfmt is able to pick up the edition used by reading the Cargo.toml file if executed through the Cargo's formatting tool cargo fmt.

If we specify 2015 in the rustfmt toml file it will override what the user has in their cargo.toml. Which is probably not what user wants. Especially since that is a really old edition that realistically few people are using.

@ishai1
Copy link
Author

ishai1 commented Jan 2, 2025

Oh, good point! That is a pointer to how I should have solved this problem in the first place. Specifically changing rustic-rustfmt-bin to cargo and rustic-rustfmt-args to fmt will then pick up the version from cargo.toml.

@ishai1 ishai1 closed this Jan 2, 2025
@ishai1 ishai1 deleted the generate-rustfmt-toml branch January 2, 2025 19:34
@ishai1
Copy link
Author

ishai1 commented Jan 2, 2025

Thank you for the patient review! 🙂

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.

2 participants