Forced rustfmt is a roadblock to contributing

Atom does.

If it detects external changes (and you don't have unsaved changes) it automatically loads them, and the load is treated as one frame of the undo stack.

I think as far as their delta rope goes, I think Atom just adds a "Remove everything and then add this entire new content" step. Not sure how well it scales with very large files.

2 Likes

Another example for rustfmt ignoring human judgment and thus making code less symmetric and harder to parse at a glance: here we have 4 very similar methods that ought to be formatted consistently, but rustfmt insists on using one-line-argument-style for 2 of them and multi-line-argument-style for the other 2.

7 Likes
  1. I would suggest that discussions of changes to rustfmt should probably be in a separate thread from discussions of rustfmt usage in CI.

  2. I remain convinced that line length is a terrible heuristic, and line complexity (e.g. "maximum nesting depth") would be a better one, which would be independent of the length of any one token.

2 Likes

Because I just had to do this myself, the command is simply

git filter-branch --tree-filter 'cargo fmt' -- origin/master...

I believe this should even deal with merges etc. correctly, though I have only tested it with a linear history descending from origin/master.

5 Likes

Another example, not rustc related but horrible:

if lr.read_until(b'\r', &mut buffer).context(Io {
    context: "reading response: ",
})? == 0
{
    return ConnectionClosed {}.fail();
}

Not even breaking .context() into a new line will get through..

How about getting rid of the line length limit?

Editors can handle arbitrarily long lines just fine with either horizontal scrollbars or word wrapping depending on the user's preference.

1 Like

I would like to see that change as well, but that would require some additional heuristics for when to break lines.

1 Like

How about inserting a line break anytime the line length exceeds 100 characters? Make it a rustfmt optional parameter if need be.

No please! 100 chars is already too much. It means that all the longs lines wrap on my vertical monitors. It's very hard to read.

6 Likes

I'd also definitely prefer if we kept the line limit. I use a wide monitor, and a particularly skinny font too (ioveska), but I still really appreciate line limits so I can split up my screen into different windows and keep those small.

If I used editor wrapping, it could work, but it'd look much worse than if it was actually formatted well within the line limit. I experience this when working with other languages without as standard of a style, and it isn't great.

2 Likes

Personally I like a maximum line length but I don't think any naive implementation of a limit will be right for all circumstances because, even with the same line length, some statements will end up feeling really bunched up, while other ones will seem too spread out. It depends on both the length of the identifiers and the complexity of the statements.

For example some code is trivial to understand (e.g. chaining very simple iterators) and so can all be put on one line without hassle. But I think more complex code may need some visual room to help guide the reader through it.

To deal with long identifiers you could perhaps have a "fuzzy" line length where the length has some limited flexibility. So a statement with a lot of short identifiers would be broken up into shorter lines than one with a lot of long identifiers. That might still have tricky edge cases though.

And that won't account for the how complex a statement is for the reader. I'm not sure if there is an answer for that. Unless it can somehow uses the writer's line breaks as a hint for how it should format the statement. So another form of fuzziness. But this might mean that the same code may have a slightly different formatting depending upon the person that wrote it.


In short, I think some fuzziness may help with readability but I'm not sure how practical it is. Implementing it well might prove challenging.

3 Likes
`./x.py fmt` does not fix long comments in `src/test/ui/parser/*`, but tidy checks rejects long lines there.

My PR was rejected for testing a long error message, because a //~^ HELP line was too long for tidy.

@kornel that issue predates using rustfmt on the codebase and indeed has nothing to do with rustfmt.

3 Likes

I've always hated rustfmt but I thankfully work with a large rust shop that banned rustfmt outright.

I'd almost suggest an interactive mode vaguely like git add -p but tells you the rules under which it makes each formatting decision and lets you reject those rules one by one. We'd all end up with rustfmt.toml that disabled almost everything ofcourse, well until someone figures out some more relaxed design.

Also, all changes must be local or configured in rustfmt.toml because random mass reflows sound horrible. If one should evaluate the code base then add some mode that suggests changes to rustfmt.toml or at least store decisions in some editable rustfmt.cache file.

You might be interested to hear that there is desire to (eventually) add an operating mode to rustfmt that only reflows the VCS-dirty lines, and leaves the rest of the project alone. That would address at least part of your gripe here.

It's just quite hard to do so, because you have to be able to somehow decide where to stop digging into not-VCS-dirty code. It sound simple -- just "don't change not-VCS-dirty code" -- but in practice it becomes a lot more difficult to actually apply formatting when part of the expression is untouchable.

5 Likes

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.