Skip to content

Commit

Permalink
Fix selective formatting (issue #1127) (#1128)
Browse files Browse the repository at this point in the history
* Fix selective formatting (#1127)

Also adapt mock server to support range formatting and
add tests.

* Bump versions
  • Loading branch information
travkin79 authored Oct 3, 2024
1 parent ca55022 commit 0324785
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 7 deletions.
2 changes: 1 addition & 1 deletion org.eclipse.lsp4e.test/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: Tests for language server bundle (Incubation)
Bundle-SymbolicName: org.eclipse.lsp4e.test;singleton:=true
Bundle-Version: 0.15.18.qualifier
Bundle-Version: 0.15.19.qualifier
Fragment-Host: org.eclipse.lsp4e
Bundle-Vendor: Eclipse.org
Bundle-RequiredExecutionEnvironment: JavaSE-17
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.lsp4e.test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
</parent>
<artifactId>org.eclipse.lsp4e.test</artifactId>
<packaging>eclipse-test-plugin</packaging>
<version>0.15.18-SNAPSHOT</version>
<version>0.15.19-SNAPSHOT</version>

<properties>
<os-jvm-flags /> <!-- for the default case -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.eclipse.lsp4e.tests.mock.MockLanguageServer;
import org.eclipse.lsp4j.Position;
import org.eclipse.lsp4j.Range;
import org.eclipse.lsp4j.ServerCapabilities;
import org.eclipse.lsp4j.TextEdit;
import org.eclipse.ui.IEditorPart;
import org.eclipse.ui.texteditor.ITextEditor;
Expand Down Expand Up @@ -111,6 +112,129 @@ public void testFormatting()

TestUtils.closeEditor(editor, false);
}

@Test
public void testSelectiveFormatting() throws Exception {
String fileContent = "Line 1\nLine 2\n\nText to be formatted.\nLine 5";

final var formattingTextEdits = new ArrayList<TextEdit>();
formattingTextEdits.add(new TextEdit(new Range(new Position(0, 5), new Position(0, 5)), " changed"));
formattingTextEdits.add(new TextEdit(new Range(new Position(3, 10), new Position(3, 11)), "\n"));
MockLanguageServer.INSTANCE.setFormattingTextEdits(formattingTextEdits);

IFile file = TestUtils.createUniqueTestFile(project, fileContent);
IEditorPart editor = TestUtils.openEditor(file);
ITextViewer viewer = LSPEclipseUtils.getTextViewer(editor);

viewer.setSelectedRange(viewer.getDocument().getLineOffset(3), viewer.getDocument().getLineLength(3));

final var formatter = new LSPFormatter();
ISelection selection = viewer.getSelectionProvider().getSelection();
Optional<VersionedEdits> edits = formatter.requestFormatting(viewer.getDocument(), (ITextSelection) selection).get();

// only the edits in the selection range are expected to be returned
assertTrue(edits.isPresent());
assertEquals(1, edits.get().data.size());

editor.getSite().getShell().getDisplay().syncExec(() -> {
try {
edits.get().apply();
} catch (ConcurrentModificationException | BadLocationException e) {
fail(e.getMessage());
}
});

final var textEditor = (ITextEditor) editor;
textEditor.getDocumentProvider().getDocument(textEditor.getEditorInput());
String formattedText = viewer.getDocument().get();
assertEquals("Line 1\nLine 2\n\nText to be\nformatted.\nLine 5", formattedText);

TestUtils.closeEditor(editor, false);
}

@Test
public void testSelectiveFormattingWithEmptySelection() throws Exception {
String fileContent = "Line 1\nLine 2\n\nText to be formatted.\nLine 5";

final var formattingTextEdits = new ArrayList<TextEdit>();
formattingTextEdits.add(new TextEdit(new Range(new Position(0, 6), new Position(0, 6)), " changed"));
formattingTextEdits.add(new TextEdit(new Range(new Position(3, 10), new Position(3, 11)), "\n"));
MockLanguageServer.INSTANCE.setFormattingTextEdits(formattingTextEdits);

IFile file = TestUtils.createUniqueTestFile(project, fileContent);
IEditorPart editor = TestUtils.openEditor(file);
ITextViewer viewer = LSPEclipseUtils.getTextViewer(editor);

viewer.setSelectedRange(viewer.getDocument().getLineOffset(3), 0);

final var formatter = new LSPFormatter();
ISelection selection = viewer.getSelectionProvider().getSelection();
Optional<VersionedEdits> edits = formatter.requestFormatting(viewer.getDocument(), (ITextSelection) selection).get();

assertTrue(edits.isPresent());
assertEquals(formattingTextEdits.size(), edits.get().data.size());

editor.getSite().getShell().getDisplay().syncExec(() -> {
try {
edits.get().apply();
} catch (ConcurrentModificationException | BadLocationException e) {
fail(e.getMessage());
}
});

final var textEditor = (ITextEditor) editor;
textEditor.getDocumentProvider().getDocument(textEditor.getEditorInput());
String formattedText = viewer.getDocument().get();
assertEquals("Line 1 changed\nLine 2\n\nText to be\nformatted.\nLine 5", formattedText);

TestUtils.closeEditor(editor, false);
}

private static ServerCapabilities customServerWithoutRangeFormatting() {
ServerCapabilities capabilities = MockLanguageServer.defaultServerCapabilities();
capabilities.setDocumentRangeFormattingProvider(false);
return capabilities;
}

@Test
public void testSelectiveFormattingWithIncapableServer() throws Exception {
MockLanguageServer.reset(FormatTest::customServerWithoutRangeFormatting);

String fileContent = "Line 1\nLine 2\n\nText to be formatted.\nLine 5";

final var formattingTextEdits = new ArrayList<TextEdit>();
formattingTextEdits.add(new TextEdit(new Range(new Position(0, 6), new Position(0, 6)), " changed"));
formattingTextEdits.add(new TextEdit(new Range(new Position(3, 10), new Position(3, 11)), "\n"));
MockLanguageServer.INSTANCE.setFormattingTextEdits(formattingTextEdits);

IFile file = TestUtils.createUniqueTestFile(project, fileContent);
IEditorPart editor = TestUtils.openEditor(file);
ITextViewer viewer = LSPEclipseUtils.getTextViewer(editor);

viewer.setSelectedRange(viewer.getDocument().getLineOffset(3), viewer.getDocument().getLineLength(3));

final var formatter = new LSPFormatter();
ISelection selection = viewer.getSelectionProvider().getSelection();
Optional<VersionedEdits> edits = formatter.requestFormatting(viewer.getDocument(), (ITextSelection) selection).get();

assertTrue(edits.isPresent());
assertEquals(formattingTextEdits.size(), edits.get().data.size());

editor.getSite().getShell().getDisplay().syncExec(() -> {
try {
edits.get().apply();
} catch (ConcurrentModificationException | BadLocationException e) {
fail(e.getMessage());
}
});

final var textEditor = (ITextEditor) editor;
textEditor.getDocumentProvider().getDocument(textEditor.getEditorInput());
String formattedText = viewer.getDocument().get();
assertEquals("Line 1 changed\nLine 2\n\nText to be\nformatted.\nLine 5", formattedText);

TestUtils.closeEditor(editor, false);
}

@Test
public void testOutdatedFormatting()
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.lsp4e.tests.mock/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: Mock Language Server to test LSP4E
Bundle-SymbolicName: org.eclipse.lsp4e.tests.mock
Bundle-Version: 0.16.11.qualifier
Bundle-Version: 0.16.12.qualifier
Bundle-Vendor: Eclipse LSP4E
Bundle-RequiredExecutionEnvironment: JavaSE-17
Require-Bundle: org.eclipse.lsp4j;bundle-version="[0.23.0,0.24.0)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ public static ServerCapabilities defaultServerCapabilities() {
capabilities.setTypeDefinitionProvider(Boolean.TRUE);
capabilities.setReferencesProvider(true);
capabilities.setDocumentFormattingProvider(true);
capabilities.setDocumentRangeFormattingProvider(true);
capabilities.setCodeActionProvider(Boolean.TRUE);
capabilities.setCodeLensProvider(new CodeLensOptions(true));
capabilities.setDocumentLinkProvider(new DocumentLinkOptions());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Function;
import java.util.stream.Collectors;

import org.eclipse.lsp4j.CodeAction;
import org.eclipse.lsp4j.CodeActionParams;
Expand Down Expand Up @@ -225,7 +226,27 @@ public CompletableFuture<List<? extends TextEdit>> formatting(DocumentFormatting

@Override
public CompletableFuture<List<? extends TextEdit>> rangeFormatting(DocumentRangeFormattingParams params) {
return CompletableFuture.completedFuture(null);
Range range = params.getRange();
List<? extends TextEdit> rangeEdits = mockFormattingTextEdits.stream()
.filter(edit -> inRange(range, edit.getRange())).collect(Collectors.toList());

return CompletableFuture.completedFuture(rangeEdits);
}

private boolean inRange(Range containingRange, Range includedRange) {
if (containingRange == null || includedRange == null) {
throw new IllegalArgumentException();
}

if (containingRange.getStart().getLine() <= includedRange.getStart().getLine()
&& containingRange.getEnd().getLine() >= includedRange.getEnd().getLine()) {
return (containingRange.getStart().getLine() != includedRange.getStart().getLine()
|| containingRange.getStart().getCharacter() <= includedRange.getStart().getCharacter())
&& (containingRange.getEnd().getLine() != includedRange.getEnd().getLine()
|| containingRange.getEnd().getCharacter() >= includedRange.getEnd().getCharacter());
}

return false;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@ public CompletableFuture<Optional<VersionedEdits>> requestFormatting(IDocument d
// range formatting, falling back to a full format if unavailable
long modificationStamp = DocumentUtil.getDocumentModificationStamp(document);
return executor.computeFirst((w, ls) -> w.getServerCapabilitiesAsync().thenCompose(capabilities -> {
if (textSelection.getLength() == 0 && isDocumentRangeFormattingSupported(capabilities)
&& !(isDocumentFormattingSupported(capabilities))) {
if (textSelection.getLength() != 0 && isDocumentRangeFormattingSupported(capabilities)) {
return ls.getTextDocumentService().rangeFormatting(rangeParams)
.thenApply(edits -> new VersionedEdits(modificationStamp, edits, document));
} else {
} else if (isDocumentFormattingSupported(capabilities)) {
return ls.getTextDocumentService().formatting(params)
.thenApply(edits -> new VersionedEdits(modificationStamp, edits, document));
}
return CompletableFuture.<VersionedEdits>completedFuture(null);
}));
}

Expand Down

0 comments on commit 0324785

Please sign in to comment.