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

Fix broken thumbs rendering & race condition #151

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ian0371
Copy link

@ian0371 ian0371 commented Oct 25, 2023

Bug fix

There are several bugs that this PR addresses:

long line bug

This PR fixes the issue by calculating the row position based on the terminal's column size. Inspired by #139.

wrong position of hint

Multibyte character handling was incorrect, so as a starship user thumbs always printed abnormaly.
(mentioned by #76)

This PR attempts a new way to insert the hint in the appropriate position, which renders each line just once.
So it works well on all the samples including samples/test1.

As in the comment, it tries to find the position of the hint inside the line:

        /*
         *  off_left: <prefix[..hint.len] + hint> <hint> <text> <suffix>
         *      left: <prefix> <hint + text[hint.len..]> <suffix>
         *     right: <prefix> <text[..hint.len] + hint> <suffix>
         * off_right: <prefix> <text> <hint + suffix[hint.len..]>
         * respect the unicode while slicing
         */

race-condition in tmux-thumbs

There's a race condition between new-window command and swap-pane command. (also mentioned by #69 )

tmux new-window -P -F '#{pane_id}' -d -n '[thumbs]' "tmux capture-pane -J -t {active_pane_id} -p{scroll_params} | tail -n {height} | {dir}/target/release/thumbs -f '%U:%H' -t {tmp} {args}; tmux swap-pane -t {active_pane_id}; {zoom_command} tmux wait-for -S {signal}"

tmux swap-pane -d -s {active_pane_id} -t {thumbs_pane_id}

This is because the executor returns as soon as the new window is created and does not block until the whole command in the parameter tmux capture-pane ~~ is executed.

Assume the active pane is small. There is an issue for each of the execution flow:

  • swap-pane - capture-pane and `thumbs: Active pane is captured in the bigger pane (created by new-window), so more lines including the lines that were not visible in the small pane are captured. This causes rendering issues on thumbs.
  • capture-pane and thumbs - swap-pane: thumbs screen is rendered on the bigger pane, and then moved to the smaller pane, which causes long lines to be trimmed.

This PR tries to enforce the execution flow of the former, by inserting sleep before capture-pane (although this isn't strict. please tell me a better idea)
The issue mentioned is fixed by the new render function.

Issue with this PR

If osc52 is enabled, tmux enters search mode with blank screen after thumbs selection, so you must press enter one more time. I have no idea why this issue occurs, but it has to do something with the new rendering function.

I should mention that I'm new to rust (literally my first writing in rust), so if there's any quirk in the style please let me know. Thanks.

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.

None yet

1 participant