Simplify greatly UI input handling

This round trip through an input callback expected to call
is_key_available and get_key was overcomplicated, just send the
keys as they arrive, the client is already buffering due to urgent
event mode.

src/client.cc

@@ -37,7 +37,12 @@ Client::Client(std::unique_ptr<UserInterface>&& ui,
     m_window->options().register_watcher(*this);
 
     m_ui->set_ui_options(m_window->options()["ui_options"].get<UserInterface::Options>());
-    m_ui->set_input_callback([this](EventMode mode) { handle_available_input(mode); });
+    m_ui->set_on_key([this](Key key) {
+        if (key == ctrl('c'))
+            killpg(getpgrp(), SIGINT);
+        else
+            m_pending_keys.push_back(key);
+    });
 
     m_window->hooks().run_hook("WinDisplay", m_window->buffer().name(), context());
 
@@ -50,54 +55,31 @@ Client::~Client()
     m_window->set_client(nullptr);
 }
 
-Optional<Key> Client::get_next_key(EventMode mode)
+void Client::process_pending_inputs()
 {
-    if (not m_pending_keys.empty())
-    {
-        Key key = m_pending_keys.front();
-        m_pending_keys.erase(m_pending_keys.begin());
-        return key;
-    }
-    if (mode != EventMode::Pending and m_ui->is_key_available())
-        return m_ui->get_key();
-    return {};
-}
-
-void Client::handle_available_input(EventMode mode)
-{
-    if (mode == EventMode::Urgent)
-    {
-        Key key = m_ui->get_key();
-        if (key == ctrl('c'))
-            killpg(getpgrp(), SIGINT);
-        else
-            m_pending_keys.push_back(key);
-        return;
-    }
-
     try
     {
         const bool debug_keys = (bool)(context().options()["debug"].get<DebugFlags>() & DebugFlags::Keys);
 
-        while (Optional<Key> key = get_next_key(mode))
+        // steal keys as we might receive new keys while handling them.
+        Vector<Key, MemoryDomain::Client> keys = std::move(m_pending_keys);
+        for (auto& key : keys)
         {
             if (debug_keys)
                 write_to_debug_buffer(format("Client '{}' got key '{}'",
-                                             context().name(), key_to_str(*key)));
+                                             context().name(), key_to_str(key)));
 
-            if (*key == ctrl('c'))
+            if (key == Key::FocusIn)
-                killpg(getpgrp(), SIGINT);
-            else if (*key == Key::FocusIn)
                 context().hooks().run_hook("FocusIn", context().name(), context());
-            else if (*key == Key::FocusOut)
+            else if (key == Key::FocusOut)
                 context().hooks().run_hook("FocusOut", context().name(), context());
-            else if (key->modifiers == Key::Modifiers::Resize)
+            else if (key.modifiers == Key::Modifiers::Resize)
             {
                 m_window->set_dimensions(m_ui->dimensions());
                 force_redraw();
             }
             else
-                m_input_handler.handle_key(*key);
+                m_input_handler.handle_key(key);
         }
     }
     catch (Kakoune::runtime_error& error)

src/client.hh

@@ -34,8 +34,7 @@ public:
 
     Client(Client&&) = delete;
 
-    // handle all the keys currently available in the user interface
+    void process_pending_inputs();
-    void handle_available_input(EventMode mode);
 
     void menu_show(Vector<DisplayLine> choices, BufferCoord anchor, MenuStyle style);
     void menu_select(int selected);

src/client_manager.cc

@@ -61,10 +61,10 @@ Client* ClientManager::create_client(std::unique_ptr<UserInterface>&& ui,
     return contains(m_clients, client) ? client : nullptr;
 }
 
-void ClientManager::handle_pending_inputs() const
+void ClientManager::process_pending_inputs() const
 {
     for (auto& client : m_clients)
-        client->handle_available_input(EventMode::Pending);
+        client->process_pending_inputs();
 }
 
 void ClientManager::remove_client(Client& client, bool graceful)

src/client_manager.hh

@@ -34,7 +34,7 @@ public:
     void add_free_window(std::unique_ptr<Window>&& window, SelectionList selections);
 
     void redraw_clients() const;
-    void handle_pending_inputs() const;
+    void process_pending_inputs() const;
 
     Client*  get_client_ifp(StringView name);
     Client&  get_client(StringView name);

src/event_manager.hh

@@ -18,7 +18,6 @@ enum class EventMode
 {
     Normal,
     Urgent,
-    Pending
 };
 
 class FDWatcher

src/json_ui.cc

@@ -184,18 +184,6 @@ void JsonUI::draw_status(const DisplayLine& status_line,
     rpc_call("draw_status", status_line, mode_line, default_face);
 }
 
-bool JsonUI::is_key_available()
-{
-    return not m_pending_keys.empty();
-}
-
-Key JsonUI::get_key()
-{
-    kak_assert(not m_pending_keys.empty());
-    Key key = m_pending_keys.front();
-    m_pending_keys.erase(m_pending_keys.begin());
-    return key;
-}
 
 void JsonUI::menu_show(ConstArrayView<DisplayLine> items,
                        DisplayCoord anchor, Face fg, Face bg,
@@ -231,11 +219,6 @@ void JsonUI::refresh(bool force)
     rpc_call("refresh", force);
 }
 
-void JsonUI::set_input_callback(InputCallback callback)
-{
-    m_input_callback = std::move(callback);
-}
-
 void JsonUI::set_ui_options(const Options& options)
 {
     // rpc_call("set_ui_options", options);
@@ -246,6 +229,11 @@ DisplayCoord JsonUI::dimensions()
     return m_dimensions;
 }
 
+void JsonUI::set_on_key(OnKeyCallback callback)
+{
+    m_on_key = std::move(callback);
+}
+
 using JsonArray = Vector<Value>;
 using JsonObject = IdMap<Value>;
 
@@ -384,7 +372,7 @@ void JsonUI::eval_json(const Value& json)
         for (auto& key_val : params)
         {
             for (auto& key : parse_keys(key_val.as<String>()))
-                m_pending_keys.push_back(key);
+                m_on_key(key);
         }
     }
     else if (method == "resize")
@@ -394,7 +382,7 @@ void JsonUI::eval_json(const Value& json)
 
         DisplayCoord dim{params[0].as<int>(), params[1].as<int>()};
         m_dimensions = dim;
-        m_pending_keys.push_back(resize(dim));
+        m_on_key(resize(dim));
     }
     else
         throw runtime_error("unknown method");
@@ -413,6 +401,9 @@ void JsonUI::parse_requests(EventMode mode)
         m_requests += StringView{buf, buf + size};
     }
 
+    if (not m_on_key)
+        return;
+
     while (not m_requests.empty())
     {
         const char* pos = nullptr;
@@ -435,9 +426,6 @@ void JsonUI::parse_requests(EventMode mode)
 
         m_requests = String{pos, m_requests.end()};
     }
-
-    while (m_input_callback and not m_pending_keys.empty())
-        m_input_callback(mode);
 }
 
 UnitTest test_json_parser{[]()

src/json_ui.hh

@@ -26,9 +26,6 @@ public:
                      const DisplayLine& mode_line,
                      const Face& default_face) override;
 
-    bool is_key_available() override;
-    Key  get_key() override;
-
     void menu_show(ConstArrayView<DisplayLine> items,
                    DisplayCoord anchor, Face fg, Face bg,
                    MenuStyle style) override;
@@ -42,18 +39,16 @@ public:
 
     void refresh(bool force) override;
 
-    void set_input_callback(InputCallback callback) override;
-
-    void set_ui_options(const Options& options) override;
-
     DisplayCoord dimensions() override;
+    void set_on_key(OnKeyCallback callback) override;
+    void set_ui_options(const Options& options) override;
 
 private:
     void parse_requests(EventMode mode);
     void eval_json(const Value& value);
 
-    InputCallback m_input_callback;
     FDWatcher m_stdin_watcher;
+    OnKeyCallback m_on_key;
     Vector<Key, MemoryDomain::Client> m_pending_keys;
     DisplayCoord m_dimensions;
     String m_requests;

src/main.cc

@@ -332,10 +332,8 @@ std::unique_ptr<UserInterface> make_ui(UIType ui_type)
         void draw(const DisplayBuffer&, const Face&, const Face&) override {}
         void draw_status(const DisplayLine&, const DisplayLine&, const Face&) override {}
         DisplayCoord dimensions() override { return {24,80}; }
-        bool is_key_available() override { return false; }
-        Key  get_key() override { return Key::Invalid; }
         void refresh(bool) override {}
-        void set_input_callback(InputCallback) override {}
+        void set_on_key(OnKeyCallback callback) override {}
         void set_ui_options(const Options&) override {}
     };
 
@@ -587,7 +585,7 @@ int run_server(StringView session, StringView init_command,
         {
             client_manager.redraw_clients();
             event_manager.handle_next_events(EventMode::Normal);
-            client_manager.handle_pending_inputs();
+            client_manager.process_pending_inputs();
             client_manager.clear_client_trash();
             client_manager.clear_window_trash();
             buffer_manager.clear_buffer_trash();

src/ncurses_ui.cc

@@ -220,8 +220,11 @@ void on_term_resize(int)
 
 NCursesUI::NCursesUI()
     : m_stdin_watcher{0, [this](FDWatcher&, EventMode mode) {
-        if (m_input_callback)
+        if (not m_on_key)
-            m_input_callback(mode);
+            return;
+
+        while (auto key = get_next_key())
+            m_on_key(*key);
       }},
       m_assistant(assistant_clippy),
       m_colors{
@@ -485,26 +488,19 @@ void NCursesUI::on_sighup()
     m_window = nullptr;
 }
 
-bool NCursesUI::is_key_available()
+Optional<Key> NCursesUI::get_next_key()
 {
     if (not m_window)
-        return false;
+        return {};
 
     check_resize();
 
     wtimeout(m_window, 0);
     const int c = wgetch(m_window);
-    if (c != ERR)
-        ungetch(c);
     wtimeout(m_window, -1);
-    return c != ERR;
-}
 
-Key NCursesUI::get_key()
+    if (c == ERR)
-{
+        return {};
-    check_resize();
-
-    const int c = wgetch(m_window);
 
     if (c == KEY_MOUSE)
     {
@@ -530,21 +526,21 @@ Key NCursesUI::get_key()
                 return res | Key::Modifiers::MousePos;
             };
 
-            return { get_modifiers(ev.bstate),
+            return Key{ get_modifiers(ev.bstate),
-                     encode_coord({ ev.y - (m_status_on_top ? 1 : 0), ev.x }) };
+                        encode_coord({ ev.y - (m_status_on_top ? 1 : 0), ev.x }) };
         }
     }
 
     if (c > 0 and c < 27)
     {
         if (c == control('m') or c == control('j'))
-            return Key::Return;
+            return {Key::Return};
         if (c == control('i'))
-            return Key::Tab;
+            return {Key::Tab};
         if (c == control('z'))
         {
             raise(SIGTSTP);
-            return Key::Invalid;
+            return {};
         }
         return ctrl(Codepoint(c) - 1 + 'a');
     }
@@ -557,8 +553,8 @@ Key NCursesUI::get_key()
             const Codepoint csi_val = wgetch(m_window);
             switch (csi_val)
             {
-                case 'I': return Key::FocusIn;
+                case 'I': return {Key::FocusIn};
-                case 'O': return Key::FocusOut;
+                case 'O': return {Key::FocusOut};
                 default: break; // nothing
             }
         }
@@ -570,28 +566,28 @@ Key NCursesUI::get_key()
             return alt(new_c);
         }
         else
-            return Key::Escape;
+            return {Key::Escape};
     }
     else switch (c)
     {
-    case KEY_BACKSPACE: case 127: return Key::Backspace;
+    case KEY_BACKSPACE: case 127: return {Key::Backspace};
-    case KEY_DC: return Key::Delete;
+    case KEY_DC: return {Key::Delete};
-    case KEY_UP: return Key::Up;
+    case KEY_UP: return {Key::Up};
-    case KEY_DOWN: return Key::Down;
+    case KEY_DOWN: return {Key::Down};
-    case KEY_LEFT: return Key::Left;
+    case KEY_LEFT: return {Key::Left};
-    case KEY_RIGHT: return Key::Right;
+    case KEY_RIGHT: return {Key::Right};
-    case KEY_PPAGE: return Key::PageUp;
+    case KEY_PPAGE: return {Key::PageUp};
-    case KEY_NPAGE: return Key::PageDown;
+    case KEY_NPAGE: return {Key::PageDown};
-    case KEY_HOME: return Key::Home;
+    case KEY_HOME: return {Key::Home};
-    case KEY_END: return Key::End;
+    case KEY_END: return {Key::End};
-    case KEY_BTAB: return Key::BackTab;
+    case KEY_BTAB: return {Key::BackTab};
     case KEY_RESIZE: return resize(m_dimensions);
     }
 
     for (int i = 0; i < 12; ++i)
     {
         if (c == KEY_F(i+1))
-            return Key::F1 + i;
+            return {Key::F1 + i};
     }
 
     if (c >= 0 and c < 256)
@@ -607,9 +603,10 @@ Key NCursesUI::get_key()
 
             WINDOW* window;
        };
-       return utf8::codepoint(getch_iterator{m_window}, getch_iterator{m_window});
+       return Key{utf8::codepoint(getch_iterator{m_window},
+                                  getch_iterator{m_window})};
     }
-    return Key::Invalid;
+    return {};
 }
 
 template<typename T>
@@ -954,16 +951,16 @@ void NCursesUI::mark_dirty(const Window& win)
     wredrawln(m_window, (int)win.pos.line, (int)win.size.line);
 }
 
+void NCursesUI::set_on_key(OnKeyCallback callback)
+{
+    m_on_key = std::move(callback);
+}
+
 DisplayCoord NCursesUI::dimensions()
 {
     return m_dimensions;
 }
 
-void NCursesUI::set_input_callback(InputCallback callback)
-{
-    m_input_callback = std::move(callback);
-}
-
 void NCursesUI::abort()
 {
     endwin();

src/ncurses_ui.hh

@@ -30,9 +30,6 @@ public:
                      const DisplayLine& mode_line,
                      const Face& default_face) override;
 
-    bool is_key_available() override;
-    Key  get_key() override;
-
     void menu_show(ConstArrayView<DisplayLine> items,
                    DisplayCoord anchor, Face fg, Face bg,
                    MenuStyle style) override;
@@ -46,11 +43,10 @@ public:
 
     void refresh(bool force) override;
 
-    void set_input_callback(InputCallback callback) override;
-
-    void set_ui_options(const Options& options) override;
 
     DisplayCoord dimensions() override;
+    void set_on_key(OnKeyCallback callback) override;
+    void set_ui_options(const Options& options) override;
 
     static void abort();
 
@@ -74,6 +70,8 @@ private:
                    ColumnCount col_index, ColumnCount max_column,
                    const Face& default_face);
 
+    Optional<Key> get_next_key();
+
     NCursesWin* m_window = nullptr;
 
     DisplayCoord m_dimensions;
@@ -119,8 +117,8 @@ private:
         InfoStyle style;
     } m_info;
 
-    FDWatcher     m_stdin_watcher;
+    FDWatcher m_stdin_watcher;
-    InputCallback m_input_callback;
+    OnKeyCallback m_on_key;
 
     bool m_status_on_top = false;
     ConstArrayView<StringView> m_assistant;

src/remote.cc

@@ -324,11 +324,10 @@ public:
 
     void refresh(bool force) override;
 
-    bool is_key_available() override;
+    DisplayCoord dimensions() override { return m_dimensions; }
-    Key  get_key() override;
-    DisplayCoord dimensions() override;
 
-    void set_input_callback(InputCallback callback) override;
+    void set_on_key(OnKeyCallback callback) override
+    { m_on_key = std::move(callback); }
 
     void set_ui_options(const Options& options) override;
 
@@ -336,46 +335,45 @@ public:
     Client* client() const { return m_client.get(); }
 
 private:
-    FDWatcher    m_socket_watcher;
+    FDWatcher     m_socket_watcher;
-    MsgReader    m_reader;
+    MsgReader     m_reader;
-    DisplayCoord m_dimensions;
+    DisplayCoord  m_dimensions;
-    InputCallback m_input_callback;
+    OnKeyCallback m_on_key;
 
     SafePtr<Client> m_client;
-    Optional<Key>   m_pending_key;
 };
 
 
 RemoteUI::RemoteUI(int socket, DisplayCoord dimensions)
     : m_socket_watcher(socket, [this](FDWatcher& watcher, EventMode mode) {
           const int sock = watcher.fd();
-          bool disconnect = false;
           try
           {
-              while (fd_readable(sock) and not m_reader.ready())
+              while (fd_readable(sock))
+              {
                   m_reader.read_available(sock);
 
-              if (m_reader.ready() and not m_pending_key)
+                  if (not m_reader.ready())
-              {
+                      continue;
-                   if (m_reader.type() == MessageType::Key)
+
+                   if (m_reader.type() != MessageType::Key)
                    {
-                       m_pending_key = m_reader.read<Key>();
+                      ClientManager::instance().remove_client(*m_client, false);
-                       m_reader.reset();
+                      return;
                    }
-                   else
+
-                       disconnect = true;
+                   auto key = m_reader.read<Key>();
+                   m_reader.reset();
+                   if (key.modifiers == Key::Modifiers::Resize)
+                       m_dimensions = key.coord();
+                   m_on_key(key);
               }
           }
           catch (const remote_error& err)
           {
               write_to_debug_buffer(format("Error while reading remote message: {}", err.what()));
-              disconnect = true;
-          }
-
-          if (disconnect)
               ClientManager::instance().remove_client(*m_client, false);
-          else if (m_pending_key and m_input_callback)
+          }
-              m_input_callback(mode);
       }),
       m_dimensions(dimensions)
 {
@@ -460,31 +458,6 @@ void RemoteUI::set_ui_options(const Options& options)
     msg.write(options);
 }
 
-bool RemoteUI::is_key_available()
-{
-    return (bool)m_pending_key;
-}
-
-Key RemoteUI::get_key()
-{
-    kak_assert(m_pending_key);
-    auto key = *m_pending_key;
-    m_pending_key.reset();
-    if (key.modifiers == Key::Modifiers::Resize)
-        m_dimensions = key.coord();
-    return key;
-}
-
-DisplayCoord RemoteUI::dimensions()
-{
-    return m_dimensions;
-}
-
-void RemoteUI::set_input_callback(InputCallback callback)
-{
-    m_input_callback = std::move(callback);
-}
-
 static sockaddr_un session_addr(StringView session)
 {
     sockaddr_un addr;
@@ -527,7 +500,10 @@ RemoteClient::RemoteClient(StringView session, std::unique_ptr<UserInterface>&&
         msg.write(env_vars);
     }
 
-    m_ui->set_input_callback([this](EventMode){ send_available_keys(); });
+    m_ui->set_on_key([this](Key key){
+        MsgWriter msg(m_socket_watcher->fd(), MessageType::Key);
+        msg.write(key);
+     });
 
     MsgReader reader;
     m_socket_watcher.reset(new FDWatcher{sock, [this, reader](FDWatcher& watcher, EventMode) mutable {
@@ -598,15 +574,6 @@ RemoteClient::RemoteClient(StringView session, std::unique_ptr<UserInterface>&&
     }});
 }
 
-void RemoteClient::send_available_keys()
-{
-    while (m_ui->is_key_available())
-    {
-        MsgWriter msg(m_socket_watcher->fd(), MessageType::Key);
-        msg.write(m_ui->get_key());
-    }
-}
-
 void send_command(StringView session, StringView command)
 {
     int sock = connect_to(session);

src/remote.hh

@@ -19,6 +19,7 @@ struct remote_error : runtime_error
 
 class FDWatcher;
 class UserInterface;
+struct Key;
 
 // A remote client handle communication between a client running on the server
 // and a user interface running on the local process.
@@ -29,8 +30,6 @@ public:
                  const EnvVarMap& env_vars, StringView init_command);
 
 private:
-    void send_available_keys();
-
     std::unique_ptr<UserInterface> m_ui;
     std::unique_ptr<FDWatcher>     m_socket_watcher;
 };

src/user_interface.hh

@@ -33,7 +33,7 @@ enum class InfoStyle
 
 enum class EventMode;
 
-using InputCallback = std::function<void(EventMode mode)>;
+using OnKeyCallback = std::function<void(Key key)>;
 
 class UserInterface
 {
@@ -60,12 +60,10 @@ public:
                              const Face& default_face) = 0;
 
     virtual DisplayCoord dimensions() = 0;
-    virtual bool is_key_available() = 0;
-    virtual Key  get_key() = 0;
 
     virtual void refresh(bool force) = 0;
 
-    virtual void set_input_callback(InputCallback callback) = 0;
+    virtual void set_on_key(OnKeyCallback callback) = 0;
 
     using Options = IdMap<String, MemoryDomain::Options>;
     virtual void set_ui_options(const Options& options) = 0;