Skip to content

Commit 426cc29

Browse files
sid-srinisid-srini
committed
[JVSC-249] Fix code folding in VS Code due to line-only folding limitation
Added the backport of the in-review NB PR 7750 diff which allows supporting LSP clients which have line-only folding support. - This is the case of VS Code as referenced in microsoft/vscode#50840. Closes #249 Signed-off-by: Siddharth Srinivasan <[email protected]>
1 parent 6079973 commit 426cc29

File tree

2 files changed

+294
-0
lines changed

2 files changed

+294
-0
lines changed

‎build.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
patches/7722.diff
4444
patches/7724.diff
4545
patches/7733.diff
46+
patches/7750-inreview-backport.diff
4647
patches/mvn-sh.diff
4748
patches/generate-dependencies.diff
4849
patches/rename-debugger.diff

‎patches/7750-inreview-backport.diff‎

Lines changed: 293 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,293 @@
1+
diff --git a/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java b/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
2+
index c112b4eb73..ff1fe7f903 100644
3+
--- a/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
4+
+++ b/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
5+
@@ -44,14 +44,18 @@ import java.net.MalformedURLException;
6+
import java.net.URISyntaxException;
7+
import java.nio.file.Path;
8+
import java.nio.file.Paths;
9+
+import java.util.ArrayDeque;
10+
import java.util.ArrayList;
11+
import java.util.Arrays;
12+
import java.util.Collection;
13+
import java.util.Collections;
14+
+import java.util.Comparator;
15+
+import java.util.Deque;
16+
import java.util.EnumMap;
17+
import java.util.EnumSet;
18+
import java.util.HashMap;
19+
import java.util.HashSet;
20+
+import java.util.Iterator;
21+
import java.util.LinkedHashMap;
22+
import java.util.List;
23+
import java.util.Locale;
24+
@@ -1561,12 +1565,13 @@ public class TextDocumentServiceImpl implements TextDocumentService, LanguageCli
25+
if (source == null) {
26+
return CompletableFuture.completedFuture(Collections.emptyList());
27+
}
28+
+ final boolean lineFoldingOnly = client.getNbCodeCapabilities().getClientCapabilities().getTextDocument().getFoldingRange().getLineFoldingOnly() == Boolean.TRUE;
29+
CompletableFuture<List<FoldingRange>> result = new CompletableFuture<>();
30+
try {
31+
source.runUserActionTask(cc -> {
32+
cc.toPhase(JavaSource.Phase.RESOLVED);
33+
Document doc = cc.getSnapshot().getSource().getDocument(true);
34+
- JavaElementFoldVisitor v = new JavaElementFoldVisitor(cc, cc.getCompilationUnit(), cc.getTrees().getSourcePositions(), doc, new FoldCreator<FoldingRange>() {
35+
+ JavaElementFoldVisitor<FoldingRange> v = new JavaElementFoldVisitor<>(cc, cc.getCompilationUnit(), cc.getTrees().getSourcePositions(), doc, new FoldCreator<FoldingRange>() {
36+
@Override
37+
public FoldingRange createImportsFold(int start, int end) {
38+
return createFold(start, end, FoldingRangeKind.Imports);
39+
@@ -1611,7 +1616,10 @@ public class TextDocumentServiceImpl implements TextDocumentService, LanguageCli
40+
});
41+
v.checkInitialFold();
42+
v.scan(cc.getCompilationUnit(), null);
43+
- result.complete(v.getFolds());
44+
+ List<FoldingRange> folds = v.getFolds();
45+
+ if (lineFoldingOnly)
46+
+ folds = convertToLineOnlyFolds(folds);
47+
+ result.complete(folds);
48+
}, true);
49+
} catch (IOException ex) {
50+
result.completeExceptionally(ex);
51+
@@ -1619,6 +1627,76 @@ public class TextDocumentServiceImpl implements TextDocumentService, LanguageCli
52+
return result;
53+
}
54+
55+
+ /**
56+
+ * Converts a list of code-folds to a line-only Range form, in place of the
57+
+ * finer-grained form of {@linkplain Position Position-based} (line, column) Ranges.
58+
+ * <p>
59+
+ * This is needed for LSP clients that do not support the finer grained Range
60+
+ * specification. This is expected to be advertised by the client in
61+
+ * {@code FoldingRangeClientCapabilities.lineFoldingOnly}.
62+
+ *
63+
+ * @implSpec The line-only ranges computed uphold the code-folding invariant that:
64+
+ * <em>a fold <b>does not end</b> at the same point <b>where</b> another fold <b>starts</b></em>.
65+
+ *
66+
+ * @implNote This is performed in {@code O(n log n) + O(n)} time and {@code O(n)} space for the returned list.
67+
+ *
68+
+ * @param folds List of code-folding ranges computed for a textDocument,
69+
+ * containing fine-grained {@linkplain Position Position-based}
70+
+ * (line, column) ranges.
71+
+ * @return List of code-folding ranges computed for a textDocument,
72+
+ * containing coarse-grained line-only ranges.
73+
+ *
74+
+ * @see <a href="https://microsoft.github.io/language-server-protocol/specifications/specification-current/#foldingRangeClientCapabilities">
75+
+ * LSP FoldingRangeClientCapabilities</a>
76+
+ */
77+
+ static List<FoldingRange> convertToLineOnlyFolds(List<FoldingRange> folds) {
78+
+ if (folds != null && folds.size() > 1) {
79+
+ // Ensure that the folds are sorted in increasing order of their start position
80+
+ folds = new ArrayList<>(folds);
81+
+ folds.sort(Comparator.comparingInt(FoldingRange::getStartLine)
82+
+ .thenComparing(FoldingRange::getStartCharacter));
83+
+ // Maintain a stack of enclosing folds
84+
+ Deque<FoldingRange> enclosingFolds = new ArrayDeque<>();
85+
+ for (FoldingRange fold : folds) {
86+
+ FoldingRange last;
87+
+ while ((last = enclosingFolds.peek()) != null &&
88+
+ (last.getEndLine() < fold.getEndLine() ||
89+
+ (last.getEndLine() == fold.getEndLine() && last.getEndCharacter() < fold.getEndCharacter()))) {
90+
+ // The last enclosingFold does not enclose this fold.
91+
+ // Due to sortedness of the folds, last also ends before this fold starts.
92+
+ enclosingFolds.pop();
93+
+ // If needed, adjust last to end on a line prior to this fold start
94+
+ if (last.getEndLine() == fold.getStartLine()) {
95+
+ last.setEndLine(last.getEndLine() - 1);
96+
+ }
97+
+ last.setEndCharacter(null); // null denotes the end of the line.
98+
+ last.setStartCharacter(null); // null denotes the end of the line.
99+
+ }
100+
+ enclosingFolds.push(fold);
101+
+ }
102+
+ // empty the stack; since each fold completely encloses the next higher one.
103+
+ FoldingRange fold;
104+
+ while ((fold = enclosingFolds.poll()) != null) {
105+
+ fold.setEndCharacter(null); // null denotes the end of the line.
106+
+ fold.setStartCharacter(null); // null denotes the end of the line.
107+
+ }
108+
+ // Remove invalid or duplicate folds
109+
+ Iterator<FoldingRange> it = folds.iterator();
110+
+ FoldingRange prev = null;
111+
+ while(it.hasNext()) {
112+
+ FoldingRange next = it.next();
113+
+ if (next.getEndLine() <= next.getStartLine() ||
114+
+ (prev != null && prev.equals(next))) {
115+
+ it.remove();
116+
+ } else {
117+
+ prev = next;
118+
+ }
119+
+ }
120+
+ }
121+
+ return folds;
122+
+ }
123+
+
124+
+
125+
@Override
126+
public void didOpen(DidOpenTextDocumentParams params) {
127+
LOG.log(Level.FINER, "didOpen: {0}", params);
128+
diff --git a/java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImplTest.java b/java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImplTest.java
129+
index 0f2bda50ae..06fd93d3e5 100644
130+
--- a/java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImplTest.java
131+
+++ b/java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImplTest.java
132+
@@ -18,14 +18,19 @@
133+
*/
134+
package org.netbeans.modules.java.lsp.server.protocol;
135+
136+
+import java.util.Collections;
137+
+import java.util.List;
138+
import java.util.concurrent.atomic.AtomicInteger;
139+
import javax.swing.event.DocumentEvent;
140+
import javax.swing.event.DocumentListener;
141+
import javax.swing.text.BadLocationException;
142+
import javax.swing.text.Document;
143+
import javax.swing.text.PlainDocument;
144+
+import org.eclipse.lsp4j.FoldingRange;
145+
import org.netbeans.junit.NbTestCase;
146+
147+
+import static org.netbeans.modules.java.lsp.server.protocol.TextDocumentServiceImpl.convertToLineOnlyFolds;
148+
+
149+
public class TextDocumentServiceImplTest extends NbTestCase {
150+
151+
public TextDocumentServiceImplTest(String name) {
152+
@@ -117,4 +122,141 @@ public class TextDocumentServiceImplTest extends NbTestCase {
153+
fail(String.valueOf(e));
154+
}
155+
}
156+
+
157+
+ public void testConvertToLineOnlyFolds() {
158+
+ assertNull(convertToLineOnlyFolds(null));
159+
+ assertEquals(0, convertToLineOnlyFolds(Collections.emptyList()).size());
160+
+ List<FoldingRange> inputFolds, outputFolds;
161+
+ inputFolds = Collections.singletonList(createRange(10, 20));
162+
+ assertEquals(inputFolds, convertToLineOnlyFolds(inputFolds));
163+
+
164+
+ // test stable sort by start index
165+
+ inputFolds = List.of(createRange(10, 20, 9, 9), createRange(5, 9, 9, 9), createRange(10, 19, 9, 9), createRange(10, 14, 13, 13));
166+
+ outputFolds = List.of(createRange(5, 9), createRange(10, 20), createRange(10, 19), createRange(10, 14));
167+
+ assertEquals(outputFolds, convertToLineOnlyFolds(inputFolds));
168+
+
169+
+ // test already disjoint folds
170+
+ inputFolds = List.of(createRange(10, 20, 9, 9), createRange(5, 9, 9, 9), createRange(15, 19, 13, 13), createRange(10, 14, 13, 13));
171+
+ outputFolds = List.of(createRange(5, 9), createRange(10, 20), createRange(10, 14), createRange(15, 19));
172+
+ assertEquals(outputFolds, convertToLineOnlyFolds(inputFolds));
173+
+
174+
+ // test invariant of range.endLine: there exists no otherRange.startLine == range.endLine.
175+
+ inputFolds = List.of(createRange(10, 20, 35, 9), createRange(5, 10, 12, 9), createRange(15, 19, 20, 13), createRange(10, 15, 51, 13));
176+
+ assertEquals(outputFolds, convertToLineOnlyFolds(inputFolds));
177+
+
178+
+ // test a complex example of a full file:
179+
+//import java.util.ArrayList;
180+
+//import java.util.Collection;
181+
+//import java.util.Collections;
182+
+//
183+
+///**
184+
+// * A top-class action performer
185+
+// *
186+
+// * @since 1.1
187+
+// */
188+
+//public class TopClass {
189+
+//
190+
+// private final String action;
191+
+// private final int index;
192+
+//
193+
+// /**
194+
+// * @param action Top action to be done
195+
+// */
196+
+// public TopClass(String action) {
197+
+// this(action, 0);
198+
+// }
199+
+//
200+
+// /**
201+
+// * @param action Top action to be done
202+
+// * @param index Action index
203+
+// */
204+
+// public TopClass(String action, int index) {
205+
+// this.action = action;
206+
+// this.index = index;
207+
+// }
208+
+//
209+
+// public void doSomethingTopClass(TopClass tc) {
210+
+// // what can we do
211+
+// {
212+
+// if (tc == this) {
213+
+// return;
214+
+// } else if (tc.getClass() == this.getClass()) {
215+
+// } else if (tc.getClass().isAssignableFrom(this.getClass())) {
216+
+//
217+
+// } else {
218+
+// if (true) {
219+
+// switch (tc) {
220+
+// default: { /* this is some comment */ ; }
221+
+// /// some outside default
222+
+// }
223+
+// } else { if (true) { { /* some */ } { /* bad blocks */ }
224+
+// }}
225+
+// /* done */
226+
+// }
227+
+// }
228+
+// tc.doSomethingTopClass(tc);
229+
+// }
230+
+//
231+
+// public class InnerClass {
232+
+// @Override
233+
+// public String toString() {
234+
+// StringBuilder sb = new StringBuilder();
235+
+// sb.append("InnerClass{");
236+
+// sb.append("action=").append(action);
237+
+// sb.append(", index=").append(index);
238+
+// sb.append('}');
239+
+// return sb.toString();
240+
+// }
241+
+// }
242+
+//}
243+
+ inputFolds = List.of(
244+
+ createRange(27, 30, 48, 5),
245+
+ createRange(0, 3, 7, 30),
246+
+ createRange(32, 52, 51, 5),
247+
+ createRange(37, 38, 59, 13),
248+
+ createRange(34, 50, 10, 9),
249+
+ createRange(46, 46, 39, 51),
250+
+ createRange(35, 37, 30, 13),
251+
+ createRange(38, 40, 74, 13),
252+
+ createRange(40, 49, 21, 13),
253+
+ createRange(46, 47, 37, 17),
254+
+ createRange(41, 46, 28, 17),
255+
+ createRange(42, 45, 34, 21),
256+
+ createRange(11, 66, 24, 1),
257+
+ createRange(43, 43, 35, 65),
258+
+ createRange(46, 47, 25, 18),
259+
+ createRange(54, 64, 30, 5),
260+
+ createRange(46, 46, 54, 72),
261+
+ createRange(6, 10, 4, 1),
262+
+ createRange(56, 63, 35, 9)
263+
+ );
264+
+ outputFolds = List.of(
265+
+ createRange(0, 3),
266+
+ createRange(6, 10),
267+
+ createRange(11, 66),
268+
+ createRange(27, 30),
269+
+ createRange(32, 52),
270+
+ createRange(34, 50),
271+
+ createRange(35, 36),
272+
+ createRange(38, 39),
273+
+ createRange(40, 49),
274+
+ createRange(41, 45),
275+
+ createRange(42, 45),
276+
+ createRange(46, 47),
277+
+ createRange(54, 64),
278+
+ createRange(56, 63)
279+
+ );
280+
+ assertEquals(outputFolds, convertToLineOnlyFolds(inputFolds));
281+
+ }
282+
+
283+
+ private static FoldingRange createRange(int startLine, int endLine) {
284+
+ return new FoldingRange(startLine, endLine);
285+
+ }
286+
+
287+
+ private static FoldingRange createRange(int startLine, int endLine, Integer startColumn, Integer endColumn) {
288+
+ FoldingRange foldingRange = new FoldingRange(startLine, endLine);
289+
+ foldingRange.setStartCharacter(startColumn);
290+
+ foldingRange.setEndCharacter(endColumn);
291+
+ return foldingRange;
292+
+ }
293+
}

0 commit comments

Comments
 (0)