Skip to content

rustfmt result are not reproducable with indent_style = "Visual" #2496

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

Closed
alatiera opened this issue Mar 1, 2018 · 4 comments
Closed

rustfmt result are not reproducable with indent_style = "Visual" #2496

alatiera opened this issue Mar 1, 2018 · 4 comments
Labels
bug Panic, non-idempotency, invalid code, etc. only-with-option requires a non-default option value to reproduce

Comments

@alatiera
Copy link

alatiera commented Mar 1, 2018

We want to start using rustfmt on the code base of librsvg. I've stumbled on a bug though where if you run rustfmt once it will change the files, but if run again it will reset them to the previous state. This seem to happen no matter which of those 2 states the code base sits. I am not sure how to describe it better.

This is sort of a blocker since the CI rustfmt test will always fail :(

Using rustfmt 0.3.8

➜ rustfmt -Vv 
0.3.8-nightly ( )

I've made a separate branch to test this.

Steps to reproduce:

  • git clone https://gitlab.gnome.org/alatiera/librsvg.git --branch rustfmt-visual-bug
  • Run cargo fmt --all
  • watch the diff
  • Run cargo fmt --all again
  • Files should be reverted to the same state as in the repo
  • Repeat endlessly
gnomesysadmins pushed a commit to GNOME/librsvg that referenced this issue Mar 1, 2018
Temporarly switch to sidestep rust-lang/rustfmt#2496
so people can start using rustfmt and the CI test succed.
@nrc nrc added bug Panic, non-idempotency, invalid code, etc. only-with-option requires a non-default option value to reproduce labels Mar 1, 2018
gnomesysadmins pushed a commit to GNOME/librsvg that referenced this issue Mar 2, 2018
Temporarly switch to sidestep rust-lang/rustfmt#2496
so people can start using rustfmt and the CI test succed.
@alatiera
Copy link
Author

Fixed as of version 0.7.

@alatiera
Copy link
Author

alatiera commented May 20, 2018

Nevermind it's still happening, I hadn't drunk coffee yet sorry!

@alatiera alatiera reopened this May 20, 2018
linabutler added a commit to mozilla/dogear that referenced this issue Aug 10, 2018
`indent_style = "Visual"` tends to switch between styles. See
rust-lang/rustfmt#2496.
@YaLTeR
Copy link
Contributor

YaLTeR commented Aug 16, 2018

This is still happening on rustfmt 0.99.2-nightly (5c9a2b6 2018-08-07). We've updated the branch to account for rustfmt differences from when the issue was reported. The instructions are the same.

@alatiera also added a CI job that runs cargo fmt --check and cargo fmt twice to showcase the issue. Note, for example, this last diff:

Diff in /builds/alatiera/librsvg/rsvg_internals/src/structure.rs at line 238:
                 Attribute::X => self.x.set(parse("x", value, LengthDir::Horizontal, None)?),
                 Attribute::Y => self.y.set(parse("y", value, LengthDir::Vertical, None)?),
 
-                Attribute::Width => {
-                    self.w.set(parse("width",
-                                     value,
-                                     LengthDir::Horizontal,
-                                     Some(RsvgLength::check_nonnegative)).map(Some)?)
-                }
-                Attribute::Height => {
-                    self.h.set(parse("height",
-                                     value,
-                                     LengthDir::Vertical,
-                                     Some(RsvgLength::check_nonnegative)).map(Some)?)
-                }
+                Attribute::Width => self.w
+                                        .set(parse("width",
+                                               value,
+                                               LengthDir::Horizontal,
+                                               Some(RsvgLength::check_nonnegative)).map(Some)?),
+                Attribute::Height => self.h
+                                         .set(parse("height",
+                                                value,
+                                                LengthDir::Vertical,
+                                                Some(RsvgLength::check_nonnegative)).map(Some)?),
 
                 _ => (),
             }

which in the second run gets reverted back:

Diff in /builds/alatiera/librsvg/rsvg_internals/src/structure.rs at line 242:
                 Attribute::X => self.x.set(parse("x", value, LengthDir::Horizontal, None)?),
                 Attribute::Y => self.y.set(parse("y", value, LengthDir::Vertical, None)?),
 
-                Attribute::Width => self.w
-                                        .set(parse("width",
-                                               value,
-                                               LengthDir::Horizontal,
-                                               Some(RsvgLength::check_nonnegative)).map(Some)?),
-                Attribute::Height => self.h
-                                         .set(parse("height",
-                                                value,
-                                                LengthDir::Vertical,
-                                                Some(RsvgLength::check_nonnegative)).map(Some)?),
+                Attribute::Width => {
+                    self.w.set(parse("width",
+                                     value,
+                                     LengthDir::Horizontal,
+                                     Some(RsvgLength::check_nonnegative)).map(Some)?)
+                }
+                Attribute::Height => {
+                    self.h.set(parse("height",
+                                     value,
+                                     LengthDir::Vertical,
+                                     Some(RsvgLength::check_nonnegative)).map(Some)?)
+                }
 
                 _ => (),
             }

@YaLTeR
Copy link
Contributor

YaLTeR commented Aug 17, 2018

Found a short example: a cycle between

fn main() {
    match option {
        None => some_function(first_reasonably_long_argument,
                              second_reasonably_long_argument),
    }
}

and

fn main() {
    match option {
        None => {
            some_function(first_reasonably_long_argument,
                          second_reasonably_long_argument)
        }
    }
}

YaLTeR added a commit to YaLTeR/rustfmt that referenced this issue Sep 11, 2018
YaLTeR added a commit to YaLTeR/rustfmt that referenced this issue Sep 11, 2018
@nrc nrc closed this as completed in #3012 Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Panic, non-idempotency, invalid code, etc. only-with-option requires a non-default option value to reproduce
Projects
None yet
Development

No branches or pull requests

3 participants