-
Notifications
You must be signed in to change notification settings - Fork 26
feat: suite of changes #73
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
base: main
Are you sure you want to change the base?
feat: suite of changes #73
Conversation
Changed tile label assignment from column-first to row-first order to eliminate duplicate labels when selecting tiles in the top half of the screen. This fixes an issue where letters like 'e' and 'u' would appear twice in the second-layer menu options when selecting from the top area of the screen. - Changed nested loops in tile_mode_render from column-first to row-first order - Updated idx_to_rect function to calculate positions with row-first indexing - Labels now appear in a more intuitive left-to-right, top-to-bottom pattern 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add screen edge detection to the division_4_or_8_render function to prevent labels from being cut off at screen edges: - For areas near the top edge, position both labels below the area - For areas near the bottom edge, position both labels above the area - For areas near the left edge, position both labels to the right (stacked vertically) - For areas near the right edge, position both labels to the left (stacked vertically) Labels are now stacked vertically rather than positioned by area height fraction in the left/right edge cases to avoid overlap with small rectangles. This improves usability by ensuring labels are always visible regardless of where the selection area is positioned on screen. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add a default case to the switch statement in print_result() to ensure the 'click' variable is always initialized before use. This fixes the compiler warning: warning: 'click' may be used uninitialized [-Wmaybe-uninitialized] The default case will use 'n' (none) for any unexpected click values. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This change adds a configuration option to make prefixes in tile mode adjacent to each other. When enabled, tiles with similar prefixes (like 'a', 'aa', 'ab') will be grouped together in the grid, making navigation more intuitive. Features: - Added a new prefix_adjacency boolean config option (defaults to false) - Implemented lexicographical sorting of tile labels when enabled - Added comprehensive documentation in config.example - Ensured proper bidirectional mapping between labels and screen positions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Thank you for your interest in the project. For "commit 1 (UI fix)", I'm not sure why you have this bug. This bug should have been fixed with the 0.4.1 release and I can't reproduce there. Do you have any custom configuration or just use a Dvorak layout? In any case, the AI went awry since the commit changed the wrong file for this mode. This would be in the For "commit 2" (UI fix), I like the idea and agree with your assessment. An easy solution might just be to get the check the bounds and if it goes outside the screen just put it at the edge then check if the other label would overlap and do the same except to the edge of the other label. For "commit 3 (compilation fix)" sure. The case where the value is not set shouldn't happen (unless there's a bug) thus why I didn't bother with this. This would probably be more adequate: case CLICK_NONE:
click = 'n';
break;
default:
LOG_ERR("Invalid `click` value. This should not have happened.")
click = 'n';
break;For "commit 4 (feature) adjacency", there shouldn't be any need to store the indices in a list. We just need to change the way the labels are generated to use the first letter as the least significant value. This means modifying the |
Overview
First: my apologies/caveat since this is one of my first PRs almost completely relying on LLM-generated changes. I made sure to preserve that information in the commit messages. However, I've been testing this branch lightly over the last 12h and I'd like to consider the ideas in the commits for review, even if the exact change set is undesirable for any number of reasons. Also, each of these commits is notionally independent, so I've done some work to extract each into its own branch off of
main(0.4.1). But, 2 of the 4 commits on this branch are interdependent (they affect the same file in dependent ways) socherry-picking wasn't clean when generating all the associated "one-commit branches. Please note that the entirety of the human interactions (prose) in this PR (including this description) will be generated manually by me.I only ask maintainers to consider the pain points and ideas, below. Given that I haven't reviewed the generated code, it may introduce new issues which I haven't considered. I'll outline the goal of each commit, below. If this entire PR branch as-is is accepted, then there's no need to consider the commits in isolation on their own branches. If certain changes need more review then we can drop any/all commits, here, and open separate PRs as-needed on a case-by-case basis.
Please note that I have not defined a
wl-kbptrconfiguration file. So, I'm using the default application logic/configuration according towl-kbptr's user documentation.Commit 1 (UX fix)
0.4.1
repeated_bisect_labels.mp4
With edfd6cd
unique_bisect_labels.mp4
Commit 2 (UI fix)
I'm finding that on at least one of my displays, the bisect labels run off the screen for those "rectangles" on the edge of the screen. Please see the screencasts below documenting the problem and the "fix". I don't love how the labels aren't positioned in the same row/column as the rectangles, exactly. But, the spirit of this change is correct: labels are always visible. I also saw that labels would be inconsistently embedded within squares at a given "level". So, toward the top of my screen, the bisect rectangles may have the text within the corresponding square, because it was deemed small enough to fit, but then at the bottom of my screen, the labels would surround the rectangle (not be placed within the square) even though the square and font size were the same - only the rectangle's position on the display was different. So, this commit also tries to make this behavior consistent.
0.4.1
labels_on_screen.mp4
589f2ab
labels_off_screen.mp4
Commit 3 (compilation fix)
This just fixes a compiler warning:
Commit 4 (feature)
I love this change/feature less than what I first created it, so no stress to discard. This change basically allows users to opt in to (
prefix_adjacency=[false|true]in their configuration file) a tile mode behavior where selectingafor something likealwill ensure thatakandamare adjacent toal. If this commit is placed on0.4.1(instead of on top ofedfd6cd193f75981938b2959f6b13c6348737dbf(commit #1)) then the adjacency will be vertical instead of horizontal, since that commit changes the layout ordering of tiles. Please see the screencast for what this looks like.0.4.1
tile_adjacency.mp4
8b99035
tile_no_adjacency.mp4