Skip to content

Conversation

@fbricon
Copy link
Collaborator

@fbricon fbricon commented Oct 29, 2018

... by copying the ones used for js/ts.

Fixes #692

@fbricon fbricon changed the title Fix indentation rules by copying the [ones used for js/ts](https://github.com/Microsoft/vscode/blob/41ddd41b71ea2bd5c6e49125beeb3e951f3d9c7c/extensions/typescript-language-features/src/features/languageConfiguration.ts#L17-L18). Fix indentation rules Oct 29, 2018
@fbricon fbricon requested a review from snjeza October 29, 2018 22:56
Copy link
Collaborator

@snjeza snjeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still can reproduce the issue #692.

@fbricon
Copy link
Collaborator Author

fbricon commented Oct 30, 2018

mmm that sucks. Seems like this works with vscode insiders but not the last official release. @snjeza can you double check that please?

@fbricon
Copy link
Collaborator Author

fbricon commented Oct 30, 2018

Looks like the same issue exists in typescript in the last vscode release. Works with insiders (I'm using the same patterns). So I'd say, we ship it anyway. It'll be available to users on regular vscode in early November.

@snjeza
Copy link
Collaborator

snjeza commented Oct 30, 2018

I can also reproduce the issue with code-insiders-1.29.0. I have tested on Windows and Linux.

Version: 1.29.0-insider (system setup)
Commit: 2b06dd1ca31e407abed24f65e1804ae86180401d
Date: 2018-10-30T11:12:35.070Z
Electron: 2.0.12
Chrome: 61.0.3163.100
Node.js: 8.9.3
V8: 6.1.534.41
Architecture: x64

The issue can't be reproduced if we exclude the Java extension.

@fbricon
Copy link
Collaborator Author

fbricon commented Oct 30, 2018

@snjeza FYI I opened microsoft/vscode#62198 upstream

@fbricon
Copy link
Collaborator Author

fbricon commented Nov 10, 2025

@snjeza I'm taking another shot at this old issue, it seems to work now. Please review.

@fbricon
Copy link
Collaborator Author

fbricon commented Nov 18, 2025

@snjeza let me know if you can't review this PR, I'll ask someone else

@snjeza
Copy link
Collaborator

snjeza commented Nov 18, 2025

I still can reproduce the problem.

fbricon7.mp4

@fbricon
Copy link
Collaborator Author

fbricon commented Nov 18, 2025

damn, works for me
move-lines

@fbricon
Copy link
Collaborator Author

fbricon commented Nov 18, 2025

wait your gif shows the problem is fixed on Java (yeah!), not typescript (we don't care). On the same vscode version, it also works for me on typescript.

@snjeza
Copy link
Collaborator

snjeza commented Nov 18, 2025

@fbricon could you try the following code:

public class E1 {
    void test() {
        if (true)
        System.out.println("hello");

        line1;
        line2;
    }
}

and call moveLinesUpAction on line2

@fbricon
Copy link
Collaborator Author

fbricon commented Nov 18, 2025

OK so if the line under the if is not indented, moving line 2 up de-increments it, which is not happening with this PR.

So we'd be swapping the bug from it's happening if line under if is incremented, to it's happening if line under it is not incremented.
BUT, in the meantime we gain better (de)increment on next line after brace-less ifs:

Nov-18-2025.19-07-37.mp4

So still better than the existing IMO

@snjeza snjeza self-requested a review November 18, 2025 18:49
Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an improvement over the existing behaviour. The bug Snjezana found is annoying, but it only affects the next two lines of code and only happens when the code is formatted "wrong".

@fbricon fbricon merged commit 34126d8 into redhat-developer:main Nov 18, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

moveLinesUpAction indentation broken after if statement

3 participants