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

Neatly-aligned multi-line expression folded into a single line #1684

Closed
kornelski opened this issue Jun 15, 2017 · 6 comments
Closed

Neatly-aligned multi-line expression folded into a single line #1684

kornelski opened this issue Jun 15, 2017 · 6 comments

Comments

@kornelski
Copy link

I know it's a tough edge case, but rustfmt is not aware of multi-line formatting patterns/regularities in expressions.

I work with image processing and my code is full of expressions repeated for R, G, and B. Laying out such expressions this way:

let diff = (r as i32 - r2 as i32).pow(2) +
           (g as i32 - g2 as i32).pow(2) +
           (b as i32 - b2 as i32).pow(2);

makes it much easier to understand, spot errors, and edit them (with sublime's multiple carets).

However, rustfmt treats it as just a one long expression, and always unwraps it given a chance:

let diff = (r as i32 - r2 as i32).pow(2) + (g as i32 - g2 as i32).pow(2) + (b as i32 - b2 as i32).pow(2);

Would it be possible to preserve such aligned blocks of code? e.g. if parens and operators are aligned, and/or if there's a small edit distance between the lines.

@kornelski
Copy link
Author

kornelski commented Jun 15, 2017

Here's another case of a compact and IMHO very readable matrix notation exploded into multiple lines:

let custom_primaries = CIExyYTRIPLE {
    Red:   CIExyY {x:0.630, y:0.340, Y:1.0},
    Green: CIExyY {x:0.310, y:0.595, Y:1.0},
    Blue:  CIExyY {x:0.155, y:0.070, Y:1.0},
};

let custom_primaries = CIExyYTRIPLE {
    Red: CIExyY {
        x: 0.630,
        y: 0.340,
        Y: 1.0,
    },
    Green: CIExyY {
        x: 0.310,
        y: 0.595,
        Y: 1.0,
    },
    Blue: CIExyY {
        x: 0.155,
        y: 0.070,
        Y: 1.0,
    },
};

@topecongiro
Copy link
Contributor

Unfortunately rustfmt is not smart enough to do these types of formatting. For now, please try #[rustfmt_skip] or #[cfg_attr(rustfmt, rustfmt_skip)] (more on README.md).

@nrc nrc closed this as completed Nov 2, 2017
@kestred
Copy link
Contributor

kestred commented Sep 22, 2018

This is one of the the larger pains I have with code formatting tools in most languages, where I otherwise love being able to automatically enforce style consistency.

@topecongiro: Is there an architectural reason that rustfmt can't (or would have a difficult time) supporting this behavior?

Would a pull request implementing it be likely to be accepted? (Any other issues specific to preserving well aligned code?)

@nrc
Copy link
Member

nrc commented Sep 24, 2018

@kestred we basically can't do this - the architecture of rustfmt is that it rewrites the AST, not the source, and we don't take hints from the source in general. We found that if we do take hints from the source then we end up with non-idempotency.

@kestred
Copy link
Contributor

kestred commented Sep 24, 2018

@nrc thanks for the explanation-- I hadn't considered the idempotency issue but am glad it is a goal.

Alternative

Rather than attempting to preserve neatly aligned code, it may instead make sense to proactively identify places in source code which could be intentionally neatly aligned. In some cases, this type of pro-active alignment already occurs (notably alignment of "end-of-line" comments for structure definitions and after use statements).

I think (at least) these additional cases should be considered for proactive alignment:

  • N or more repetitions of const, let, or static without interleaved beginning-of-line comments.
  • Array/vec literals with N or more elements (especially of structs or tuples)
  • Within struct literals where N or more fields of the same type are declared subsequently.
  • Argument lists which take N or more arguments of the same type
  • Algebraic expressions
  • Boolean expressions (maybe)

Heuristics to identify when items/expressions would benefit from alignment:

  • A repetition of 3 similar things ([A] items such as let, const or [B] an array literal (etc) with 3 or more elements/arguments)
  • Where each [A] item or [B] expression fits on a single line (<100 characters with indentation) and for [B] possibly the full expression itself does not fit on a single line.
  • Where for [A] each item is of the same type, and [B] each item is of the same type and sufficiently complex to warrant alignment (such as tuples, or structs -- like (2+)-tuples, and structs with 2+ fields).

Reasoning

The reasoning for the change (and consequently, a possible RFC to the style guide) I hope is fairly straightforward, as @kornelski mentioned:

  • It is easier to spot errors
  • It is easier to bulk edit definitions in many common editors (either using multi-caret with sublime, intellij; or using regexp search/replace e.g. atom/sed/ etc where single-line regular expressions are simpler to write and have better support across tolling.

Other Comments

Other, good reasoning, but with a harder solution:

  • For math, it can often be easier to understand --- but also this case may also be the hardest to support; e.g. for a matrix literal we have no idea what the width of the matrix is unless each line is itself a tuple
// Can't easily solve
[1, 0, 0,
 0, 1, 0,
 0, 0, 1]

vs

// Possibly solvable (also addresses the `CIExyY` example from kornelski)
[(1, 0, 0),
 (0, 1, 0),
 (0, 0, 1)]

If the second solution was implemented, well supported, and known to users; then as rustfmt gains adoption I'd hope that libraries such as a hypothetical linear algebra crate would intentionally be designed to support the format-friendly pattern-- (a matrix! [ ... ] macro for example).

@kestred
Copy link
Contributor

kestred commented Sep 24, 2018

Tangentially, I suspect this closed issue may not be the right place for a slightly more involved discussion ^; happy to move somewhere else if it makes sense (rustfmt RFC? internals forum?)

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

No branches or pull requests

4 participants