Hi @charleslambert, thanks for opening the PR! ✨

Could we refactor this to not use a functor? I'd like the API to be as small/simple as possible.

I was thinking maybe we could have a type like (Event.key * model -> model) list so creating bindings becomes rather trivial. Something like this:

module Key_bindings : sig 
  type 'model t
  type 'model binding
= struct
 type t = 'model binding list
 type 'model binding = key * 'model -> 'model
 let on key fn = (key, fn)
 let make bs = bs

 let update key model t =
   match event with
   | KeyDown k -> 
      let fns = find_fns k t in
      run_fns fns model
   | _ -> model
end

let bindings = Key_bindings.make [
 on Key.Enter enter_fullscreen;
 on Key.Down move_left
]

What do you think?

I agree removing the functor is probably a good idea. I would still argue to keep the idea of a msg variant rather than directly binding to functions. It makes it easier for a component author to make changes without breaking a consumer of that component. It also gives direct documentation of the possible operations on a component. I will refactor to remove the functor and add a few convenience functions to make the api easier to consume.

Hey @leostera, I removed the functor and updated the api to hopefully be a little better.

What do you think?

Oh i see what you mean now! Love the update looking super clean 🤩

We also have the ability to send custom events which could work even more uniformly since you'd just have to add:

| Event.KeyDown _ as ev -> KeyMap.update m.keys ev

and this would be enough for they KeyMap to update itself and send a custom event to your app that you can match in your update loop:

| Event.Custom My_event -> (* do thing *)

Custom events are just typed process messages so you defined them with type Messgae.t = | My_event.

i'll have another look in a minute when i get to a computer but overall is looking like we could merge this.

thanks again for contributing ✨

After giving this some more thought I'm not sure what the custom events buy us that the function approach wouldn't. 🤔

It seems to me that not knowing what the model is makes it hard to map from Minttea events to actual behaviors in an app. Maybe I'm missing something here tho!

For example, say you're building a vim-style mini editor. You have a rough keymap of hjkl that likely works the same everywhere all the time, but if you are in "insert mode", then hjkl mean something different.

With the function keymap I can write this:

type model = {
  document : string;
  cursor : Cursor.t;
  mode : [ `insert | `normal ];
  keys : Key_map.t;
}

let exit_mode key m =
  match (key, m.mode) with
  | Event.Escape, `insert -> ({ m with mode = `normal }, Command.Noop)
  | _ -> (m, Command.Noop)

let handle_input key m =
  match (m.mode, key) with
  | `insert, Event.Key char -> ({ m with document = document ^ char }, Command.Noop)
  | `normal, Event.Key ("j" | "k" | "l" | "h") -> move_cursor key m

let keymap =
  Key_map.
    [
      on [ Escape ] exit_mode;
      on [ Up; Down; Left; Right ] move_cursor;
      on [ AnyKey ] handle_input;
    ]

and then my update function becomes a tidy call to Key_map.update

let update ev m =
  match ev with
  | Event.KeyDown _ -> Key_map.update m.keys ev m
  | _ -> (* ... handle other events in my app *)

This approach makes one (i think) reasonable assumption that your keymaps will be dependant on the model – it gives you a lot of flexibility and its rather easy to think about: you associate a little update function to a set of keys.

When I try to do the same thing with the custom event approach it looked like this:

(* same model and other functions from the last example *)

type key_ev = | Exit_mode | Move_cursor | Handle_input

let keymap =
  Key_map.
    [
      on [ Escape ] Exit_mode;
      on [ Up; Down; Left; Right ] Move_cursor;
      on [ AnyKey ] Handle_input;
    ]

let update ev m =
  match ev with
  | Event.KeyDown k -> 
     (match Key_map.find_match m.keys k with
      | Some Exit_mode -> exit_mode k m
      | Some Move_cursor -> move_cursor k m
      | Some Handle_input -> handle_input k m
      | None -> m)
  | _ -> (* ... handle other events in my app *)

Which seems to be repeating the dispatching code between the custom event (Exit_mode, Move_cursor, etc) and the functions that I want to trigger from them.

Maybe I'm missing something, so feel free to let me know if there's something about the custom events approach that I can't see here.

Do you see any drawbacks to the function approach? 🤔

Hi, just so you know, I am playing around with a toggleable help bar leave, and it requires some representation of key binding, so I am waiting on this to be completed before I submit the leave.

Hey @leostera, sorry for dropping off the earth. I am getting married in about a month or so and it has eaten most of my time in last couple of months.

I think I see the misunderstanding we are hitting here. The way I see it there are two types of keybindings, there are bindings you use in your application code and bindings made by components you use (3rd party). In the case of your application code it totally makes sense to use a function based style. The solution I have been working is more useful for components that don't want to expose their internals to the application layer and don't want to handle key presses directly.

For example:
let's say I have a list component that can move up and down. It can export a variant that describes all the messages that can be passed to it. The keymap feature takes that variant and allows the user of that component to define what keys should trigger actions in that component. The result is that we can pass the main minttea message (which is a keypress) to the component and it will have a keymap to translate into other actions.

Here is a code example of how I thought this would be used

module ExampleComponent = struct
  type msg = Upcase | Reset
  type model = { keymap : msg Key_map.t; value : string }

  let update event model =
    match event with
    | Event.KeyDown (key, _mod) -> (
        match Key_map.find_match key model.keymap with
        | Some Upcase ->
            ( { model with value = String.uppercase_ascii model.value },
              Command.Noop )
        | Some Reset -> ({ model with value = "hello world" }, Command.Noop)
        | _ -> (model, Command.Noop))
    | _ -> (model, Command.Noop)

  let view model = model.value
end

type model = { example_component_model : ExampleComponent.model }

let initial_model =
  ({
     example_component_model =
       {
         value = "hello world";
         keymap =
           (let open Key_map in
            [
              on
                ~help:{ key = "u"; desc = "upcase" }
                [ Minttea.Event.Key "u" ] ExampleComponent.Upcase;
              on
                ~help:{ key = "r"; desc = "reset text" }
                [ Minttea.Event.Key "r" ] ExampleComponent.Reset;
            ]);
       };
   }
    : model)

let init _ = Command.Noop

let update event model =
  let ec_model, _ec_cmd =
    ExampleComponent.update event model.example_component_model
  in
 
  (* Handle some other logic *)
  ({ example_component_model = ec_model }, Command.Noop)

let view model = ExampleComponent.view model.example_component_model
let app = Minttea.app ~init ~update ~view ()
let () = Minttea.start app ~initial_model

Does that make more sense?

Hi, @charleslambert I think you have a good point: having written a small component myself, I see the value of defining basic actions, but letting the user define the effective key presses.
I think if we don't have that, we will end up with ad hoc configuration ways for each component.
On the other hand, I agree with @leostera that the double abstraction may feel clunky on the user side.

Here is my two cent proposal.
Maybe we could devise instead two modules:

• Action: designed for component interface, it would map custom events and help messages to internal functions.
• Key_map: the user facing keymap handling, would just map key events to functions. It would be used by the user to make their own mappings directly, as well as triggering component actions.

The usage for the user could look something like that:

let keymap =
  Key_map.
    [
      on [ Escape ] exit_mode;  (* exit_mode is a callback *)
      on [ Left; Right ] move_cursor;
      on [ Up ] (action My_component.up);  (* action is a helper function in Key_map that emit the custom event *)
      on [ Down ] (action My_component.down);
      on [ AnyKey ] handle_input;
    ]

Compared to @leostera proposition, this one gives a straightforward way to configure component keymaps.
Compared to @charleslambert proposition, this one increases the level of indirection in the case of components, but it simplifies the user side.

Hey @Lattay, thank you for the input 😃. The only problem I see with that approach is that we would have to have all the mapped functions be imperative callbacks, as I don't think there would any way to generalize over their combined return types.

I guess you would have the functions be model -> model or 'something -> unit.

So the current implementation of bindings looks like:

type binding = {
  keys : Minttea.Event.key  list;
  help : help option;
  disabled : bool;
}

type 'a t = ('a * binding) list

We could add a variant type to account for the different types of (actions, functions)

type ('model, 'msg) binding_action =
  | Fn of ('model -> 'model)
  | Action of ('msg -> unit)

The problem with this type though is that it can't be generic for multiple msg variants. In other words you could not bind to msgs for more than one component in single keymap. It also requires that Action types be callbacks, which I don't think is ideal.