Feature/merge layers with corner glyphs

[hollowhemlock]

#191 Pull request.

[caksoylar]

Can you split adding the corner legends to a separate PR? That would resolve #151.

[caksoylar]

Thanks, I like the approach of a separate command to intentionally merge and output another keymap yaml.

In general I am not a big fan of combined diagrams because they are hard to read, but it might be useful to have the functionality for some cases. I'll review if you can split as above.

[hollowhemlock]

Awesome, will do.

[hollowhemlock]

commit dd711a2 is identical to #193 pull request

[hollowhemlock]

Also, I like the combined diagrams because they take less real estate when printing for easy reference.

[caksoylar]

Can you rebase this when you get a chance?

[hollowhemlock]

yeah

[hollowhemlock]

Rebased to main and dropped the 1 commit related to #193. Ran the following and still functions.

cd examples/stacked

# With combos stacked
poetry run keymap -c config.yaml stack-layers --center colemak_dh --tl fun --tr sys --bl num --br nav \
  ferris_sweep.yaml --include-combos colemak_dh nav num -o stacked.yaml
poetry run keymap -c config.yaml draw -k "ferris/sweep" stacked.yaml -o stacked.svg

# With combos as separate layer (--separate-combo-layer flag)
poetry run keymap -c config.yaml stack-layers --center colemak_dh --tl fun --tr sys --bl num --br nav \
  ferris_sweep.yaml --include-combos colemak_dh nav num --separate-combo-layer -o stacked.yaml
poetry run keymap -c config.yaml draw -k "ferris/sweep" stacked.yaml -o stacked.svg

[urob]

Wow, this is great! I wish I had seen this before hacking my own solution for this.

I want to look at this more closely, but I wanted to raise one design-level question first for discussion. I was wondering whether it might be possible to get something both cleaner and more versatile by extending the existing parse command, rather than introducing a new stack-layers command.

More concretely, my impression is that most of the functionality could be obtained from three additional parse options. I haven't thought through the implementation, so take this with a grain of salt. But from a design perspective the following would make the most sense to me.

1/ Layer inclusion/exclusion control

I'd propose adding two command line options (config options would also word, but CLI flags have the advantage of allowing reuse of the same config for different outputs):

--exclude-layers {list}
--include-layers {list}

This seems useful not just for stacked layouts, but more generally -- for example, when generating detailed write-ups that describe only certain parts of a keymap at a time.

2/ Stack layer control

For the stacking behavior itself, I could imagine a --stack-layer-to option that:

1. Overwrites the pos position of the target layer with the tap position (or a specified source position) of a src layer
2. Automatically adds the src layer to the --exclude-layers

--stack-layer-to {src} [{src_pos}] {target} {pos}

3/ Optional: show combos on separate layer

This one is more optional (there's a single one-line yq workaround in my hack), but for layouts with many combos it could be quite useful. The idea would be to move all combos to a layer other than the ones they’re defined on (typically a virtual layer created via the existing --virtual-layers flag). Again, this seems potentially useful beyond stacked layouts (for example, I also use it for my full layer-by-layer keymap).

--move-combos-to {layer} [{source_layers}]

The optional {source_layers} argument would restrict which included layers combos are moved from.

---

In combination, those options could be used roughly like this:

keymap -c config.yaml parse -z base.keymap \
    --virtual-layers Combos      \
    --move-combos-to Combos      \
    --exclude-layers Mouse       \
    --stack-layer-to Nav Base tr \
    --stack-layer-to Fun Base tl \
    --stack-layer-to Num Base bl \
    --stack-layer-to Sys Base br \
    >keymap.yaml

With the caveat that I haven’t yet dug into the implementation details, this approach seems like it might reduce the maintenance and cognitive overhead of adding a separate command, while also giving users more flexibility and reducing the need for additional config files.

Curious what you and @caksoylar think.

[hollowhemlock]

I'm open to CLI flags if @caksoylar is.

I'm unfamiliar with cli naming conventions. I think flag naming should be optimized for source to destination ordering. I prefer a suffix of "from" for stack-layer because the fist argument after the flag is the source.

 keymap -c config.yaml parse -z base.keymap \
      --virtual-layers Combos          \
      --move-combos-to Combos   \
      --exclude-layers Mouse           \
      --stack-layer-from Nav Base tr   \
      --stack-layer-from Fun Base tl   \
      --stack-layer-from Num Base bl   \
      --stack-layer-from Sys Base br   \
      >keymap.yaml

[caksoylar]

Thanks for the thoughts to both, and sorry I haven't properly reviewed the PR. This year has been busy.

Some thoughts:

• I like that the command is separate; I don't want to overload parse and a command dedicated to restructuring the keymap yaml makes sense to me. We can more generically call it keymap rearrange and add different functionality, all taking an input keymap yaml and outputting another one.
• It'd be great to have the flexibility to specify arbitrary source->target mappings, as would be afforded by --stack-layer-* arguments proposed.
  • To that end, I like this interface (without a preposition): --move-layer {src} [{src_pos}] {target} {pos} (not sure if argparse will love this; we might convert it to {src}[:{src_pos}] {target}[:{pos}] for a fixed nargs. not sure if it likes repeated options either, worth checking)
  • {target} doesn't have to exist, it can be created empty if not in keymap
  • This supersedes --center, --tl etc.
• We can add the combo layer functionality (for combos without draw_separate) via --move-combos [{source_layers}] {layer_name} which can be specified multiple times, and source_layers can be '*' or omitted for all
• We should still have an --exclude-layers that will simply ignore the layers, and all other layers will stay as is unless they are moved by --move-layer (or their combos via --move-combos)
  • We can also add --include-layers as a filter that applies before everything else
• In the future we could move some functionality from other commands to this one, like keymap parse --virtual-layers and --select-layers, --keys-only, --combos-only, --ghost-keys from keymap draw

To that end, here is a final interface stub to replace the original stack-layers functionality (let me know if I am missing something):

 keymap rearrange \
      --move-combos Combos   \
      --exclude-layers Mouse \
      --move-layer Nav Base tr   \
      --move-layer Fun Base tl   \
      --move-layer Num Base bl   \
      --move-layer Sys Base br   \
      keymap.yaml >keymap_arranged.yaml

[hollowhemlock]

I like it. Separating the commands retains a "source of truth" as reference. The rearrange command could cause unintended results and this will lead to easier end user debugging.

Questions

1. What do you think about keymap transform instead of keymap rearrange. It has a more math feel to it and is easier to type.

[caksoylar]

Sure, transform seems OK (and a bit more generic).

[hollowhemlock]

Cool. I'll give it a go in the coming weeks.