Skip to content

Commit

Permalink
GH-1154: removed logic to handle client-side magic indentation, no lo…
Browse files Browse the repository at this point in the history 
…nger necessary as completions are marked as-is for indentation
  • Loading branch information
@martinlippert
martinlippert committed Apr 23, 2024
1 parent 9b35be3 commit 029cc73
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 127 deletions.
2 changes: 1 addition & 1 deletion concourse/tasks/fatjars-language-servers.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ else
fi

cd ${sources}
xvfb-run ./mvnw clean install -DargLine="-Dlsp.completions.indentation.enable=true -Dlsp.yaml.completions.errors.disable=true"
xvfb-run ./mvnw clean install -DargLine="-Dlsp.yaml.completions.errors.disable=true"

# Copy fatjars to `out` directory
timestamp=`date -u +%Y%m%d%H%M`
Expand Down
1 change: 0 additions & 1 deletion ...ork.tooling.boot.ls/src/org/springframework/tooling/boot/ls/SpringBootLanguageServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ private List<String> getJVMArgs() {

// args.add("-Xdebug");
// args.add("-Xrunjdwp:server=y,transport=dt_socket,address=1044,suspend=n");
args.add("-Dlsp.completions.indentation.enable=true");
args.add("-Xmx1024m");
args.add("-XX:TieredStopAtLevel=1");

Expand Down
1 change: 0 additions & 1 deletion ...framework.tooling.bosh.ls/src/org/springframework/tooling/bosh/ls/BoshLanguageServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ public BoshLanguageServer() {
"application.properties",
Arrays.asList(
"-Dlsp.lazy.completions.disable=true",
"-Dlsp.completions.indentation.enable=true",
"-XX:TieredStopAtLevel=1"
)
);
Expand Down
1 change: 0 additions & 1 deletion .../springframework/tooling/cloudfoundry/manifest/ls/CloudFoundryManifestLanguageServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ public CloudFoundryManifestLanguageServer() {
"application.properties",
Arrays.asList(
"-Dlsp.lazy.completions.disable=true",
"-Dlsp.completions.indentation.enable=true",
"-XX:TieredStopAtLevel=1"
)
);
Expand Down
1 change: 0 additions & 1 deletion ...ng.concourse.ls/src/org/springframework/tooling/concourse/ls/ConcourseLanguageServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ public ConcourseLanguageServer() {
"application.properties",
Arrays.asList(
"-Dlsp.lazy.completions.disable=true",
"-Dlsp.completions.indentation.enable=true",
"-XX:TieredStopAtLevel=1"
)
);
Expand Down
54 changes: 6 additions & 48 deletions ...framework/ide/vscode/commons/languageserver/completion/VscodeCompletionEngineAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@
import org.springframework.ide.vscode.commons.languageserver.util.SimpleTextDocumentService;
import org.springframework.ide.vscode.commons.languageserver.util.SortKeys;
import org.springframework.ide.vscode.commons.protocol.CursorMovement;
import org.springframework.ide.vscode.commons.util.BadLocationException;
import org.springframework.ide.vscode.commons.util.Renderable;
import org.springframework.ide.vscode.commons.util.StringUtil;
import org.springframework.ide.vscode.commons.util.text.IRegion;
import org.springframework.ide.vscode.commons.util.text.TextDocument;

Expand Down Expand Up @@ -399,7 +397,7 @@ private static void resolveItem(TextDocument doc, ICompletionProposal completion

private void resolveMainEdit(TextDocument doc, ICompletionProposal completion, CompletionItem item) {
AtomicBoolean usedSnippets = new AtomicBoolean();
Optional<TextEdit> mainEdit = adaptEdits(doc, completion.getTextEdit(), usedSnippets, isCommandExecution(item));
Optional<TextEdit> mainEdit = adaptEdits(doc, completion.getTextEdit(), usedSnippets);
if (mainEdit.isPresent()) {
item.setTextEdit(Either.forLeft(mainEdit.get()));
if (server.hasCompletionSnippetSupport()) {
Expand All @@ -419,7 +417,7 @@ private void resolveAdditionalEdits(TextDocument doc, ICompletionProposal comple
if (!edit.isResolved()) {
edit.resolve();
}
adaptEdits(doc, edit, null, isCommandExecution(item)).ifPresent(extraEdit -> {
adaptEdits(doc, edit, null).ifPresent(extraEdit -> {
item.setAdditionalTextEdits(ImmutableList.of(extraEdit));
});
} else {
Expand All @@ -435,7 +433,7 @@ private static String toMarkdown(Renderable r) {
return null;
}

private Optional<TextEdit> adaptEdits(TextDocument doc, DocumentEdits edits, AtomicBoolean usedSnippets, boolean ignoreClientIndent) {
private Optional<TextEdit> adaptEdits(TextDocument doc, DocumentEdits edits, AtomicBoolean usedSnippets) {
try {
TextReplace replaceEdit = edits == null ? null : edits.asReplacement(doc);
if (usedSnippets != null && edits != null) {
Expand All @@ -448,9 +446,10 @@ private Optional<TextEdit> adaptEdits(TextDocument doc, DocumentEdits edits, Ato
TextDocument newDoc = doc.copy();
edits.apply(newDoc);
TextEdit vscodeEdit = new TextEdit();
vscodeEdit.setRange(doc.toRange(replaceEdit.start, replaceEdit.end-replaceEdit.start));
vscodeEdit.setRange(doc.toRange(replaceEdit.start, replaceEdit.end - replaceEdit.start));
String newText = replaceEdit.newText;
IRegion selection = edits.getSelection();

if (selection!=null && usedSnippets != null) {
//Special handling for the case where cursor is *not* just at the end of the newText
int cursor = selection.getOffset() + selection.getLength();
Expand All @@ -465,9 +464,7 @@ private Optional<TextEdit> adaptEdits(TextDocument doc, DocumentEdits edits, Ato
usedSnippets.set(true);
}
}
// if (isMagicIndentingClient() && !ignoreClientIndent) {
// newText = vscodeIndentFix(doc, vscodeEdit.getRange().getStart(), replaceEdit.newText);
// }

vscodeEdit.setNewText(newText);
return Optional.of(vscodeEdit);
}
Expand All @@ -477,42 +474,6 @@ private Optional<TextEdit> adaptEdits(TextDocument doc, DocumentEdits edits, Ato
}
}

/**
* When this is true, it means the client does 'magic indents' (basically.. that is only on vscode since the magics aren't part of the LSP spec).
*/
// private boolean isMagicIndentingClient() {
// return !Boolean.getBoolean("lsp.completions.indentation.enable");
// }

private static String vscodeIndentFix(TextDocument doc, Position start, String newText) {
//Vscode applies some magic indent to a multi-line edit text. We do everything ourself so we have adjust for the magic
// and do some kind of 'inverse magic' here.
//See here: https://github.com/Microsoft/language-server-protocol/issues/83
IndentUtil indenter = new IndentUtil(doc);
try {
String refIndent = indenter.getReferenceIndent(doc.toOffset(start), doc);
if (!refIndent.isEmpty()) {
return StringUtil.stripIndentation(refIndent, newText);
}
} catch (BadLocationException e) {
log.error("{}", e);
}
return newText;
}

private static String revertVscodeIndentFix(TextDocument doc, Position start, String newText) {
IndentUtil indenter = new IndentUtil(doc);
try {
String refIndent = indenter.getReferenceIndent(doc.toOffset(start), doc);
if (!refIndent.isEmpty()) {
return StringUtil.reverseStripIndentation(refIndent, newText);
}
} catch (BadLocationException e) {
log.error("{}", e);
}
return newText;
}

@Override
public CompletionItem resolveCompletion(CancelChecker cancelToken, CompletionItem unresolved) {
resolver.resolveNow(cancelToken, unresolved);
Expand All @@ -532,7 +493,4 @@ public interface CompletionFilter {

}

private boolean isCommandExecution(CompletionItem item) {
return RESOLVE_EDIT_COMMAND == item.getLabel();
}
}
12 changes: 0 additions & 12 deletions ...rness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,18 +465,6 @@ public void apply(CompletionItem completion) throws Exception {
String replaceWith = edit.getLeft().getNewText();
int cursorReplaceOffset = 0;

// if (!Boolean.getBoolean("lsp.completions.indentation.enable")) {
// //Apply indentfix, this is magic vscode seems to apply to edits returned by language server. So our harness has to
// // mimick that behavior. See https://github.com/Microsoft/language-server-protocol/issues/83
// int referenceLine = edit.getLeft().getRange().getStart().getLine();
// int cursorOffset = edit.getLeft().getRange().getStart().getCharacter();
// String referenceIndent = doc.getLineIndentString(referenceLine);
// if (cursorOffset<referenceIndent.length()) {
// referenceIndent = referenceIndent.substring(0, cursorOffset);
// }
// replaceWith = replaceWith.replaceAll("\\n", "\n"+referenceIndent);
// }

// Replace the cursor string
cursorReplaceOffset = replaceWith.indexOf(VS_CODE_CURSOR_MARKER);
if (cursorReplaceOffset >= 0) {
Expand Down
121 changes: 59 additions & 62 deletions ...ver/src/test/java/org/springframework/ide/vscode/boot/test/ApplicationYamlEditorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -651,68 +651,65 @@ void testJumpyInsertion() throws Exception {
//This test not working in vscode. It is only meant to work in environment that
// a) don't apply magic indents
// b) allow completions with no restrictions on the main edit.
withSystemProperty("lsp.completions.indentation.enable", "true", () -> {
String[] names = {"foo", "nested", "bar"};
int levels = 4;
generateNestedProperties(levels, names, "");

//Note: jumpy completions use the snippet '$0' placeholder to move the cursor
//Our harness actually does not understand / interpret snippet placeholders.
//Thus the examples below expected outcome will have both a '$0' and '<*>'
//showing the cursor position. You can think of the second '<*>' as showing
//where the cursor will end up when the client doesn't have snippet support capability.

assertCompletion(
"foo:\n" +
" nested:\n" +
" bar:\n" +
" foo:\n" +
"other:\n" +
"foo.nested.bar.b<*>"
,
"foo:\n" +
" nested:\n" +
" bar:\n" +
" foo:\n" +
" bar: $0\n" +
"other:<*>"
);

assertCompletion(
"foo:\n" +
" nested:\n" +
" bar:\n" +
" foo:\n" +
"other:\n" +
"foo.nested.nested.b<*>"
,
"foo:\n" +
" nested:\n" +
" bar:\n" +
" foo:\n" +
" nested:\n" +
" bar: $0\n" +
"other:<*>"
);

assertCompletion(
"foo.nested.nested.b<*>\n" +
"foo:\n" +
" nested:\n" +
" bar:\n" +
" foo:\n" +
"other:"
,
"foo:\n" +
" nested:\n" +
" bar:\n" +
" foo:\n" +
" nested:\n" +
" bar: <*>\n" +
"other:"
);
return; // Skip running this test
});
String[] names = {"foo", "nested", "bar"};
int levels = 4;
generateNestedProperties(levels, names, "");

//Note: jumpy completions use the snippet '$0' placeholder to move the cursor
//Our harness actually does not understand / interpret snippet placeholders.
//Thus the examples below expected outcome will have both a '$0' and '<*>'
//showing the cursor position. You can think of the second '<*>' as showing
//where the cursor will end up when the client doesn't have snippet support capability.

assertCompletion(
"foo:\n" +
" nested:\n" +
" bar:\n" +
" foo:\n" +
"other:\n" +
"foo.nested.bar.b<*>"
,
"foo:\n" +
" nested:\n" +
" bar:\n" +
" foo:\n" +
" bar: $0\n" +
"other:<*>"
);

assertCompletion(
"foo:\n" +
" nested:\n" +
" bar:\n" +
" foo:\n" +
"other:\n" +
"foo.nested.nested.b<*>"
,
"foo:\n" +
" nested:\n" +
" bar:\n" +
" foo:\n" +
" nested:\n" +
" bar: $0\n" +
"other:<*>"
);

assertCompletion(
"foo.nested.nested.b<*>\n" +
"foo:\n" +
" nested:\n" +
" bar:\n" +
" foo:\n" +
"other:"
,
"foo:\n" +
" nested:\n" +
" bar:\n" +
" foo:\n" +
" nested:\n" +
" bar: <*>\n" +
"other:"
);
}

@Test
Expand Down

0 comments on commit 029cc73

Please sign in to comment.