Skip to content

Fix for indention issue#617

Merged
fbricon merged 4 commits intoredhat-developer:masterfrom
jdneo:cs/indention
Aug 29, 2018
Merged

Fix for indention issue#617
fbricon merged 4 commits intoredhat-developer:masterfrom
jdneo:cs/indention

Conversation

@jdneo
Copy link
Copy Markdown
Collaborator

@jdneo jdneo commented Aug 27, 2018

A workaround for issue #557.

For now, just send a range format request after workspace.applyEdit()

Some GIFs for illustration:

Add unimplemented methods

unimplement

Extract to local variable

local

Extract to constant

const

Extract to method

method

Signed-off-by: Sheng Chen <sheche@microsoft.com>
@jdneo jdneo changed the title add a workaround for indention issue Fix for indention issue Aug 27, 2018
Comment thread src/extension.ts Outdated
let startCharacterNum = changes[0].range.start.character;
let endLineNum = startLineNum;
for (let newText of changes) {
endLineNum += newText.newText.split('\n').length - 1;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What if the line delimiter is \r\n? The solution is probably platform dependent

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok, let me change it to split(/\r?\n/)

@fbricon
Copy link
Copy Markdown
Collaborator

fbricon commented Aug 27, 2018

The PR seems to work as expected so far. Except for the potential line-delimiter issue, my main concern is about the use of seemingly random (it's not) index values while manipulating objects.
For future maintainers' sake, please add some more comments to the code as it's not obvious what is happening.

jdneo added 3 commits August 28, 2018 10:24
Signed-off-by: Sheng Chen <sheche@microsoft.com>
Signed-off-by: Sheng Chen <sheche@microsoft.com>
Signed-off-by: Sheng Chen <sheche@microsoft.com>
@jdneo
Copy link
Copy Markdown
Collaborator Author

jdneo commented Aug 29, 2018

@fbricon Updated the code to support the discontinuous condition.

Sometimes the server side may response multiple ranges which are discontinuous. We need to range format each of them separately.

@fbricon
Copy link
Copy Markdown
Collaborator

fbricon commented Aug 29, 2018

Testing that fix, I found that formatting with tabs always uses spaces instead. See eclipse-jdtls/eclipse.jdt.ls#775

@fbricon fbricon merged commit e495906 into redhat-developer:master Aug 29, 2018
@jdneo jdneo deleted the cs/indention branch August 30, 2018 01:11
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.

2 participants