Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli/list-keybinds: add pretty printing #2052

Merged
merged 7 commits into from
Aug 6, 2024

Conversation

rockorager
Copy link
Collaborator

@rockorager rockorager commented Aug 6, 2024

Add pretty printing to the +list-keybinds command. This is done by
bringing in a dependency on libvaxis to handle the styling. Pretty
printing happens automatically when printing to a tty, and can be
disabled either by redirecting output or using the flag --plain

Fixes #2048

Add pretty printing to the +list-keybinds command. This is done by
bringing in a dependency on libvaxis to handle the styling. Pretty
printing happens automatically when printing to a tty, and can be
disabled either by redirecting output or using the flag `--plain`
@rockorager rockorager changed the title pretty print cli/list-keybinds: add pretty printing Aug 6, 2024
@rockorager
Copy link
Collaborator Author

This is fully functional, but the alignment could use some work I think - we need some consensus on how these should be aligned for best viewing experience.

@rockorager
Copy link
Collaborator Author

The current output looks like this:

image

These oses don't supply a tty layer, which prevents us from using the
libvaxis tty. Eventually we can add in using stdout as a writer. For
now, we just don't pretty print there.
@deviantsemicolon
Copy link

deviantsemicolon commented Aug 6, 2024

The current output looks like this:

image

I'm satisfied with this. The alignment could definitely use some work though. But good job :)

@kareigu
Copy link
Collaborator

kareigu commented Aug 6, 2024

Running this on my mac breaks the terminal output completely afterwards

image

Exception is running +list-keybinds again that works like normal but everything else is broken

Deinit the tty and vaxis to restore the terminal upon exiting the
command
@rockorager
Copy link
Collaborator Author

Doh! Forgot to deinit. Should be fixed now

When on windows, set some default terminal size. The actual size is not
very important to our use case here, but we do need one
@kareigu
Copy link
Collaborator

kareigu commented Aug 6, 2024

For the alignment, maybe keeping everything else the same but pushing all of the action names to be at the same alignment.

Here I set them to be longest_encountered_col + widest_key + 2.

Looks a lot better with them not jumping around imo, and it's really trivial to implement since you already sorted them based on modifier count.

image

@rockorager
Copy link
Collaborator Author

That looks much better! Can you push a commit to this PR?

@kareigu
Copy link
Collaborator

kareigu commented Aug 6, 2024

I can't access your fork so probably not.

Here's the patch however:

From 8960b54f81d62810cf6f0991f617416870b6a847 Mon Sep 17 00:00:00 2001
From: karei <mail@karei.dev>
Date: Tue, 6 Aug 2024 22:06:19 +0300
Subject: cli/list-keybinds: align actions at the same column

---
 src/cli/list_keybinds.zig | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/cli/list_keybinds.zig b/src/cli/list_keybinds.zig
index 8abd6b0c..e40d9e3a 100644
--- a/src/cli/list_keybinds.zig
+++ b/src/cli/list_keybinds.zig
@@ -123,6 +123,8 @@ fn prettyPrint(alloc: Allocator, keybinds: Config.Keybinds) !u8 {
     const alt_style: vaxis.Style = .{ .fg = .{ .index = 3 } };
     const shift_style: vaxis.Style = .{ .fg = .{ .index = 4 } };
 
+    var longest_col: usize = 0;
+
     // Print the list
     for (bindings.items) |bind| {
         win.clear();
@@ -155,6 +157,8 @@ fn prettyPrint(alloc: Allocator, keybinds: Config.Keybinds) !u8 {
         // nice alignment no matter what was printed for mods
         _ = try win.printSegment(.{ .text = key }, .{ .col_offset = result.col });
 
+        if (longest_col < result.col) longest_col = result.col;
+
         const action = try std.fmt.allocPrint(alloc, "{}", .{bind.action});
         // If our action has an argument, we print the argument in a different color
         if (std.mem.indexOfScalar(u8, action, ':')) |idx| {
@@ -162,9 +166,9 @@ fn prettyPrint(alloc: Allocator, keybinds: Config.Keybinds) !u8 {
                 .{ .text = action[0..idx] },
                 .{ .text = action[idx .. idx + 1], .style = .{ .dim = true } },
                 .{ .text = action[idx + 1 ..], .style = .{ .fg = .{ .index = 5 } } },
-            }, .{ .col_offset = result.col + widest_key + 2 });
+            }, .{ .col_offset = longest_col + widest_key + 2 });
         } else {
-            _ = try win.printSegment(.{ .text = action }, .{ .col_offset = result.col + widest_key + 2 });
+            _ = try win.printSegment(.{ .text = action }, .{ .col_offset = longest_col + widest_key + 2 });
         }
         try vx.prettyPrint(writer);
     }
-- 
2.46.0


@mitchellh mitchellh merged commit 64c267a into ghostty-org:main Aug 6, 2024
1 of 4 checks passed
@mitchellh mitchellh deleted the pretty-print branch August 6, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

better output for +list-keybinds
4 participants