Option to preserve match alignment

[azerupi]

Some people like to align match arms for readability

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"),
 }

And currently it gets reformatted like this

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"),
}

It would be nice if rustfmt offered an option like match_align_arms that can have multiple values:

• Always: rustfmt will try to align all (non-block) match arms
• Preserve: rustfmt will preserve alignment if it is already aligned manually. For matches that are not already aligned, current behavior will be used.
• Never: current behavior

I think Preserve should be the default behavior.

/cc @joelself

[chris-morgan]

I would still expect Preserve/Always to remove three spaces on each line, to take the => as far to the left as it reasonably can:

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"),
 }

(Aside: is rustfmt doing anything as advanced as turning match x { &A => ..., &B => ... } to match *x { A => ..., B => ... }? Another aside: TOMLValue should be TomlValue.)

The really fun question would be how it handles a situation like the following (imagine we just added InlineTable):

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"),
}

[nrc]

re aside: no. I'm keen to do some refactoring-ish changes, I'm not sure where to draw the line though.

[azerupi]

I would still expect Preserve/Always to remove three spaces on each line, to take the => as far to the left as it reasonably can

Absolutely, I just copy pasted the problematic code portion from a PR I made that ran rustfmt on someone else's repo. I didn't pay much attention to where the alignment began. It would indeed make a lot of sense to align as far left as possible.

The really fun question would be how it handles a situation like the following (imagine we just added InlineTable)

I think, for starters, it would be totally reasonable to let the burden be on the programmer. If he wants it aligned, he can do so manually with the guarantee that rustfmt will respect the alignment. (assuming config is set to "Preserve")

Later a "smarter" algorithm could be introduced to detect / guess if there was intent to align and format accordingly. It would be a nice addition but definitely not mandatory for this configuration option.

[joelself]

I only line up match arms sometimes (not even very consistently), so this isn't that important to me. And alining it to the left makes sense. I'm always torn between lining things up or not and if I line things up do I line it up all the way to the left for to the closest tab stop.

Another aside: TOMLValue should be TomlValue

Gah, and I worried so much over that. Is it a language standard? Man, I really don't want to have to make a breaking change to change the casing on a struct.

[stouset]

In general, preserving alignments like these (not just in match arms) would be super useful for rustfmt.

[barafael]

Is there anything new on this? I use tabular in vim which can handle all situations described above but rustfmt doesn't preserve alignment unfortunately.

[nrc]

Is there anything new on this?

No update, sorry. This is a 'some day' feature tbh.

[umurgdk]

Any update on this? I like aligning those match right hand sides.

[dato]

As far as I can tell it’s not possible to even apply #[rustfmt::skip] to the example in the first comment, because they’re expressions? And attributes on expressions are not allowed as per rust-lang/rust#15701.

This is a minimal example of what I need:

fn f(b: bool) -> u8 {
    #[rustfmt::skip]
    match b {
        true  => 1,
        false => 0,
    }
}

Is there an alternative way to specify the attribute? Does rustfmt support any kind of markup via comments, like clang-format?

Right now I’m doing:

fn f(b: bool) -> u8 {
    #[rustfmt::skip]
    let r = match b {
        true  => 1,
        false => 0,
    };
    r
}

but this is coding around the tool instead of the tool helping me. (Could use return as well.)

[ghost]

@dato worksforme (with rust nightly 2018)
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=64dfad7e127c23cdf7f3109fbe5679d3

#![feature(stmt_expr_attributes)]
fn main() {
    fn f(b: bool) -> u8 {
        #[rustfmt::skip]
        match b {
             true => 1,
            false => 0,
        }
    }
}

[dato]

Well, that’s assuming I’m on nightly… (I’m not; I prefer to stay in stable).

[estk]

@nrc, i'm interested in implementing an config option or attribute to do this sort of alignment. Would you think a PR for that would be well received?

[nrc]

@estk that would depend on how invasive the patch is. If it can be done without too much change, then it would be good to have, if it requires a lot of new stuff, then I'd rather not.

[CreepySkeleton]

Any progress on this? #[rustfmt::skip] is not always possible to apply

[Soupertonic]

Any updates on this?

[ijackson]

As well as match arms, here are some other examples whre I would like to be able to preserve regular two-dimensional formatting:

impl ExitStatusExt for process::ExitStatus {
    fn from_raw(raw: i32) -> Self {
        process::ExitStatus::from_inner(From::from(raw))
    }

    fn signal        (&self) -> Option<i32> { self.as_inner().signal()                 }
    fn core_dumped   (&self) -> bool        { self.as_inner().core_dumped()            }
    fn stopped_signal(&self) -> Option<i32> { self.as_inner().stopped_signal()         }
    fn continued     (&self) -> bool        { self.as_inner().continued()              }

    fn into_raw      ( self) -> i32         { self.as_inner().into_raw()       .into() }
}

    // status magic numbers.  So restrict these to Linux.
    #[cfg(target_os = "linux")] t(0x0137f, "stopped (not terminated) by signal: 19");
    #[cfg(target_os = "linux")] t(0x0ffff, "continued (WIFCONTINUED)");

In neither case does it seem like #[rustfmt::skip] is a good fit, since it would have to be applied to each item individually (or to the whole impl or fn).

[ijackson]

Solution suggestion 1 - heuristics

Is it OK for rustfmt to have heuristics? ISTM that this could be detected fairly reliably by a combination of:

• Characters identical to those on the previous line - especially punctuation
• More-than-one space (esp. significantly more than one), other than for indentation

The effect of the heuristic would be preserve whitespace and prevent wrapping in the whole of the affected lines.

Solution suggestion 2 - more specific skip attribute

#[rustfmt::skip] is often too big a hammer. But maybe we could have something like #[rustfmt::skip::tables] which would

• Prevent the insertion of new newlines
• Preserve mutliple whitespace

[slightknack]

Any updates on this?

[marcelarie]

Any updates on this? :)

[abrasumente233]

Any updates on this? Is there a way to format a single line rather than the whole file (like what clang-format does).

[ms-ati]

Canary: is this still the right place to ask about support for vertically aligned match arms?

[tbottomparallel]

Any updates? It looks like multiple people have attempted to implement this over the past six years, but the PRs get very little attention and are ultimately closed.

[jaisw7]

Adding some information on this thread (since it has not been mentioned):

clang-format supports something similar:
https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignconsecutiveassignments

[eugenesvk]

And to add a bit more alignment perfection to the original example

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"),
}

[denosaurtrain]

TL;DR: this issue may be blocked, awaiting clarification from current maintainers about what sort of rewrite is required (if any).

That's IIUC, and I hope that I am wrong! My goal here is only to summarize the issue's status based on my read of the comments. I'm not familiar enough with the code to have an opinion about whether or not it should be blocked.

Question 1: Is this issue blocked by the need for a rewrite?

A contributor attempted to resolve this issue with a PR in 2020, which was closed by maintainer topecongiro with this message:

...we are not likely to add support for vertical alignment without rewriting the current implementation first, as it is a real mess...

That seemingly contradicts (supercedes?) maintainer nrc's 2019 comment:

...that would depend on how invasive the patch is. If it can be done without too much change, then it would be good to have, if it requires a lot of new stuff, then I'd rather not.

topecongiro and nrc are no longer members of the Rustfmt team, but I can't find newer comments from other maintainers. Since topecongiro's comment is the last word from a maintainer, probably folks assume that's the current status of this issue: not (yet) accepting PRs.

Perhaps the current maintainers have a different view about whether this issue is blocked by a rewrite?

Question 2: If yes, what rewrite?

If a rewrite is required, it's not clear to me what sort of rewrite topecongiro had in mind. Some aspect of matches.rs? All of matches.rs? Some broader rustfmt rewrite? What are the problems with the current implementation that need to be addressed by a rewrite? Et cetera.

Perhaps the rewrite is already tracked in a separate issue, which folks here can follow and discuss?

Question 3: Once unblocked, is the option match_arm_align_threshold acceptable?

I don't think Q3 is a blocker because that's the option that maintainer nrc suggested, and I haven't found dissenting opinions. Presumably that option would behave similarly to the existing options, enum_discrim_align_threshold and struct_field_align_threshold.

I mention Q3 because it might surprise some folks if they are expecting a match_align_arms option that accepts Always/Preserve/Never like the issue description. IIUC, the proposed match_arm_align_threshold will not be an "Option to preserve match alignment" per se. However, other issues requesting match arm alignment have been closed as a duplicate of this issue, so perhaps one shouldn't interpret the title too literally.