mawww / kakoune

mawww's experiment for a better code editor
http://kakoune.org
The Unlicense
9.7k stars 705 forks source link

Add right hand side mapping to debug command output #5164

Closed dontlaugh closed 1 month ago

dontlaugh commented 2 months ago

The output likely differs from what a user writes in their kakrc file, but it seems to be a simple improvement on the status quo. Closes #5152

arachsys commented 1 month ago

This is a great idea; I could imagine adding the bindings to the info unconditionally like this, or just as a replacement for the docstring when one isn't otherwise set. At the moment, with this patch and a simple binding like

map global normal <#> ':comment-line<ret>'

the <ret> is being rendered in the *debug* buffer as a unicode surrogate. I think you might want to_string(k) instead of to_string(k.key)? This fixes that behaviour for me, and will also make sure things like < get rendered as <lt> to avoid confusion.

arachsys commented 1 month ago

PS The alternative version that shows the mapping only if the docstring isn't set might look like:

diff --git a/src/commands.cc b/src/commands.cc
index 373f3b5c..3c39fcc7 100644
--- a/src/commands.cc
+++ b/src/commands.cc
@@ -1671,9 +1671,14 @@ const CommandDesc debug_cmd = {
             for (auto mode : concatenated(modes, user_modes))
             {
                 KeymapMode m = parse_keymap_mode(mode, user_modes);
-                for (auto& key : keymaps.get_mapped_keys(m))
-                    write_to_debug_buffer(format(" * {} {}: {}",
-                                          mode, key, keymaps.get_mapping_docstring(key, m)));
+                for (auto& key : keymaps.get_mapped_keys(m)) {
+                   auto& docstring = keymaps.get_mapping_docstring(key, m);
+                   String mapping;
+                   for (auto& k : keymaps.get_mapping_keys(key, m))
+                       mapping += to_string(k);
+                   write_to_debug_buffer(format(" * {} {}: {}",
+                       mode, key, docstring.empty() ? mapping : docstring));
+                }
             }
         }
         else if (parser[0] == "regex")
dontlaugh commented 1 month ago

I think you might want to_string(k)...

Thanks for this. I don't know why I didn't notice the funny characters before.

Regarding your other comment, my preference would be to print the bindings in every case, docstring or not.

But really I defer to mawww, and would be happy with any improvement to this output. Whether it's my patch or someone else's.

arachsys commented 1 month ago

Yes, I'm not sure what mawww will make of it, but I've definitely pinched a variant of your idea for my local build as I found it really handy. Thanks! :)

mawww commented 1 month ago

This is for debug, so more info is probably better