Skip to content

Commit cabd2d6

Browse files
Achal1607Achal1607
committed
addressed some review comments
1 parent 9f7448c commit cabd2d6

37 files changed

+346
-305
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: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
patches/nb-telemetry.diff
6666
patches/change-method-parameters-refactoring-qualified-names.diff
6767
patches/upgrade-lsp4j.diff
68+
patches/updated-show-input-params.diff
6869
patches/java-notebooks.diff
6970
</string>
7071
<filterchain>

‎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/src/org/netbeans/modules/nbcode/java/notebook/CellState.java‎

Lines changed: 7 additions & 15 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;
@@ -100,15 +101,15 @@ public void setContent(String newContent, int newVersion) throws InterruptedExce
100101

101102
if (receivedVersion > currentContent.getVersion()) {
102103
VersionAwareContent newVersionContent = new VersionAwareContent(newCellState.getText(), receivedVersion);
103-
content.set(newVersionContent);
104+
content.updateAndGet(current -> current != currentContent && receivedVersion <= current.getVersion() ? current : newVersionContent);
104105
} else {
105-
throw new IllegalStateException("Version mismatch: Received version to be greater than current version, received version: " + (receivedVersion) + ", current version: " + currentContent.getVersion());
106+
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()});
106107
}
107108
} else {
108109
VersionAwareContent newVersionContent = new VersionAwareContent(normalizedContent, 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: 5 additions & 7 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;
@@ -77,20 +76,19 @@ public CompletableFuture<Either<List<CompletionItem>, CompletionList>> getCodeCo
7776
);
7877

7978
List<CompletionItem> completionItems = new ArrayList<>();
80-
Map<String, Boolean> visited = new HashMap<>();
79+
HashSet<String> visited = new HashSet<>();
8180

8281
for (Suggestion suggestion : suggestions) {
8382
String continuation = suggestion.continuation();
84-
if (!visited.containsKey(continuation)) {
83+
if(visited.add(continuation)){
8584
completionItems.add(createCompletionItem(continuation));
86-
visited.put(continuation, Boolean.TRUE);
8785
}
8886
}
8987

9088
return Either.<List<CompletionItem>, CompletionList>forLeft(completionItems);
9189

9290
} catch (Exception e) {
93-
LOG.log(Level.WARNING, "Error getting code completions: {0}", e.getMessage());
91+
LOG.log(Level.WARNING, "Error getting code completions: {0}", e.toString());
9492
return Either.<List<CompletionItem>, CompletionList>forLeft(new ArrayList<>());
9593
}
9694
});
@@ -110,7 +108,7 @@ private String getTextToComplete(String uri, Position position, NotebookDocument
110108

111109
String offsetText = content.substring(0, cursorOffset);
112110
List<String> snippets = NotebookUtils.getCodeSnippets(sourceCodeAnalysis, offsetText);
113-
111+
// TODO: Code completion only considers the last incomplete snippet, missing context from earlier snippets which are not executed.
114112
return snippets.isEmpty() ? "" : snippets.getLast();
115113
}
116114
}

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

Lines changed: 23 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import jdk.jshell.JShell;
1919
import jdk.jshell.SnippetEvent;
2020
import java.util.ArrayList;
21+
import java.util.Collections;
2122
import java.util.List;
2223
import java.util.Map;
2324
import java.util.concurrent.CompletableFuture;
@@ -28,9 +29,11 @@
2829
import java.util.regex.Matcher;
2930
import java.util.regex.Pattern;
3031
import jdk.jshell.Diag;
32+
import jdk.jshell.JShellException;
3133
import jdk.jshell.SourceCodeAnalysis;
3234
import org.netbeans.modules.java.lsp.server.notebook.CellExecutionResult;
3335
import org.netbeans.modules.java.lsp.server.notebook.NotebookCellExecutionProgressResultParams;
36+
import org.netbeans.modules.java.lsp.server.notebook.NotebookCellExecutionProgressResultParams.Builder;
3437
import org.netbeans.modules.java.lsp.server.notebook.NotebookCellExecutionProgressResultParams.EXECUTION_STATUS;
3538
import org.netbeans.modules.java.lsp.server.protocol.NbCodeLanguageClient;
3639
import org.openide.util.RequestProcessor;
@@ -70,14 +73,14 @@ private static class Singleton {
7073
public String interrupt(List<Object> arguments) {
7174
if (arguments == null) {
7275
LOG.warning("Received null in interrupt execution request");
73-
return "Arguments list is null";
76+
throw new IllegalArgumentException("Recevied null arguments");
7477
}
7578

7679
String notebookId = NotebookUtils.getArgument(arguments, 0, String.class);
7780

7881
if (notebookId == null) {
7982
LOG.warning("Received empty notebookId in interrupt execution request");
80-
return "Empty notebookId received";
83+
throw new IllegalArgumentException("Empty notebookId received");
8184
}
8285

8386
return interruptCodeExecution(notebookId);
@@ -150,13 +153,12 @@ public CompletableFuture<Boolean> evaluate(List<Object> arguments) {
150153

151154
private void codeEvalTaskRunnable(CompletableFuture<Boolean> future, JShell jshell, String notebookId, String cellId, String sourceCode) {
152155
try {
153-
activeCellExecutionMapping.put(notebookId, cellId);
154-
sendNotification(notebookId, EXECUTION_STATUS.EXECUTING);
155-
156156
if (jshell == null) {
157-
future.completeExceptionally(new ExceptionInInitializerError("notebook session not found or closed"));
157+
future.completeExceptionally(new IllegalStateException("notebook session not found or closed"));
158158
return;
159159
}
160+
activeCellExecutionMapping.put(notebookId, cellId);
161+
sendNotification(notebookId, EXECUTION_STATUS.EXECUTING);
160162

161163
runCode(jshell, sourceCode, notebookId);
162164
flushStreams(notebookId);
@@ -220,25 +222,22 @@ private List<String> getCompilationErrors(JShell jshell, SnippetEvent event) {
220222

221223
private List<String> getRuntimeErrors(SnippetEvent event) {
222224
List<String> runtimeErrors = new ArrayList<>();
223-
if (event.exception() != null) {
224-
if (!event.exception().getMessage().isBlank()) {
225-
runtimeErrors.add(event.exception().getMessage());
225+
JShellException jshellException = event.exception();
226+
if (jshellException != null) {
227+
String msg = jshellException.getMessage();
228+
if (msg != null && !msg.isBlank()) {
229+
runtimeErrors.add(msg);
226230
}
227-
if (!event.exception().fillInStackTrace().toString().isBlank()) {
228-
runtimeErrors.add(event.exception().fillInStackTrace().toString());
231+
if (!jshellException.fillInStackTrace().toString().isBlank()) {
232+
runtimeErrors.add(jshellException.fillInStackTrace().toString());
229233
}
230234
}
231235

232236
return runtimeErrors;
233237
}
234238

235239
private List<String> getSnippetValue(SnippetEvent event) {
236-
List<String> snippetValues = new ArrayList<>();
237-
if (event.value() != null) {
238-
snippetValues.add(event.value());
239-
}
240-
241-
return snippetValues;
240+
return event.value() != null ? List.of(event.value()) : Collections.emptyList();
242241
}
243242

244243
private void flushStreams(String notebookId) {
@@ -335,42 +334,19 @@ private void sendNotification(String notebookId, String cellId, byte[] msg, List
335334
}
336335
}
337336

338-
NotebookCellExecutionProgressResultParams params;
337+
Builder b = NotebookCellExecutionProgressResultParams.builder(notebookId, cellId).status(status);
339338
if (msg == null) {
340339
if (diags != null) {
341-
params = NotebookCellExecutionProgressResultParams
342-
.builder(notebookId, cellId)
343-
.diagnostics(diags)
344-
.status(status)
345-
.build();
340+
b.diagnostics(diags);
346341
} else if (errorDiags != null) {
347-
params = NotebookCellExecutionProgressResultParams
348-
.builder(notebookId, cellId)
349-
.errorDiagnostics(errorDiags)
350-
.status(status)
351-
.build();
352-
} else {
353-
params = NotebookCellExecutionProgressResultParams
354-
.builder(notebookId, cellId)
355-
.status(status)
356-
.build();
342+
b.errorDiagnostics(errorDiags);
357343
}
358344
} else {
359-
if (isError) {
360-
params = NotebookCellExecutionProgressResultParams
361-
.builder(notebookId, cellId)
362-
.status(status)
363-
.errorStream(CellExecutionResult.text(msg))
364-
.build();
365-
} else {
366-
params = NotebookCellExecutionProgressResultParams
367-
.builder(notebookId, cellId)
368-
.status(status)
369-
.outputStream(CellExecutionResult.text(msg))
370-
.build();
371-
}
345+
b = isError
346+
? b.errorStream(CellExecutionResult.text(msg))
347+
: b.outputStream(CellExecutionResult.text(msg));
372348
}
373-
349+
NotebookCellExecutionProgressResultParams params = b.build();
374350
NbCodeLanguageClient client = LanguageClientInstance.getInstance().getClient();
375351
if (client != null) {
376352
client.notifyNotebookCellExecutionProgress(params);

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public class CustomInputStream extends InputStream {
3535

3636
private static final Logger LOG = Logger.getLogger(CustomInputStream.class.getName());
3737
private ByteArrayInputStream currentStream;
38-
WeakReference<NbCodeLanguageClient> client;
38+
private final WeakReference<NbCodeLanguageClient> client;
3939
private static final String USER_PROMPT_REQUEST = "Please provide scanner input here";
4040

4141
public CustomInputStream(NbCodeLanguageClient client) {
@@ -45,13 +45,14 @@ public CustomInputStream(NbCodeLanguageClient client) {
4545
@Override
4646
public synchronized int read(byte[] b, int off, int len) throws IOException {
4747
try {
48-
if (client == null || client.get() == null) {
48+
NbCodeLanguageClient client = this.client.get();
49+
if (client == null) {
4950
LOG.log(Level.WARNING, "client is null");
5051
return -1;
5152
}
5253

5354
if (currentStream == null || currentStream.available() == 0) {
54-
CompletableFuture<String> future = client.get().showInputBox(new ShowInputBoxParams(USER_PROMPT_REQUEST, "", true));
55+
CompletableFuture<String> future = client.showInputBox(new ShowInputBoxParams(USER_PROMPT_REQUEST, "", true));
5556
String userInput = future.get();
5657

5758
if (userInput == null) {

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,13 @@ public String getNotebookId() {
8888
public void flushOutputStreams() {
8989
try {
9090
outStream.flush();
91+
} catch (IOException exception) {
92+
LOG.log(Level.WARNING, "IOException occurred while flushing out stream: {0}", exception.getMessage());
93+
}
94+
try {
9195
errStream.flush();
92-
} catch (IOException ignored) {
93-
// nothing can be done
96+
} catch (IOException exception) {
97+
LOG.log(Level.WARNING, "IOException occurred while flushing error stream: {0}", exception.getMessage());
9498
}
9599
}
96100

@@ -99,11 +103,23 @@ public void close() {
99103
try {
100104
printOutStream.close();
101105
printErrStream.close();
106+
} catch (Exception exception) {
107+
LOG.log(Level.WARNING, "Exception occurred while closing print streams: {0}", exception.getMessage());
108+
}
109+
try {
102110
outStream.close();
111+
} catch (IOException exception) {
112+
LOG.log(Level.WARNING, "IOException occurred while closing out stream: {0}", exception.getMessage());
113+
}
114+
try {
103115
errStream.close();
116+
} catch (IOException exception) {
117+
LOG.log(Level.WARNING, "IOException occurred while closing error stream: {0}", exception.getMessage());
118+
}
119+
try {
104120
inputStream.close();
105-
} catch (IOException ex) {
106-
LOG.log(Level.WARNING, "IOException occurred while closing the streams {0}", ex.getMessage());
121+
} catch (IOException exception) {
122+
LOG.log(Level.WARNING, "IOException occurred while closing input stream: {0}", exception.getMessage());
107123
}
108124
}
109125
}

0 commit comments

Comments
 (0)