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

Split out a rustfmt-core crate #1583

Closed
nrc opened this issue May 25, 2017 · 5 comments
Closed

Split out a rustfmt-core crate #1583

nrc opened this issue May 25, 2017 · 5 comments
Assignees
Labels
Milestone

Comments

@nrc
Copy link
Member

nrc commented May 25, 2017

Motivation is to have a crate which can be used as a library (e.g., by RLS, Rustdoc) with minimal deps.

We would split out a rustfmt-bin crate which would include the current bin directory, anything for parsing config files, doing diffs, output to terminal, etc. The rustfmt-core crate would be left with just the core reformatting stuff. The trickiest bit of this is that the core crate will need to contain the actual in-memory config, but the config file parsing should be in the bin crate. I think that means we either have to define the config in two places (not very DRY, but tolerable) or use a fancy build script.

@nrc nrc added the p-high label May 25, 2017
@mjkillough
Copy link
Contributor

The rustfmt-core crate would be left with just the core reformatting stuff. The trickiest bit of this is that the core crate will need to contain the actual in-memory config, but the config file parsing should be in the bin crate. I think that means we either have to define the config in two places (not very DRY, but tolerable) or use a fancy build script.

Could there be a rustfmt-config create, which contains all the config machinery, with the serialization/deserialization bits (and dependencies) hidden behind a non-default feature? (rustfmt-core could then depend on it without the feature, and rustfmt-bin with the feature).

I have done a very quick prototype and it seemed to be possible.

@nrc
Copy link
Member Author

nrc commented Jun 8, 2017

That sounds like a good idea! We could also have the config stuff in rustfmt-core, and have a feature for that crate.

@karyon
Copy link
Contributor

karyon commented Feb 17, 2018

this seems to be fixed?

@nrc
Copy link
Member Author

nrc commented Feb 18, 2018

Probably. But I do want to check that it has the right API, etc. We also need a way to work with workspaces and the Rust repo before this is done.

@topecongiro topecongiro added this to the 2.0.0 milestone Sep 4, 2019
@topecongiro
Copy link
Contributor

AFAIK, we cannot use workspace to separate crates as long as rustfmt is registered as a submodule under the rustc repo. We could still have multiple crates under the same repository without using workspace, however (like we currently do with rustfmt-config-proc-macro).

Starting from 2.0, I would like to re-organize this repo into something like the following:

src/
  cargo-fmt.rs
  rustfmt.rs
  ...and other binaries
rustfmt-config/
  rustfmt-config-proc-macro/
rustfmt-core/
  lib.rs
  expr.rs
  ...and other library files

...that is, we will have three crates inside this repo: rustfmt for binaries, rustfmt-core for rustfmt-as-a-library and rustfmt-config for rustfmt configuration. Or we could have rustfmt-config under rustfmt-core and add a feature for it, like @nrc stated in #1583 (comment).

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

No branches or pull requests

4 participants