You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2022/06/14 14:12:26 UTC
[GitHub] [netbeans] sdedic commented on a diff in pull request #4203: NotifyDescriptor.ComposedInput added.
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists