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

[unstable option] enum_discrim_align_threshold #3372

Open
scampi opened this issue Feb 13, 2019 · 4 comments
Open

[unstable option] enum_discrim_align_threshold #3372

scampi opened this issue Feb 13, 2019 · 4 comments
Labels
unstable option tracking issue of an unstable option

Comments

@scampi
Copy link
Contributor

scampi commented Feb 13, 2019

Tracking issue for unstable option: enum_discrim_align_threshold

@scampi scampi added the unstable option tracking issue of an unstable option label Feb 13, 2019
@scampi scampi changed the title [unstable option] unstable option: enum_discrim_align_threshold [unstable option] enum_discrim_align_threshold Feb 18, 2019
@najamelan
Copy link

najamelan commented Mar 4, 2020

I was looking at the vertical alignment options of rustfmt. I was suprised by this.

20:

enum Foo {
    A   = 0,
    Bb  = 1,
    RandomLongVariantGoesHere = 10,
    Ccc = 2,
}

enum Bar {
    VeryLongVariantNameHereA = 0,
    VeryLongVariantNameHereBb = 1,
    VeryLongVariantNameHereCcc = 2,
}

I would expect this:

enum Foo {
    A   = 0,
    Bb  = 1,
    RandomLongVariantGoesHere = 10,
    Ccc = 2,
}

enum Bar {
    VeryLongVariantNameHereA   = 0,
    VeryLongVariantNameHereBb  = 1,
    VeryLongVariantNameHereCcc = 2,
}

I think it would make more sense if the threshold would refer to the maximum number of spaces to insert to make things align. The length of the actual text doesn't seem to be very relevant here. It's just when one starts to align very long items with very short items it looks a bit weird to people, but personally I still prefer it most of the times. Another reason to limit vertical alignment might be the resulting total line length, as aligning wrapped lines doesn't make much sense.

I suppose the same issue might exist on other vertical alignment options like struct_field_align_threshold.

Maybe this could fall into a more general category of "align assignment operator"? Or maybe even a vertical_align_max_spaces which could apply to all vertical alignment?

@Leorii
Copy link

Leorii commented Sep 17, 2020

I agree with the above, and would be happy to look into making a PR if it's something that's wanted.

@jmrgibson
Copy link

Any blocking issues on stabilizing this feature? We've been using it for years, pretty happy with it, and its currently the last remaining reason we still have nightly installed.

@calebcartwright
Copy link
Member

Any blocking issues on stabilizing this feature? We've been using it for years, pretty happy with it, and its currently the last remaining reason we still have nightly installed.

See #5365 and #5367 for general information on stabilization. I don't know the answers/details of those items for this specific option, but it's the material that would need to be attested to in order for this option to be judged as ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unstable option tracking issue of an unstable option
Projects
None yet
Development

No branches or pull requests

5 participants