sdedic commented on code in PR #4203:
URL: https://github.com/apache/netbeans/pull/4203#discussion_r895501098
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/input/LspInputServiceImpl.java:
##########
@@ -32,32 +32,42 @@
@JsonSegment("input")
public class LspInputServiceImpl implements InputService {
- private final Map<String, Function<InputCallbackParams,
CompletableFuture<Either<QuickPickStep, InputBoxStep>>>> stepCallbacks = new
HashMap<>();
- private final Map<String, Function<InputCallbackParams,
CompletableFuture<String>>> validateCallbacks = new HashMap<>();
+ private final RegistryImpl registry = new RegistryImpl();
- public String registerInput(Function<InputCallbackParams,
CompletableFuture<Either<QuickPickStep, InputBoxStep>>> stepCallback,
Function<InputCallbackParams, CompletableFuture<String>> validateCallback) {
- String id = "ID:" + System.identityHashCode(stepCallback);
- stepCallbacks.put(id, stepCallback);
- if (validateCallback != null) {
- validateCallbacks.put(id, validateCallback);
+ @Override
+ public CompletableFuture<Either<QuickPickStep, InputBoxStep>>
step(InputCallbackParams params) {
+ Callback callback = registry.callbacks.get(params.getInputId());
+ if (callback == null) {
+ return CompletableFuture.completedFuture(null);
}
- return id;
+ return callback.step(params).handle((step, ex) -> {
+ if (ex != null || step == null) {
+ registry.callbacks.remove(params.getInputId());
+ }
+ return step;
Review Comment:
Q: in case `ex != null` -- wouldn't it be better to throw something to break
the Future's dependents processing ?
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/input/LspInputServiceImpl.java:
##########
@@ -32,32 +32,42 @@
@JsonSegment("input")
public class LspInputServiceImpl implements InputService {
- private final Map<String, Function<InputCallbackParams,
CompletableFuture<Either<QuickPickStep, InputBoxStep>>>> stepCallbacks = new
HashMap<>();
- private final Map<String, Function<InputCallbackParams,
CompletableFuture<String>>> validateCallbacks = new HashMap<>();
+ private final RegistryImpl registry = new RegistryImpl();
- public String registerInput(Function<InputCallbackParams,
CompletableFuture<Either<QuickPickStep, InputBoxStep>>> stepCallback,
Function<InputCallbackParams, CompletableFuture<String>> validateCallback) {
- String id = "ID:" + System.identityHashCode(stepCallback);
- stepCallbacks.put(id, stepCallback);
- if (validateCallback != null) {
- validateCallbacks.put(id, validateCallback);
+ @Override
+ public CompletableFuture<Either<QuickPickStep, InputBoxStep>>
step(InputCallbackParams params) {
+ Callback callback = registry.callbacks.get(params.getInputId());
+ if (callback == null) {
+ return CompletableFuture.completedFuture(null);
}
- return id;
+ return callback.step(params).handle((step, ex) -> {
+ if (ex != null || step == null) {
+ registry.callbacks.remove(params.getInputId());
+ }
+ return step;
+ });
}
- public void unregisterInput(String inputId) {
- stepCallbacks.remove(inputId);
- validateCallbacks.remove(inputId);
+ @Override
+ public CompletableFuture<String> validate(InputCallbackParams params) {
+ Callback callback = registry.callbacks.get(params.getInputId());
+ return callback != null ? callback.validate(params) :
CompletableFuture.completedFuture(null);
}
- @Override
- public CompletableFuture<Either<QuickPickStep, InputBoxStep>>
step(InputCallbackParams params) {
- Function<InputCallbackParams, CompletableFuture<Either<QuickPickStep,
InputBoxStep>>> callback = stepCallbacks.get(params.getInputId());
- return callback != null ? callback.apply(params) :
CompletableFuture.completedFuture(null);
+ public Registry getRegistry() {
+ return registry;
}
- @Override
- public CompletableFuture<String> validate(InputCallbackParams params) {
- Function<InputCallbackParams, CompletableFuture<String>> callback =
validateCallbacks.get(params.getInputId());
- return callback != null ? callback.apply(params) :
CompletableFuture.completedFuture(null);
+ private static class RegistryImpl implements Registry {
+
+ private final Map<String, Callback> callbacks = new HashMap<>();
+ private final AtomicInteger cnt = new AtomicInteger();
+
+ @Override
+ public String registerInput(Callback callback) {
+ String id = "ID:" + cnt.incrementAndGet();
+ callbacks.put(id, callback);
Review Comment:
synchronized access to the map ? Callbacks may eventually complete in
another thread, so the eventual remove() might happen concurrently.
##########
platform/openide.dialogs/apichanges.xml:
##########
@@ -26,6 +26,20 @@
<apidef name="dialogs">Dialogs API</apidef>
</apidefs>
<changes>
+ <change>
+ <api name="dialogs"/>
+ <summary>NotifyDescriptor.ComposedInput added</summary>
+ <version major="7" minor="63"/>
+ <date day="7" month="6" year="2022"/>
+ <author login="dbalek"/>
+ <compatibility addition="yes" binary="compatible" source="compatible"
semantic="compatible" deprecation="no" deletion="no" modification="no"/>
+ <description>
+ <p>
+ Added <code>ComposedInput</code> providing a composed input
of multiple nested selection lists and/or input lines.
Review Comment:
nitpick: maybe "chained" instead of "nested"
##########
platform/openide.dialogs/src/org/openide/NotifyDescriptor.java:
##########
@@ -1326,4 +1336,110 @@ public void setSelected(boolean selected) {
}
}
}
+
+ /** Notification providing a composed input of multiple nested selection
lists and/or input lines.
+ * @since 7.63
+ */
+ public static final class ComposedInput extends NotifyDescriptor {
+
+ private final List<NotifyDescriptor> inputs = new ArrayList<>();
+ private final Callback callback;
+ private int total;
+
+ /** Construct dialog with the specified title and nested inputs.
+ * @param title title of the dialog
+ * @param total total number of nested inputs
+ * @param callback callback used to create nested inputs
+ * @since 7.63
+ */
+ public ComposedInput(final String title, final int total, final
Callback callback) {
+ super(null, title, OK_CANCEL_OPTION, PLAIN_MESSAGE, null, null);
+ this.callback = callback;
+ this.total = total;
+ }
+
+ /**
+ * Total number of nested inputs.
+ * @since 7.63
+ */
+ public int getTotalInputs() {
+ return total;
Review Comment:
The total number of steps is fixed for the whole 'composed' process. But
quite often response in a step determines if a subsequent step is relevant at
all - often a total number (better: estimated number) of steps changes as user
fills inputs.
So I'd allow to change the total steps here, `NotifyDescriptor` has
add/removePropertyChangeListener, so the change can be observed + reflected in
the UI ?
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/ui/NotifyDescriptorAdapter.java:
##########
@@ -342,20 +352,21 @@ public <T extends NotifyDescriptor> CompletableFuture<T>
clientNotifyCompletion(
return ctrl;
} else if (descriptor instanceof NotifyDescriptor.QuickPick) {
NotifyDescriptor.QuickPick qp = (NotifyDescriptor.QuickPick)
descriptor;
- Map<QuickPickItem, NotifyDescriptor.QuickPick.Item> items = new
HashMap<>();
+ List<QuickPickItem> items = new ArrayList<>();
for (NotifyDescriptor.QuickPick.Item item : qp.getItems()) {
- items.put(new QuickPickItem(item.getLabel(),
item.getDescription(), null, item.isSelected(), null), item);
+ items.add(new QuickPickItem(item.getLabel(),
item.getDescription(), null, item.isSelected(),
Integer.toString(System.identityHashCode(item))));
Review Comment:
is identityHashCode safe here ?
##########
platform/openide.dialogs/src/org/openide/NotifyDescriptor.java:
##########
@@ -1326,4 +1336,110 @@ public void setSelected(boolean selected) {
}
}
}
+
+ /** Notification providing a composed input of multiple nested selection
lists and/or input lines.
+ * @since 7.63
+ */
+ public static final class ComposedInput extends NotifyDescriptor {
+
+ private final List<NotifyDescriptor> inputs = new ArrayList<>();
+ private final Callback callback;
+ private int total;
+
+ /** Construct dialog with the specified title and nested inputs.
+ * @param title title of the dialog
+ * @param total total number of nested inputs
+ * @param callback callback used to create nested inputs
+ * @since 7.63
+ */
+ public ComposedInput(final String title, final int total, final
Callback callback) {
+ super(null, title, OK_CANCEL_OPTION, PLAIN_MESSAGE, null, null);
+ this.callback = callback;
+ this.total = total;
+ }
+
+ /**
+ * Total number of nested inputs.
+ * @since 7.63
+ */
+ public int getTotalInputs() {
+ return total;
+ }
+
+ /**
+ * Lazy creates nested input of the given ordinal.
+ * @param number input number from interval <1, totalInputs+1>
+ * @return nested selection list, input line, or null
+ * @since 7.63
+ */
+ public NotifyDescriptor getInput(int number) {
+ NotifyDescriptor step = callback.getInput(number);
+ if (step != null) {
Review Comment:
If called twice with the same ordinal it will insert the same step twice (?)
##########
platform/openide.dialogs/src/org/openide/NotifyDescriptor.java:
##########
@@ -1326,4 +1336,110 @@ public void setSelected(boolean selected) {
}
}
}
+
+ /** Notification providing a composed input of multiple nested selection
lists and/or input lines.
+ * @since 7.63
+ */
+ public static final class ComposedInput extends NotifyDescriptor {
+
+ private final List<NotifyDescriptor> inputs = new ArrayList<>();
+ private final Callback callback;
+ private int total;
+
+ /** Construct dialog with the specified title and nested inputs.
+ * @param title title of the dialog
+ * @param total total number of nested inputs
+ * @param callback callback used to create nested inputs
+ * @since 7.63
+ */
+ public ComposedInput(final String title, final int total, final
Callback callback) {
+ super(null, title, OK_CANCEL_OPTION, PLAIN_MESSAGE, null, null);
+ this.callback = callback;
+ this.total = total;
+ }
+
+ /**
+ * Total number of nested inputs.
+ * @since 7.63
+ */
+ public int getTotalInputs() {
+ return total;
+ }
+
+ /**
+ * Lazy creates nested input of the given ordinal.
+ * @param number input number from interval <1, totalInputs+1>
+ * @return nested selection list, input line, or null
+ * @since 7.63
+ */
+ public NotifyDescriptor getInput(int number) {
+ NotifyDescriptor step = callback.getInput(number);
+ if (step != null) {
+ if (number - 1 < inputs.size()) {
+ inputs.set(number - 1, step);
+ } else if (number - 1 == inputs.size()) {
+ inputs.add(step);
+ } else {
+ return null;
+ }
+ if (number >= total) {
+ total = number;
+ }
+ }
+ return step;
+ }
+
+ /**
+ * Returns all created nested inputs.
+ * @since 7.63
+ */
+ public NotifyDescriptor[] getInputs() {
+ return inputs.toArray(new NotifyDescriptor[0]);
+ }
+
+ @Override
+ public Object getMessage() {
+ Object msg = super.getMessage();
+ if (msg != null) {
+ return msg;
+ }
+ JPanel panel = new JPanel();
+ panel.setOpaque (false);
+ panel.setLayout(new java.awt.GridBagLayout());
+
+ NotifyDescriptor input;
+ int i = 0;
+ java.awt.GridBagConstraints gridBagConstraints = null;
+ while ((input = getInput(++i)) != null) {
+ gridBagConstraints = new java.awt.GridBagConstraints();
+ gridBagConstraints.gridx = 0;
+ gridBagConstraints.gridy = i - 1;
+ gridBagConstraints.gridwidth =
java.awt.GridBagConstraints.RELATIVE;
+ gridBagConstraints.gridheight =
java.awt.GridBagConstraints.RELATIVE;
+ gridBagConstraints.anchor =
java.awt.GridBagConstraints.FIRST_LINE_START;
+ panel.add((JPanel)input.getMessage(), gridBagConstraints);
+ }
+ if (gridBagConstraints != null) {
+ gridBagConstraints.weighty = 1.0;
+ }
+
+ this.setMessage(panel);
+ return panel;
+ }
+
+ /**
+ * Callback used to lazy create nested inputs.
+ * @since 7.63
+ */
+ public static interface Callback {
+
+ /**
+ * Lazy creates nested input of the given ordinal.
+ * @param number input ordinal from interval <1,
totalInputs+1>
+ * @return nested selection list, input line, or null
+ * @since 7.63
+ */
+ public NotifyDescriptor getInput(int number);
Review Comment:
Maybe add the `ComposedInput` instance as a parameter: avoids
chicken-and-egg since callback is passed to the constructor of
NotifyDescriptor. The handler might change the overall validity (through
createNotificationLineSupport) - and maybe adjust the total number of steps.
I am still thinking about the ordinal as the identification of the
composedinput state; with multiple possible paths through the composition, the
Callback will need to make different comparisons for different paths - it would
be better to have fixed/constant IDs for individual constituents.
I.e. if the `ComposedInput` had a `String stepId`, it would be easier to
`switch` or branch on the 'current id' regardless of whether preceding
composition members were skipped or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
