[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #1419: IGNITE-18239: Exclude 'compute' and 'raft' from completions on 'node …

[GitBox <gi...@apache.org>]

PakhomovAlexander commented on code in PR #1419:
URL: https://github.com/apache/ignite-3/pull/1419#discussion_r1042168129


##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/core/repl/completer/DynamicCompleterActivationPoint.java:
##########
@@ -55,10 +57,22 @@ public void activateDynamicCompleter(DynamicCompleterRegistry registry) {
         );
         registry.register(
                 CompleterConf.builder()
-                        .command("node", "config", "show")
-                        .command("node", "config", "update").build(),
+                        .command("node", "config", "show").build(),
                 nodeConfigDynamicCompleterFactory
         );
+
+        registry.register(
+                CompleterConf.builder()
+                        .command("node", "config", "update")
+                        .filter((unused, candidates) -> {

Review Comment:
   This is a good idea to define a filter as a lambda right here but this approach has an issue: it is not covered with unit tests. You can define a separate class like `NodeConfigUpdateFilter` and test it with unite tests.



##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/core/repl/completer/DynamicCompleterActivationPoint.java:
##########
@@ -55,10 +57,22 @@ public void activateDynamicCompleter(DynamicCompleterRegistry registry) {
         );
         registry.register(
                 CompleterConf.builder()
-                        .command("node", "config", "show")
-                        .command("node", "config", "update").build(),
+                        .command("node", "config", "show").build(),
                 nodeConfigDynamicCompleterFactory
         );
+
+        registry.register(
+                CompleterConf.builder()
+                        .command("node", "config", "update")
+                        .filter((unused, candidates) -> {
+                            Set<String> exclusions = Set.of("compute", "raft");

Review Comment:
   Could you check please the case: `compute.`, `raft.`



-- 
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@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org