Mercurial > hg > thermostat

changeset 2323:48fd6f717c4f

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fix bug where CompleterServices could be added to unexpected command options Reviewed-by: jerboaa Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-May/019040.html Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-June/019063.html
author Andrew Azores <aazores@redhat.com>
date Wed, 01 Jun 2016 11:41:22 -0400
parents 52d7d3d03fa7
children 6521ebfe69cd
files launcher/src/main/java/com/redhat/thermostat/launcher/internal/TabCompletion.java launcher/src/test/java/com/redhat/thermostat/launcher/internal/TabCompletionTest.java
diffstat 2 files changed, 90 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/TabCompletion.java	Tue May 31 12:43:36 2016 -0400
+++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/TabCompletion.java	Wed Jun 01 11:41:22 2016 -0400
@@ -49,6 +49,7 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -99,17 +100,20 @@
 
     private void addCompleterServiceImpl(TreeCompleter.Node command, CompleterService service) {
         for (Map.Entry<CliCommandOption, ? extends TabCompleter> entry : service.getOptionCompleters().entrySet()) {
-            if (entry.getKey() == CliCommandOption.POSITIONAL_ARG_COMPLETION) {
+            CliCommandOption cliCommandOption = entry.getKey();
+            if (cliCommandOption == CliCommandOption.POSITIONAL_ARG_COMPLETION) {
                 TreeCompleter.Node node = new TreeCompleter.Node(command.getTag() + " completer", entry.getValue());
                 node.setRestartNode(command);
                 command.addBranch(node);
             }
-            for (TreeCompleter.Node branch : command.getBranches()) {
-                Set<String> completerOptions = getCompleterOptions(entry.getKey());
-                if (completerOptions.contains(branch.getTag())) {
-                    TreeCompleter.Node node = new TreeCompleter.Node(command.getTag() + " completer", entry.getValue());
-                    node.setRestartNode(command);
-                    branch.addBranch(node);
+            if (completerIsApplicable(cliCommandOption, command)) {
+                for (TreeCompleter.Node branch : command.getBranches()) {
+                    if (branch.getTag().equals("-" + cliCommandOption.getOpt())
+                            || branch.getTag().equals("--" + cliCommandOption.getLongOpt())) {
+                        TreeCompleter.Node node = new TreeCompleter.Node(command.getTag() + " completer", entry.getValue());
+                        node.setRestartNode(command);
+                        branch.addBranch(node);
+                    }
                 }
             }
         }
@@ -129,23 +133,30 @@
 
     private void removeCompleterServiceImpl(TreeCompleter.Node command, CompleterService service) {
         for (Map.Entry<CliCommandOption, ? extends TabCompleter> entry : service.getOptionCompleters().entrySet()) {
-            if (entry.getKey() == CliCommandOption.POSITIONAL_ARG_COMPLETION) {
+            CliCommandOption cliCommandOption = entry.getKey();
+            if (cliCommandOption == CliCommandOption.POSITIONAL_ARG_COMPLETION) {
                 command.removeByTag(command.getTag() + " completer");
             }
             for (TreeCompleter.Node branch : command.getBranches()) {
-                Set<String> completerOptions = getCompleterOptions(entry.getKey());
-                if (completerOptions.contains(branch.getTag())) {
+                if (branch.getTag().equals("-" + cliCommandOption.getOpt())
+                        || branch.getTag().equals("--" + cliCommandOption.getLongOpt())) {
                     command.removeByTag(branch.getTag());
                 }
             }
         }
     }
 
-    private static Set<String> getCompleterOptions(CliCommandOption option) {
-        Set<String> options = new HashSet<>();
-        options.add(LONG_OPTION_PREFIX + option.getLongOpt());
-        options.add(SHORT_OPTION_PREFIX + option.getOpt());
-        return options;
+    private static boolean completerIsApplicable(CliCommandOption option, TreeCompleter.Node node) {
+        boolean matchesLongOpt = false;
+        boolean matchesShortOpt = false;
+        for (TreeCompleter.Node branch : node.getBranches()) {
+            if (branch.getTag().equals("--" + option.getLongOpt())) {
+                matchesLongOpt = true;
+            } else if (branch.getTag().equals("-" + option.getOpt())) {
+                matchesShortOpt = true;
+            }
+        }
+        return matchesLongOpt && matchesShortOpt;
     }
 
     public void setupTabCompletion(ConsoleReader reader, CommandInfoSource commandInfoSource, BundleContext context, ClientPreferences prefs) {
--- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/TabCompletionTest.java	Tue May 31 12:43:36 2016 -0400
+++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/TabCompletionTest.java	Wed Jun 01 11:41:22 2016 -0400
@@ -52,7 +52,6 @@
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.logging.Logger;
 
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
@@ -68,14 +67,12 @@
 public class TabCompletionTest {
 
     private TabCompletion tabCompletion;
-    private Logger log;
     private TreeCompleter treeCompleter;
     private Map<String, TreeCompleter.Node> commandMap;
 
     @Before
     @SuppressWarnings("unchecked")
     public void setup() {
-        log = mock(Logger.class);
         treeCompleter = mock(TreeCompleter.class);
         commandMap = new HashMap<>();
         tabCompletion = new TabCompletion(treeCompleter, commandMap);
@@ -144,6 +141,70 @@
         }
     }
 
+    @Test
+    public void testAddCompleterWithConflictingShortOpt() {
+        ConsoleReader reader = mock(ConsoleReader.class);
+
+        Options mockOptions = new Options();
+        mockOptions.addOption("a", "agentId", true, "Agent ID");
+
+        Options fakeOptions = new Options();
+        fakeOptions.addOption("a", "all", false, "all data");
+
+        CommandInfo mockCommand = mock(CommandInfo.class);
+        when(mockCommand.getName()).thenReturn("mock-command");
+        when(mockCommand.getBundles()).thenReturn(Collections.<BundleInformation>emptyList());
+        when(mockCommand.getDescription()).thenReturn("description");
+        when(mockCommand.getEnvironments()).thenReturn(EnumSet.of(Environment.CLI, Environment.SHELL));
+        when(mockCommand.getOptions()).thenReturn(mockOptions);
+
+        CommandInfo fakeCommand = mock(CommandInfo.class);
+        when(fakeCommand.getName()).thenReturn("fake-command");
+        when(fakeCommand.getBundles()).thenReturn(Collections.<BundleInformation>emptyList());
+        when(fakeCommand.getDescription()).thenReturn("description");
+        when(fakeCommand.getEnvironments()).thenReturn(EnumSet.of(Environment.CLI, Environment.SHELL));
+        when(fakeCommand.getOptions()).thenReturn(fakeOptions);
+
+        CommandInfoSource infoSource = mock(CommandInfoSource.class);
+        when(infoSource.getCommandInfos()).thenReturn(Arrays.asList(mockCommand, fakeCommand));
+        when(infoSource.getCommandInfo(mockCommand.getName())).thenReturn(mockCommand);
+        when(infoSource.getCommandInfo(fakeCommand.getName())).thenReturn(fakeCommand);
+
+        BundleContext context = mock(BundleContext.class);
+
+        ClientPreferences prefs = mock(ClientPreferences.class);
+
+        CompleterService service = mock(CompleterService.class);
+        when(service.getCommands()).thenReturn(TabCompletion.ALL_COMMANDS_COMPLETER);
+        Map completerMap = new HashMap();
+        TabCompleter completer = mock(TabCompleter.class);
+        completerMap.put(new CliCommandOption("a", "agentId", true, "Agent ID", false), completer);
+        when(service.getOptionCompleters()).thenReturn(completerMap);
+
+        tabCompletion.addCompleterService(service);
+        tabCompletion.setupTabCompletion(reader, infoSource, context, prefs);
+
+        TreeCompleter.Node mockNode = commandMap.get("mock-command");
+        assertThat(mockNode, is(not(equalTo(null))));
+        for (TreeCompleter.Node branch : mockNode.getBranches()) {
+            if (branch.getTag().equals("--agentId")) {
+                assertThat(branch.getBranches(), is(not(equalTo(Collections.<TreeCompleter.Node>emptyList()))));
+            } else if (branch.getTag().equals("-a")) {
+                assertThat(branch.getBranches(), is(not(equalTo(Collections.<TreeCompleter.Node>emptyList()))));
+            }
+        }
+
+        TreeCompleter.Node fakeNode = commandMap.get("fake-command");
+        assertThat(fakeNode, is(not(equalTo(null))));
+        for (TreeCompleter.Node branch : fakeNode.getBranches()) {
+            if (branch.getTag().equals("--all")) {
+                assertThat(branch.getBranches(), is(equalTo(Collections.<TreeCompleter.Node>emptyList())));
+            } else if (branch.getTag().equals("-a")) {
+                assertThat(branch.getBranches(), is(equalTo(Collections.<TreeCompleter.Node>emptyList())));
+            }
+        }
+    }
+
     private void doSetupTabCompletion() {
         ConsoleReader reader = mock(ConsoleReader.class);