Stream: t-compiler/rust-analyzer

Topic: #6110 Add quick fix to sort methods


Fisher Darling (Dec 10 2020 at 19:53, on Zulip):

In regards to this issue: https://github.com/rust-analyzer/rust-analyzer/issues/6110
The issue is for reordering methods either alphabetically or by trait definition.

I have what seems to be a working method of reordering impl methods and I have a test that "works", it just shows that the Left and the Right are different, but they look exactly the same in the error output. Am I missing some sort of zero-width character, or did I not do the replacement correctly?

Here's the code that does the replacement: https://github.com/fisherdarling/rust-analyzer/blob/6fb5c4b0a70cc9837396ddc164a47802b747229c/crates/assists/src/handlers/reorder_methods_sort.rs#L36

Here's the test I am looking at: https://github.com/fisherdarling/rust-analyzer/blob/master/crates/assists/src/handlers/reorder_methods_sort.rs#L151

Here's the PR (ignore the obvious copy-paste for the other tests): https://github.com/rust-analyzer/rust-analyzer/pull/6809

Here's the test output:

Left:
struct Foo {c: usize}

impl Foo {
    fn add(&mut self, b: usize) { self.c += b }
    fn sub(&mut self, b: usize) { self.c -= b }
    fn ten() -> usize { 10 }
}


Right:
struct Foo {c: usize}

impl Foo {
    fn add(&mut self, b: usize) { self.c += b }
    fn sub(&mut self, b: usize) { self.c -= b }
    fn ten() -> usize { 10 }
}


Diff:
struct Foo {c: usize}

impl Foo {
    fn add(&mut self, b: usize) { self.c += b } // <- red
    fn sub(&mut self, b: usize) { self.c -= b } // <- red
    fn add(&mut self, b: usize) { self.c += b } // <- green
    fn sub(&mut self, b: usize) { self.c -= b } // <- green
    fn ten() -> usize { 10 }
}
Lukas Wirth (Dec 10 2020 at 19:57, on Zulip):

It most likely is trailing whitespace

Lukas Wirth (Dec 10 2020 at 19:57, on Zulip):

as those become invisible in these diffs

Fisher Darling (Dec 10 2020 at 19:58, on Zulip):

Oh man thank you so much!

Fisher Darling (Dec 10 2020 at 19:59, on Zulip):

Jeez, I really should've thought of that

matklad (Dec 10 2020 at 19:59, on Zulip):

I guess we should patch diff-rendering code to render trailing ws visibly

Fisher Darling (Dec 11 2020 at 00:28, on Zulip):

Should I let users sort alphabetically even if it's an impl Trait?

Fisher Darling (Dec 11 2020 at 03:12, on Zulip):

I decided to just have it sort alphabetically if it's an impl Struct and match the def order if it's an impl Trait

matklad (Dec 11 2020 at 09:38, on Zulip):

SGTM!

I am not entirely sure if we should have "sort alphabetically" at all. I think almost all guides suggesting sorting in semantic order

matklad (Dec 11 2020 at 09:39, on Zulip):

OTOH, if you do want to sort alphabetically, I wish the IDE has some way to do that....

Joshua Nelson (Dec 11 2020 at 13:16, on Zulip):

I what I'd want personally is "reorder methods", with an interactive prompt

Joshua Nelson (Dec 11 2020 at 13:16, on Zulip):

Then I can choose the order I want and possibly even the groupings if there are impl blocks

Joshua Nelson (Dec 11 2020 at 13:18, on Zulip):

Because yeah alphabetical doesn't make sense to me, that would put unwrap and expect across the file from each other

Florian Diebold (Dec 11 2020 at 14:25, on Zulip):

I wouldn't want an interactive prompt -- I'd just want a hotkey to move methods up/down

matklad (Dec 11 2020 at 14:42, on Zulip):

That... is actually something we should do

matklad (Dec 11 2020 at 14:42, on Zulip):

https://github.com/intellij-rust/intellij-rust/tree/25f37175094192ba88f345f03b2a4b5b941dd395/src/main/kotlin/org/rust/ide/actions/mover

matklad (Dec 11 2020 at 14:45, on Zulip):

https://github.com/rust-analyzer/rust-analyzer/issues/6823

Last update: Jul 28 2021 at 03:45UTC