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

Use proc macro to define Config #3873

Closed
wants to merge 11 commits into from

Conversation

topecongiro
Copy link
Contributor

This PR implements a proc macro for defining Config struct, which replaces the current define_config macro.

The ultimate goal is to define the config like the following:

#[rustfmt_config]
pub struct Config {
    /// A maximum width of each line.
    ///
    /// - Default value: 100
    /// - Possible values: any positive integers
    /// - Stable: Yes
    ///
    /// See also err_on_line_overflow.
    ///
    /// ## Example
    ///
    /// ### Input
    ///
    /// ```rust
    /// // An example code.
    /// ```
    ///
    /// ### 100 (default)
    /// ```rust
    /// // An example code formatted with max_width = 100.
    /// ```
    ///
    /// ### 80 (default)
    /// ```rust
    /// // An example code formatted with max_width = 80.
    /// ```
    #[rustfmt::config(default = 100, stable = "1.0.0")]
    max_width: usize,

    // ... other configs
}

and let the config proc macro generate Configurations.md and test code under test/source/configs/ and tests/target/configs/. This PR is the first step toward the goal.

Other minor benefits include the following:

  • a better interface for setters
  • internal config options are now private

Since the Config is part of the public API of rustfmt-as-a-lib, and this PR changes its interface, we should merge this as a part of the preparation for 2.0 release. I am creating a PR at the moment, however, for early feedback.

@topecongiro topecongiro added this to the 2.0.0 milestone Oct 19, 2019
impl std::fmt::Display for ConfigError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
match self {
ConfigError::UnstableConfigOption(unstables) => {
Copy link
Member

Choose a reason for hiding this comment

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

This will resolve #3872 correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately no, IIRC the issue is that we are parsing the same rustfmt.toml and emitting errors multiple times.

@@ -243,11 +242,11 @@ fn format_string(input: String, options: GetOptsOptions) -> Result<i32, FailureE
let (mut config, _) = load_config(Some(Path::new(".")), Some(options.clone()))?;

// emit mode is always Stdout for Stdin.
config.set().emit_mode(EmitMode::Stdout);
config.set().verbose(Verbosity::Quiet);
config.set_emit_mode(EmitMode::Stdout);
Copy link
Member

Choose a reason for hiding this comment

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

👍 definitely a friendly interface on getters/setters

src/config/mod.rs Outdated Show resolved Hide resolved
@topecongiro
Copy link
Contributor Author

I will try this again after 2.0 release 🤷‍♂️

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