Stream: t-compiler/wg-rls-2.0

Topic: folding of `match` and `if`


centril (Sep 19 2019 at 15:09, on Zulip):

@matklad one thing that I felt regressed when I switched to RA was the folding of match and if... specifically:

if cond {
    block_if
} else {
    block_else
}

is displayed after folding as:

if cond {...
    block_else
}
matklad (Sep 19 2019 at 15:10, on Zulip):

should be easy to tweak here: https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ra_ide_api/src/folding_ranges.rs

matklad (Sep 19 2019 at 15:11, on Zulip):

this definitelly should contain a MATCH_ARM_LIST: https://github.com/rust-analyzer/rust-analyzer/blob/eeac224c3543ce4c869877fe46ea8815286f97d2/crates/ra_ide_api/src/folding_ranges.rs#L84

matklad (Sep 19 2019 at 15:11, on Zulip):

Do you want to open a PR ^^

matklad (Sep 19 2019 at 15:11, on Zulip):

?

centril (Sep 19 2019 at 15:12, on Zulip):

@matklad heh, I'm not familiar with the RA infra; would take some time to learn ^^

centril (Sep 19 2019 at 15:13, on Zulip):

it's just adding that constant to "the list"?

matklad (Sep 19 2019 at 15:14, on Zulip):

for folding match arms -- yeah (well, and updating the test below). For tweaking folding of if, I think there might be something else to do

centril (Sep 19 2019 at 15:15, on Zulip):

@matklad what FoldKind do I need to use in the tests?

centril (Sep 19 2019 at 15:15, on Zulip):

(for match)

matklad (Sep 19 2019 at 15:16, on Zulip):

I think it should be Block as well

centril (Sep 19 2019 at 15:17, on Zulip):

Alright, I'll give it a go by just editing the file on github :P

centril (Sep 19 2019 at 15:17, on Zulip):

Let's hope it compiles :D

centril (Sep 19 2019 at 15:23, on Zulip):

@matklad https://github.com/rust-analyzer/rust-analyzer/pull/1873

centril (Sep 19 2019 at 15:26, on Zulip):

@matklad in the case of if ... { ... } else { ... } it seems to me (from a naive POV) that the problem is that it includes elsein the folding

centril (Sep 19 2019 at 15:27, on Zulip):

should I file an issue?

matklad (Sep 19 2019 at 15:28, on Zulip):

I am not sure, but I think it might be fixed by https://github.com/rust-analyzer/rust-analyzer/pull/1874/commits/184e80007b1d549ebabb6c7a86103648470a8c9f

centril (Sep 19 2019 at 15:29, on Zulip):

@matklad cool; I guess just a test is left to be added?

matklad (Sep 19 2019 at 15:30, on Zulip):

That's a bug in the rust-analyzer <-> lsp conversion layer, and we don't add tests there

centril (Sep 19 2019 at 15:30, on Zulip):

ah

matklad (Sep 19 2019 at 15:30, on Zulip):

the reason is that, to actually properly test this, you need to test against a specific client, and that's pretty brittle to setup

centril (Sep 19 2019 at 15:31, on Zulip):

@matklad yea, guess the testing is just opening VSCode then

matklad (Sep 19 2019 at 15:32, on Zulip):

Yeah, I find that the best testing stragy for such "integration" tests is to do early releases and just let the users find the bugs

Jeremy Kolb (Sep 19 2019 at 15:33, on Zulip):

I love little refactors like that

centril (Sep 19 2019 at 15:35, on Zulip):

@matklad aw; seems my PR failed :frown: -- https://travis-ci.org/rust-analyzer/rust-analyzer/jobs/587070667

matklad (Sep 19 2019 at 15:35, on Zulip):

looks like rustfmt failure

centril (Sep 19 2019 at 15:36, on Zulip):

ah, yes, will fix

centril (Sep 19 2019 at 15:37, on Zulip):

@matklad done

centril (Sep 23 2019 at 01:32, on Zulip):

@matklad updated RA to master now and the else issue persists

matklad (Sep 23 2019 at 07:38, on Zulip):

Interesting! So it looks like Vs Code client doesn't actually take start character/end character into account?

I bet this is because the VS Code UI for folds is petty restricted :(

matklad (Sep 23 2019 at 07:38, on Zulip):

VS Code folds like this:

pasted image

matklad (Sep 23 2019 at 07:39, on Zulip):

IntelliJ folds like this:

pasted image

matklad (Sep 23 2019 at 07:40, on Zulip):

Apparently, VS Code simply can't fold the range in the middle. So, to work like vs code, we should supply an "interior" of a block as a folding range, while, for intellij-like behavior, we should specify the exteriour plus a replacement string

matklad (Sep 23 2019 at 08:07, on Zulip):

@centril filed https://github.com/microsoft/language-server-protocol/issues/827.

matklad (Sep 23 2019 at 08:08, on Zulip):

I think I wouldn't, in principle, be against the fix that makes rust behave like typescript, on the LSP layer, though I suspect it might be more involved than just "add/subtract one to the folding range boundary"

centril (Sep 23 2019 at 12:58, on Zulip):

thanks for investigating!

lqd (Sep 23 2019 at 13:44, on Zulip):

@matklad are you sure about your screenshot ? here's how VS Code looks on my machine, notice the "..." ellipse in the folded loop here. It looks the same for other blocks like your if example, or in javacript for example

matklad (Sep 23 2019 at 13:46, on Zulip):

@lqd this is exactly what I am talking about: the } is on the new line

matklad (Sep 23 2019 at 13:46, on Zulip):

the elipsis is not shown in my screenshot because the current line is highlighted

lqd (Sep 23 2019 at 13:46, on Zulip):

haha ok

Last update: Nov 12 2019 at 16:50UTC