Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: adhere to lsp spec when calculating offsets #671

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedList;
import java.util.Random;

import org.eclipse.core.filesystem.EFS;
import org.eclipse.core.internal.utils.FileUtil;
Expand Down Expand Up @@ -84,19 +85,33 @@ public void testOpenInEditorExternalFile() throws Exception {
}

@Test
public void testWorkspaceEdit() throws Exception {
public void testWorkspaceEdit_insertText() throws Exception {
TextEdit textEdit = new TextEdit(new Range(new Position(0, 0), new Position(0, 0)), "insert");
AbstractTextEditor editor = applyWorkspaceTextEdit(textEdit);
Assert.assertEquals("insertHere", ((StyledText)editor.getAdapter(Control.class)).getText());
Assert.assertEquals("insertHere", editor.getDocumentProvider().getDocument(editor.getEditorInput()).get());
}

@Test
public void testWorkspaceEdit_WithExaggeratedRange() throws Exception {
TextEdit textEdit = new TextEdit(new Range(new Position(0, 0), new Position(Integer.MAX_VALUE, Integer.MAX_VALUE)), "insert");
AbstractTextEditor editor = applyWorkspaceTextEdit(textEdit);
Assert.assertEquals("insert", ((StyledText)editor.getAdapter(Control.class)).getText());
Assert.assertEquals("insert", editor.getDocumentProvider().getDocument(editor.getEditorInput()).get());
}


private AbstractTextEditor applyWorkspaceTextEdit(TextEdit textEdit) throws CoreException, PartInitException {
IProject p = TestUtils.createProject(getClass().getSimpleName() + System.currentTimeMillis());
IFile f = TestUtils.createFile(p, "dummy", "Here");
IFile f = TestUtils.createFile(p, "dummy"+new Random().nextInt(), "Here");
AbstractTextEditor editor = (AbstractTextEditor)TestUtils.openEditor(f);
WorkspaceEdit workspaceEdit = new WorkspaceEdit(Collections.singletonMap(
LSPEclipseUtils.toUri(f).toString(),
Collections.singletonList(new TextEdit(new Range(new Position(0, 0), new Position(0, 0)), "insert"))));
Collections.singletonList(textEdit)));
LSPEclipseUtils.applyWorkspaceEdit(workspaceEdit);
Assert.assertEquals("insertHere", ((StyledText)editor.getAdapter(Control.class)).getText());
Assert.assertEquals("insertHere", editor.getDocumentProvider().getDocument(editor.getEditorInput()).get());

return editor;
}

@Test
public void testWorkspaceEditMultipleChanges() throws Exception {
IProject p = TestUtils.createProject(getClass().getSimpleName() + System.currentTimeMillis());
Expand Down
25 changes: 24 additions & 1 deletion org.eclipse.lsp4e/src/org/eclipse/lsp4e/LSPEclipseUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* Markus Ofterdinger (SAP SE) - Bug 552140 - NullPointerException in LSP4E
* Rubén Porras Campo (Avaloq) - Bug 576425 - Support Remote Files
* Pierre-Yves Bigourdan <pyvesdev@gmail.com> - Issue 29
* Bastian Doetsch (Snyk Ltd)
*******************************************************************************/
package org.eclipse.lsp4e;

Expand Down Expand Up @@ -181,7 +182,29 @@ public static Position toPosition(int offset, IDocument document) throws BadLoca
}

public static int toOffset(Position position, IDocument document) throws BadLocationException {
return document.getLineOffset(position.getLine()) + position.getCharacter();
var line = position.getLine();
var character = position.getCharacter();

/*
* The LSP spec allow for positions to specify the next line if a line should be
* included completely, specifying the first character of the following line. If
* this is at the end of the document, we therefore take the last document line
* and set the character to line length - 1 (to remove delimiter)
*/
var zeroBasedDocumentLines = Math.max(0, document.getNumberOfLines() - 1);
if (zeroBasedDocumentLines < position.getLine()) {
line = zeroBasedDocumentLines;
character = getLineLength(document, line);
} else {
// We just take the line length to be more forgiving and adhere to the LSP spec.
character = Math.min(getLineLength(document, line), position.getCharacter());
}

return document.getLineOffset(line) + character;
}

private static int getLineLength(IDocument document, int line) throws BadLocationException {
return Math.max(0, document.getLineLength(line));
}

public static boolean isOffsetInRange(int offset, Range range, IDocument document) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ public final CompletableFuture<ApplyWorkspaceEditResponse> applyEdit(ApplyWorksp
final var job = new Job(Messages.serverEdit) {
@Override
public IStatus run(IProgressMonitor monitor) {
LSPEclipseUtils.applyWorkspaceEdit(params.getEdit(), params.getLabel());
UI.getDisplay().syncExec(() -> {
LSPEclipseUtils.applyWorkspaceEdit(params.getEdit(), params.getLabel());
});
bastiandoetsch marked this conversation as resolved.
Show resolved Hide resolved
return Status.OK_STATUS;
}
};
Expand Down