Skip to content

Commit 6fdd3b7

Browse files
Achal1607shivam71sid-srini
committed
Improvements in notebook configuration, stability and performance
- Addressed review comments - Fixed some labels - Improvements to NotebookUtils for conversions between line-based positions and string offsets - Avoiding unnecessary string split and joins - Added unit tests - Notebook configurations related changes 1. Made classpath,modulepath and add modules configs as array type on frontend 2. Corresponding changes on backend to process the array 3. Updated notebook configs unit tests - Add enable-preview flag from the project context if not configured - Addressed minor gaps in notebook cell execution - Correctly printing the runtime errors stacktrace - Handling null output stream callback - Performing lineEnding normalization only where required in cell setContent. - Minor performance improvements Co-authored-by: Achal Talati <achal.talati@oracle.com> Co-authored-by: Shivam Madan <shivam.madan@oracle.com> Co-authored-by: Siddharth Srinivasan <siddharth.srinivasan@oracle.com>
1 parent bbabb97 commit 6fdd3b7

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+1015
-542
lines changed

‎THIRD_PARTY_LICENSES.txt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9835,8 +9835,11 @@ SOFTWARE.
98359835

98369836
------------------ END OF DEPENDENCY LICENSE --------------------
98379837

9838-
Dependancy: nbformat
9838+
Dependency: nbformat.v4.5.schema.json
98399839
====================
9840+
Accessible at: vscode/src/notebooks/nbformat.v4.5.d7.schema.json
9841+
Modified from source at: https://github.com/jupyter/nbformat/blob/main/nbformat/v4/nbformat.v4.5.schema.json
9842+
License: BSD 3-Clause License
98409843

98419844
------------------ START OF DEPENDENCY LICENSE --------------------
98429845
BSD 3-Clause License

‎build.xml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,9 @@
7979
patches/dev-dependency-licenses.diff
8080
patches/nb-telemetry.diff
8181
patches/change-method-parameters-refactoring-qualified-names.diff
82-
patches/upgrade-lsp4j.diff
83-
patches/java-notebooks.diff
82+
patches/upgrade-lsp4j.diff
83+
patches/updated-show-input-params.diff
84+
patches/java-notebooks.diff
8485
</string>
8586
<filterchain>
8687
<tokenfilter delimoutput=" ">

‎nbcode/notebooks/nbproject/project.properties‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,7 @@
1515
#
1616
javac.source=1.8
1717
requires.nb.javac=true
18-
javac.compilerargs=-Xlint -Xlint:-serial
19-
test.run.args=--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
20-
--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED \
21-
--add-exports=jdk.compiler/com.sun.tools.javac.resources=ALL-UNNAMED \
22-
--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED \
23-
18+
javac.compilerargs=-Xlint -Xlint:-serial
2419
test.unit.lib.cp=
2520
test.unit.run.cp.extra=
2621
license.file=../../LICENSE.txt

‎nbcode/notebooks/nbproject/project.xml‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,15 @@
2929
<specification-version>2.11.0</specification-version>
3030
</run-dependency>
3131
</dependency>
32+
<dependency>
33+
<code-name-base>org.netbeans.api.annotations.common</code-name-base>
34+
<build-prerequisite/>
35+
<compile-dependency/>
36+
<run-dependency>
37+
<release-version>1</release-version>
38+
<specification-version>1.57</specification-version>
39+
</run-dependency>
40+
</dependency>
3241
<dependency>
3342
<code-name-base>org.netbeans.api.java</code-name-base>
3443
<build-prerequisite/>
@@ -153,10 +162,10 @@
153162
<recursive/>
154163
<compile-dependency/>
155164
</test-dependency>
156-
<test-dependency>
157-
<code-name-base>org.netbeans.modules.java.lsp.server</code-name-base>
158-
<compile-dependency/>
159-
</test-dependency>
165+
<test-dependency>
166+
<code-name-base>org.netbeans.modules.java.lsp.server</code-name-base>
167+
<compile-dependency/>
168+
</test-dependency>
160169
</test-type>
161170
</test-dependencies>
162171
<public-packages/>

‎nbcode/notebooks/src/org/netbeans/modules/nbcode/java/notebook/CellState.java‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.concurrent.CompletableFuture;
1919
import java.util.concurrent.ExecutionException;
2020
import java.util.concurrent.atomic.AtomicReference;
21+
import java.util.logging.Level;
2122
import java.util.logging.Logger;
2223
import org.eclipse.lsp4j.ExecutionSummary;
2324
import org.eclipse.lsp4j.NotebookCell;
@@ -82,7 +83,6 @@ public String getNotebookUri() {
8283
}
8384

8485
public void setContent(String newContent, int newVersion) throws InterruptedException, ExecutionException {
85-
String normalizedContent = NotebookUtils.normalizeLineEndings(newContent);
8686
VersionAwareContent currentContent = content.get();
8787

8888
if (currentContent.getVersion() != newVersion - 1) {
@@ -99,16 +99,17 @@ public void setContent(String newContent, int newVersion) throws InterruptedExce
9999
int receivedVersion = newCellState.getVersion();
100100

101101
if (receivedVersion > currentContent.getVersion()) {
102-
VersionAwareContent newVersionContent = new VersionAwareContent(newCellState.getText(), receivedVersion);
103-
content.set(newVersionContent);
102+
VersionAwareContent newVersionContent = new VersionAwareContent(NotebookUtils.normalizeLineEndings(newCellState.getText()), receivedVersion);
103+
content.updateAndGet(current -> current != currentContent && receivedVersion <= current.getVersion() ? current : newVersionContent);
104104
} else {
105-
throw new IllegalStateException("Version mismatch: Received version to be greater than current version, received version: " + (receivedVersion) + ", current version: " + currentContent.getVersion());
105+
LOG.log(Level.WARNING, "Version mismatch: Received version to be greater than current version, received version: {0}, current version: {1}", new Object[]{receivedVersion, currentContent.getVersion()});
106106
}
107107
} else {
108-
VersionAwareContent newVersionContent = new VersionAwareContent(normalizedContent, newVersion);
108+
// newContent is already normalized during applyChanges
109+
VersionAwareContent newVersionContent = new VersionAwareContent(newContent, newVersion);
109110

110111
if (!content.compareAndSet(currentContent, newVersionContent)) {
111-
throw new IllegalStateException("Concurrent modification detected. Version expected: " + (newVersion - 1) + ", current: " + content.get().getVersion());
112+
LOG.log(Level.WARNING, "Concurrent modification detected. Version expected: {0}, current: {1}", new Object[]{newVersion - 1, content.get().getVersion()});
112113
}
113114
}
114115
}
@@ -122,7 +123,7 @@ public void requestContentAndSet() throws InterruptedException, ExecutionExcepti
122123
if (newCellState.getVersion() <= 0) {
123124
throw new IllegalStateException("Received incorrect version number: " + newCellState.getVersion());
124125
}
125-
VersionAwareContent newVersionContent = new VersionAwareContent(newCellState.getText(), newCellState.getVersion());
126+
VersionAwareContent newVersionContent = new VersionAwareContent(NotebookUtils.normalizeLineEndings(newCellState.getText()), newCellState.getVersion());
126127
content.set(newVersionContent);
127128
}
128129

@@ -151,8 +152,8 @@ protected VersionAwareContent getVersionAwareContent() {
151152

152153
protected class VersionAwareContent {
153154

154-
private String content;
155-
private int version;
155+
private final String content;
156+
private final int version;
156157

157158
public VersionAwareContent(String content, int version) {
158159
this.content = content;
@@ -163,17 +164,8 @@ public String getContent() {
163164
return content;
164165
}
165166

166-
public void setContent(String content) {
167-
this.content = content;
168-
}
169-
170167
public int getVersion() {
171168
return version;
172169
}
173-
174-
public void setVersion(int version) {
175-
this.version = version;
176-
}
177-
178170
}
179171
}

‎nbcode/notebooks/src/org/netbeans/modules/nbcode/java/notebook/CodeCompletionProvider.java‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,8 @@
1616
package org.netbeans.modules.nbcode.java.notebook;
1717

1818
import java.util.ArrayList;
19-
import java.util.HashMap;
19+
import java.util.HashSet;
2020
import java.util.List;
21-
import java.util.Map;
2221
import java.util.concurrent.CompletableFuture;
2322
import java.util.logging.Level;
2423
import java.util.logging.Logger;
@@ -63,34 +62,27 @@ public CompletableFuture<Either<List<CompletionItem>, CompletionList>> getCodeCo
6362
}
6463

6564
SourceCodeAnalysis sourceCodeAnalysis = instance.sourceCodeAnalysis();
66-
String textToComplete = getTextToComplete(
65+
List<Suggestion> suggestions = getSuggestions(
6766
params.getTextDocument().getUri(),
6867
params.getPosition(),
6968
state,
7069
sourceCodeAnalysis
7170
);
7271

73-
List<Suggestion> suggestions = sourceCodeAnalysis.completionSuggestions(
74-
textToComplete,
75-
textToComplete.length(),
76-
new int[1]
77-
);
78-
7972
List<CompletionItem> completionItems = new ArrayList<>();
80-
Map<String, Boolean> visited = new HashMap<>();
73+
HashSet<String> visited = new HashSet<>();
8174

8275
for (Suggestion suggestion : suggestions) {
8376
String continuation = suggestion.continuation();
84-
if (!visited.containsKey(continuation)) {
77+
if (visited.add(continuation)) {
8578
completionItems.add(createCompletionItem(continuation));
86-
visited.put(continuation, Boolean.TRUE);
8779
}
8880
}
8981

9082
return Either.<List<CompletionItem>, CompletionList>forLeft(completionItems);
9183

9284
} catch (Exception e) {
93-
LOG.log(Level.WARNING, "Error getting code completions: {0}", e.getMessage());
85+
LOG.log(Level.WARNING, "Error getting code completions: {0}", e.toString());
9486
return Either.<List<CompletionItem>, CompletionList>forLeft(new ArrayList<>());
9587
}
9688
});
@@ -103,14 +95,28 @@ private CompletionItem createCompletionItem(String label) {
10395
return item;
10496
}
10597

106-
private String getTextToComplete(String uri, Position position, NotebookDocumentStateManager state, SourceCodeAnalysis sourceCodeAnalysis) {
98+
private List<Suggestion> getSuggestions(String uri, Position position, NotebookDocumentStateManager state, SourceCodeAnalysis sourceCodeAnalysis) {
10799
CellState cellState = state.getCell(uri);
108100
String content = cellState.getContent();
109101
int cursorOffset = NotebookUtils.getOffset(content, position);
110-
102+
int[] anchor = new int[1];
111103
String offsetText = content.substring(0, cursorOffset);
112104
List<String> snippets = NotebookUtils.getCodeSnippets(sourceCodeAnalysis, offsetText);
113105

114-
return snippets.isEmpty() ? "" : snippets.getLast();
106+
String lastSnippet = snippets.isEmpty() ? "" : snippets.get(snippets.size()-1);
107+
List<Suggestion> suggestions = new ArrayList<>();
108+
suggestions.addAll(sourceCodeAnalysis.completionSuggestions(
109+
lastSnippet,
110+
lastSnippet.length(),
111+
anchor
112+
));
113+
if (snippets.size() > 1) {
114+
suggestions.addAll(sourceCodeAnalysis.completionSuggestions(
115+
offsetText,
116+
offsetText.length(),
117+
anchor
118+
));
119+
}
120+
return suggestions;
115121
}
116122
}

0 commit comments

Comments
 (0)