run rustfmt

[azerupi]

I was investigating why the parse function took the parser by move and came across some messed up indentation. So I ran rustfmt on your code base. I used the config file I use for my projects but feel free to tweak it to your needs.

Example of messed up indentation it fixes.

[coveralls]

Changes Unknown when pulling 4285f32 on azerupi:master into * on joelself:master*.

[joelself]

Thank you, I completely agree with the rustformat.toml specifications you added. It looks like some files defaulted to 2 space indentation, while others defaulted to 2 space tabs. I know that for some reason tests in the test directory defaulted to 4 spaces or 4 space tabs for indentation, but I changed that. At first everything looks fine, but then I hit a match where all the arms had been carefully lined up for easier readability where rustfmt and reverted to them to, what I consider, a much harder to read alignment.

 match val {
   &TOMLValue::Integer(_)        => ArrayType::Integer,
   &TOMLValue::Float(_)          => ArrayType::Float,
   &TOMLValue::Boolean(_)        => ArrayType::Boolean,
   &TOMLValue::DateTime(_)       => ArrayType::DateTime,
   &TOMLValue::Array(_)          => ArrayType::Array,
   &TOMLValue::String(_,_)       => ArrayType::String,
   &TOMLValue::InlineTable(_)    => ArrayType::InlineTable,
   &TOMLValue::Table             => panic!("Cannot have a table in an array"),
 }

I find the above easier to understand than:

   match val {
       &TOMLValue::Integer(_) => ArrayType::Integer,
       &TOMLValue::Float(_) => ArrayType::Float,
       &TOMLValue::Boolean(_) => ArrayType::Boolean,
       &TOMLValue::DateTime(_) => ArrayType::DateTime,
       &TOMLValue::Array(_) => ArrayType::Array,
       &TOMLValue::String(_, _) => ArrayType::String,
       &TOMLValue::InlineTable(_) => ArrayType::InlineTable,
       &TOMLValue::Table => panic!("Cannot have a table in an array"),
   }

The worst part is the way it mangles the tests. It seems to add a new line and indent for some opening braces (parentheses, square brackets, curly braces), but never unindents them for closing braces:

assert_eq!(p.expression(" \t[\"δřáƒƭ\".THISKEY  . \tkeythethird] \t#Mèƨƨáϱè Rèƥℓïèδ\n").1,
                   Done("\n",
                        Expression::new(WSSep::new_str(" \t", " \t"),
                                        None,
                                        Some(Rc::new(TableType::Standard(Table::new_str(WSSep::new_str("", ""),
                                                                                        "\"δřáƒƭ\"",
                                                                                        vec![
              WSKeySep::new_str(WSSep::new_str("", ""), "THISKEY"),
              WSKeySep::new_str(WSSep::new_str("  ", " \t"), "keythethird")
            ])))),
                                        Some(Comment::new_str("Mèƨƨáϱè Rèƥℓïèδ")))));

The above example is particularly egregious because it inexplicably unindents 2 lines a lot after an opening square bracket, then unindents further the closing square bracket and a number of parenthesis (all of which are now far further left than their opening counterparts). The result is completely unreadable.

    assert_eq!(p.expression(" \t[\"δřáƒƭ\".THISKEY  . \tkeythethird] \t#Mèƨƨáϱè Rèƥℓïèδ\n").1,
      Done("\n",
        Expression::new(
          WSSep::new_str(" \t", " \t"), None, Some(Rc::new(TableType::Standard(Table::new_str(
            WSSep::new_str("", ""), "\"δřáƒƭ\"", vec![
              WSKeySep::new_str(WSSep::new_str("", ""), "THISKEY"),
              WSKeySep::new_str(WSSep::new_str("  ", " \t"), "keythethird")
            ]
          )))),
          Some(Comment::new_str("Mèƨƨáϱè Rèƥℓïèδ"))
        )
    ));

The current way I do it is a lot easier to make sense of. Ideally I would add a newline and indent for each opening brace (unlike the Some(Rc::new(TableType::Standard(Table::new_str( part which puts 4 opening braces on he same line), but I found it's a trade-off between simplicity and readability and taking up too much vertical space.

Since your rustfmt.toml settings look completely reasonable to me, I can only assume that it's code mangling behavior is default (it doesn't help that they leave the sparse documentation to the command line).

The best I can do without completely mangling half of my code is make sure each file uses 2 spaces instead of tabs, hunt down extraneous trailing spaces, and wrap use statements at a non-arbitrary length.

[joelself]

Closing this in favor of manually correcting indentation, trailing whitespaces, word wrapping and spacing after beginning a comment in 36d1ecf since rustfmt will mess up the formatting of some dev code and completely mangle beyond recognition most of the test code.

[azerupi]

Fair enough :)

rustfmt is not perfect yet. But I think it could be interesting to report the issues you have with it, so that they can be fixed.

I already submitted this one for the match statements: rust-lang/rustfmt/issues/894

For the tests, I have taken a look and I must say that they are really really dense. I am not exactly sure how rustfmt should handle them. I would recommend that you try to move some parts out of the asserts and into helper functions and separate lines. It will make it more readable in my opinion.

Still, I encourage you to create an issue in the rustfmt repo to see what they say.

[joelself]

The problem with the test cases is that I already have helper functions that create the data structures easily, but those are the exact functions that I'm testing. I don't want to copy and paste the existing code because then I wouldn't actually be testing anything and I don't want to rewrite it because it's a just a waste of time that will probably end up looking a lot like the existing function.

It is a real pain to fix up all the tests whenever I make a change to the AST though, which happens a lot.