Disabling import statement collapsing

[sebthom]

With #818 import statements are collapsed automatically. However since this is an async operation, it happens with a certain delay depending on how fast your LS starts/responds.

This means the moment you open a file imports are not yet collapsed. Then you start working on the source code and suddenly auto collapsing kicks in and rearranges the lines in the editor - this is very irritating.

I therefore suggest to disable auto folding by default and add a UI option to enable it.

[mickaelistria]

delay depending on how fast your LS starts/responds.

Then it seems more like it's a LS issue. In general, LSs are expected to be fast. The default behavior must best serve fast/good language servers.
A preference would be welcome though. It's just that I disagree it's worth turning a feature off by default and disserver fast LSs become some other language servers are not reactice enough.

[sebthom]

I am using the typescript LS that comes with wild web developer.

[mickaelistria]

A possible workaround here is to apply te folding of imports only if the response is received in the ~2 seconds after the editor gets opened.

[jcompagner]

its for me not een the problem once after opening
But for me just typing in the editor also many times relayouts it and because of that it really flickers
and if i am in a place a bit more below and the collapse area is quite large then it can be that the view is suddenly on a completely different scroll position..

this is really annoying behavior that for me makes it almost impossible to really work in ts files at this moment

[brianvfernandes]

In the latest LSP4E update, it's a bit worse than only after opening, I believe this is what @jcompagner was referring to - trying to type directly in the import area results in constants fold/unfold flicker, and if you're trying to type in a multi-line import, you end up with something else entirely; have demonstrated both these in the attached GIF.

This was fold/unfold behavior was not an issue in org.eclipse.lsp4e:0.18.4 which was shipped with Eclipse 2024-03, but does occur if you update to 0.18.7. I haven't had the chance to test this yet, but I wonder if the new behavior has been caused by #925 which was merged for 0.18.5 from what I can see.

[jcompagner]

Can the comimit that causes this not be reverted for a while. Make a release with that. Then reopen or make a new case for folding support?

Because the current state is really not workable, I am forced to use VSC

[brianvfernandes]

I have confirmed that the code in LSPFoldingReconcilingStrategy.reconcile(DirtyRegion, IRegion) appears to be causing the problem I describe. The implementation has changed since the original commit in #925, but the latest iteration still causes the issue; not reproducible if this code is commented out.

@jcompagner what version of the org.eclipse.lsp4e plugin do you have? In 0.18.4 which is what was included in Eclipse's 2024-03 packages, the constant fold/unfold issue I describe wasn't present and so I wonder if there is another issue / cause out there. The "fold on open after delay" issue initially described by @sebthom does exist though.

[rubenporras]

@sebthom , I also think that adding an UI option would be nice.

Regarding the particular problem, should the behaviour not be similar to the JDT, that is, if the user uncollapses the folded region (as he would do for editing the region), the region stays uncollapsed until the user collapses it again? Also before collapsing the first time, we could also check that the cursor is not positioned in the region that will be collapsed.

Would that fix your problem of constants fold/unfold flicker, @brianvfernandes?

[jcompagner]

i have installed now: 0.18.7.202403091716 (that is what installed plugins says)
wierd thing is if i really look at the files i also see: org.eclipse.lsp4e_0.18.7.202403260740

but that doesn't seem to be the one that the install itself reports

[brianvfernandes]

@rubenporras having it work just like JDT would be great. I can't say I've ever had to think about JDT's folding behavior, "it just worked" for me always. I don't believe it suffers from the issue where folding could occur later, it seems to open with the sections correctly folded on open itself (not sure if it's designed in this way, or it's just too fast for me to catch). This would possibly be the ideal solution, but I realize it may not be possible / viable for LSP editors.

Definitely +1 to not automatically folding sections after they have been explicitly unfolded.

Coming to your point about not collapsing if the cursor is within the region that will be collapsed, even if the cursor is below the region, possibly even pages below, wouldn't any folding / unfolding behavior cause the editor to "jump" or flicker as the effective line changes as a result? The original issue reported by @sebthom would remain.

[jcompagner]

ok i did an update and now i am on 0.18.7.202404031149

that one seems a bit better, it still flickers a bit but at least as long as i am not in the import section its not that i am suddenly all over the place. (at first testing)

But changing something in the import area itself, is pretty much impossible because of always collapses again at pretty much every keystroke

[rubenporras]

actually, it looks like we are not recognizing the import section as existing, because we are only collapsing be default new regions in LSPFoldingReconcilingStrategy.updateAnnotation(List<FoldingAnnotation>, List<FoldingAnnotation>, Map<Annotation, Position>, int, Integer, boolean).
@sebthom , do you think you will have same time soon to fix the problem or to implement the preference as a workaround?
If so I would create a new release once we have a workaround/solution to the issue, as I can imagine it can be annoying for the users.

[sebthom]

@rubenporras I won't have time to look into this in the near future as I am mostly away from keyboard the next weeks.

[rubenporras]

@jcompagner , @brianvfernandes , would you have time to add an option so that you would be able to disable import statement collapsing?

If you do not have time for the UI, you could even start with just an option that can be set in the .ini file.

[brianvfernandes]

@rubenporras I'd be happy to take a look but must admit (looking at LSPFoldingReconcilingStrategy) that I don't know immediately how to do this - I'll try to step through this code to figure it out, but any pointers you can share would help. Additionally, given that the import folding behavior makes editing imports unusable (a fundamental break), why make this a preference at all? Perhaps we could simple disable it entirely until the folding is properly fixed?

[rubenporras]

@brianvfernandes , the configuration option is more flexible. I think #971 should be enough to disable it by default.

It does not provide an UI but the user can still enable it in the .ini file.

Would you like to test it?

[mickaelistria]

I think the current path towards resolution is the wrong one.
What is expected for import statement folding is that they happen on editor open, but not later. Instead of fully disabling them, something like suggested in #895 (comment) would provide good result without need for configuration.

[rubenporras]

Yes, I agree that offering an option to disable it is not the solution to the problem reported by @jcompagner and @brianvfernandes, rather a poor man solution for them, but I have looked at the code not quite understand why it happens in the first place.

I understand that it can happen once if the server is slow to answer (as you point out), but I do understand the flicker: it looks to me that the code will not collapse existing folding regions, only new ones.

I do not have time to investigate further, neither anyone else, and since I think that it is nevertheless not wrong to have the option to disable it (one might dislike collapsing no anyway) we can offer it in case it is already useful instead of a real fix.

I hope this explains better my reasoning for adding the configuration option.

[PyvesB]

In my opinion, collapsing imports by default is not necessarily desirable. Whilst this is useful for languages like Java, this is not the case for JavaScript, see eclipse-wildwebdeveloper/wildwebdeveloper#1513 (comment).

I don't believe IDEs like VSCode collapse by default for most languages.

[jcompagner]

I was on a short vacation, so couldn't jump in last week, but i do agree with @PyvesB i think i don't need that folding that doesn't really bring me anything, so a preference that is default off is fine by me
Because if folding is always done "a bit later" then opening the editor and scrolling down it shouldn't suddenly jump that is really annoying, and i am not sure if we really get it that good that we can do it before the user really does or can do anything with the editor...

[nelson777]

I'm experiencing the same problem using WildWebDeveloper in an Angular Project. Is there any workaround yet ? This is really annoying.

[mickaelistria]

While I prefer to have imports folded by default, I'm fine with LSP4E obeying the most consensual request here and just skip default import folding.

[nelson777]

Is there any configuration I can do to disable this in order to avoid the problem ?

[rubenporras]

@nelson777 , if you just want it disabled, you can upgrade to the release done two days ago.

Regards

[rubenporras]

@sebthom , I leave the issue open because the UI option is still lacking.