CommandMap: recurse to resolve command sequences properly

src/core/Command.cc

@@ -1,2 +1,14 @@
 #include "Command.h"
 #include <string.h>
+
+bool Command::Unit::operator==(const Command::Unit & other) const
+{
+    return (id == other.id) &&
+        (count == other.count) &&
+        (following_char == other.following_char);
+}
+
+bool Command::operator==(const Command & other) const
+{
+    return (main == other.main) && (motion == other.motion);
+}

src/core/Command.h

@@ -47,10 +47,14 @@ public:
             count = 0u;
             following_char = 0u;
         }
+
+        bool operator==(const Unit & other) const;
     };
 
     Unit main;
     Unit motion;
+
+    bool operator==(const Command & other) const;
 };
 
 #endif

src/core/CommandMap.cc

@@ -35,44 +35,42 @@ void CommandMap::add(const char * s, uint32_t id,
 uint8_t CommandMap::lookup_command(const uint32_t * command_characters,
         size_t length, Command & command) const
 {
-    const Node * node = &m_root_node;
+    std::list<Command::Unit> units;
-    for (size_t unit_index = 0u; unit_index < 2u; unit_index++)
+    uint8_t scan_result = scan_units(units, command_characters, length, &m_root_node);
+    if (scan_result == COMMAND_COMPLETE)
     {
-        size_t consumed_length = 0u;
+        int i = 0;
-        Command::Unit & unit = (unit_index == 0u) ? command.main : command.motion;
+        for (const auto & unit : units)
-        uint8_t unit_status = extract_command_unit(command_characters,
-                length, unit, node, &node, &consumed_length);
-        switch (unit_status)
         {
-        case UNIT_INVALID:
+            switch (i)
-            return COMMAND_INVALID;
+            {
-        case UNIT_NEED_MORE_CHARACTERS:
+            case 0:
-            return COMMAND_NEED_MORE_CHARACTERS;
+                command.main = unit;
-        case UNIT_COMPLETE:
+                break;
-            return COMMAND_COMPLETE;
+            case 1:
-        case UNIT_COMPLETE_NEED_ANOTHER:
+                command.motion = unit;
-            command_characters += consumed_length;
+                break;
-            length -= consumed_length;
+            }
-            break;
+            i++;
-        default:
-            return COMMAND_INVALID;
         }
     }
-    return COMMAND_INVALID;
+    return scan_result;
 }
 
-uint8_t CommandMap::extract_command_unit(const uint32_t * command_characters,
+uint8_t CommandMap::scan_units(
-        size_t length, Command::Unit & unit, const Node * start_node,
+        std::list<Command::Unit> & units,
-        const Node ** next_node, size_t * consumed_length) const
+        const uint32_t * command_characters, size_t length,
+        const Node * start_node) const
 {
     if (length < 1u)
-        return UNIT_NEED_MORE_CHARACTERS;
+        return COMMAND_INCOMPLETE;
+    uint8_t result = COMMAND_INCOMPLETE;
+    uint8_t recurse_result = COMMAND_INVALID;
     bool gathering_count = true;
-    unit.count = 0u;
+    Command::Unit unit;
     const Node * node = start_node;
     for (size_t i = 0u; i < length; i++)
     {
-        *consumed_length = i + 1u;
         uint32_t c = command_characters[i];
         if (gathering_count && ('0' <= c) && (c <= '9'))
         {
@@ -93,28 +91,36 @@ uint8_t CommandMap::extract_command_unit(const uint32_t * command_characters,
                     {
                         if (i < (length - 1u))
                         {
-                            *consumed_length = i + 2u;
                             unit.following_char = command_characters[i + 1u];
-                            return UNIT_COMPLETE;
+                            units.push_front(unit);
+                            return COMMAND_COMPLETE;
                         }
-                        return UNIT_NEED_MORE_CHARACTERS;
+                        return COMMAND_INCOMPLETE;
                     }
                     else if (node->next_map)
                     {
-                        *next_node = &node->next_map->m_root_node;
+                        recurse_result = scan_units(units,
-                        return UNIT_COMPLETE_NEED_ANOTHER;
+                                &command_characters[i + 1u],
+                                length - i - 1u, &node->next_map->m_root_node);
+                        if (recurse_result == COMMAND_COMPLETE)
+                        {
+                            units.push_front(unit);
+                            return COMMAND_COMPLETE;
+                        }
+                        result = recurse_result;
                     }
                     else
                     {
-                        return UNIT_COMPLETE;
+                        units.push_front(unit);
+                        return COMMAND_COMPLETE;
                     }
                 }
             }
             else
             {
-                return UNIT_INVALID;
+                return recurse_result;
             }
         }
     }
-    return UNIT_NEED_MORE_CHARACTERS;
+    return result;
 }

src/core/CommandMap.h

@@ -2,8 +2,8 @@
 #define COMMANDMAP_H
 
 #include <unordered_map>
-#include <vector>
 #include <memory>
+#include <list>
 #include "Command.h"
 
 class CommandMap
@@ -12,7 +12,7 @@ public:
     enum : uint8_t
     {
         COMMAND_INVALID,
-        COMMAND_NEED_MORE_CHARACTERS,
+        COMMAND_INCOMPLETE,
         COMMAND_COMPLETE,
     };
 
@@ -44,17 +44,10 @@ protected:
 
     Node m_root_node;
 
-    enum : uint8_t
+    uint8_t scan_units(
-    {
+            std::list<Command::Unit> & units,
-        UNIT_INVALID,
+            const uint32_t * command_characters, size_t length,
-        UNIT_NEED_MORE_CHARACTERS,
+            const Node * start_node) const;
-        UNIT_COMPLETE,
-        UNIT_COMPLETE_NEED_ANOTHER,
-    };
-
-    uint8_t extract_command_unit(const uint32_t * command_characters,
-            size_t length, Command::Unit & unit, const Node * start_node,
-            const Node ** next_node, size_t * consumed_length) const;
 };
 
 #endif

test/src/test_CommandMap.cc

@@ -0,0 +1,162 @@
+#include "gtest/gtest.h"
+#include "CommandMap.h"
+#include <cstring>
+#include <vector>
+
+std::shared_ptr<CommandMap> build()
+{
+    auto dcm = std::make_shared<CommandMap>();
+    dcm->add("iw", 1, nullptr, false);
+    dcm->add("f", 2, nullptr, true);
+    auto cm = std::make_shared<CommandMap>();
+    cm->add("dd", 1, nullptr, false);
+    cm->add("f", 2, nullptr, true);
+    cm->add("d", 3, dcm, false);
+    return cm;
+}
+
+uint8_t lookup_command(std::shared_ptr<CommandMap> cm, const char * s, Command & command)
+{
+    std::vector<uint32_t> v;
+    while (*s)
+    {
+        v.push_back(*s);
+        s++;
+    }
+    return cm->lookup_command(&v[0], v.size(), command);
+}
+
+TEST(CommandMapTest, add_does_nothing_for_empty_sequence)
+{
+    auto cm = build();
+    cm->add("", 0, nullptr, false);
+}
+
+TEST(CommandMapTest, returns_invalid_for_invalid_first_level_sequences)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_INVALID, lookup_command(cm, "Q", command));
+}
+
+TEST(CommandMapTest, returns_invalid_for_invalid_second_level_sequences)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_INVALID, lookup_command(cm, "dx", command));
+}
+
+TEST(CommandMapTest, returns_incomplete_when_only_count_given)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_INCOMPLETE, lookup_command(cm, "42", command));
+}
+
+TEST(CommandMapTest, returns_incomplete_for_incomplete_sequence)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_INCOMPLETE, lookup_command(cm, "d", command));
+}
+
+TEST(CommandMapTest, returns_incomplete_when_waiting_for_following_character)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_INCOMPLETE, lookup_command(cm, "f", command));
+}
+
+TEST(CommandMapTest, returns_incomplete_when_only_count_given_for_second_unit)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_INCOMPLETE, lookup_command(cm, "d23", command));
+}
+
+TEST(CommandMapTest, returns_incomplete_for_empty_sequence)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_INCOMPLETE, lookup_command(cm, "", command));
+}
+
+TEST(CommandMapTest, finds_command_with_no_motion_or_following_character)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_COMPLETE, lookup_command(cm, "dd", command));
+    Command expected;
+    expected.main.id = 1;
+    EXPECT_EQ(expected, command);
+}
+
+TEST(CommandMapTest, includes_command_count_with_no_following_character)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_COMPLETE, lookup_command(cm, "4dd", command));
+    Command expected;
+    expected.main.count = 4;
+    expected.main.id = 1;
+    EXPECT_EQ(expected, command);
+}
+
+TEST(CommandMapTest, finds_command_with_following_character)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_COMPLETE, lookup_command(cm, "fx", command));
+    Command expected;
+    expected.main.id = 2;
+    expected.main.following_char = 'x';
+    EXPECT_EQ(expected, command);
+}
+
+TEST(CommandMapTest, includes_command_count_with_following_character)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_COMPLETE, lookup_command(cm, "30fx", command));
+    Command expected;
+    expected.main.count = 30;
+    expected.main.id = 2;
+    expected.main.following_char = 'x';
+    EXPECT_EQ(expected, command);
+}
+
+TEST(CommandMapTest, finds_command_with_motion)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_COMPLETE, lookup_command(cm, "diw", command));
+    Command expected;
+    expected.main.id = 3;
+    expected.motion.id = 1;
+    EXPECT_EQ(expected, command);
+}
+
+TEST(CommandMapTest, finds_command_with_motion_with_following_char)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_COMPLETE, lookup_command(cm, "dfx", command));
+    Command expected;
+    expected.main.id = 3;
+    expected.motion.id = 2;
+    expected.motion.following_char = 'x';
+    EXPECT_EQ(expected, command);
+}
+
+TEST(CommandMapTest, includes_motion_unit_count)
+{
+    auto cm = build();
+    Command command;
+    EXPECT_EQ(CommandMap::COMMAND_COMPLETE, lookup_command(cm, "d5fx", command));
+    Command expected;
+    expected.main.id = 3;
+    expected.motion.count = 5;
+    expected.motion.id = 2;
+    expected.motion.following_char = 'x';
+    EXPECT_EQ(expected, command);
+}