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 selective formatting (issue #1127) #1128

Merged
merged 2 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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
Loading