Navigation Menu

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: wezterm cli activate-pane-direction now repaints #3303

Merged
merged 5 commits into from Mar 23, 2023
Merged

Conversation

ir-ae
Copy link
Contributor

@ir-ae ir-ae commented Mar 21, 2023

Closes #2879
I have no idea if this change is good or not but from testing it out it seems to fix the issue I was having (the activated pane not changing highlight) as well as other strange behavior around pane content overflowing into other panes when calling activate-pane-direction with one pane zoomed

Copy link
Owner

@wez wez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for diving in!

I think the general idea sounds fine, but the current implementation is a bit of a fluke!

I have a suggestion for you to try!

mux/src/tab.rs Outdated
@@ -1417,6 +1417,8 @@ impl TabInner {
if let Some(panel_idx) = self.get_pane_direction(direction, false) {
self.set_active_idx(panel_idx);
}
let mux = Mux::get();
mux.notify(MuxNotification::WindowInvalidated(self.id));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WindowInvalidated is for a window, but here we are a tab.
This likely just so happens to work because the ids for both of these start at 0.

What you may want to try instead is the PaneOutput notification, and emit it for both the prior active and the newly active pane ids.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having trouble getting PaneOutput to work correctly. It does fix the issue of only working for the initial window, however it results in this error each time wezterm cli activate-pane-direction is called:

22:47:57.991  ERROR  wezterm_mux_server_impl::local > encoding PDU to client: writing pdu data buffer: An existing connection was forcibly closed by the remote host. (os error 10054)

I'm really not sure what's causing it, nor how to fix it as it seems to happen regardless of how I call notify with PaneOutput.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you share the latest version of your code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed my changes, although my latest attempt was to try to send PaneOutput to every pane.
Some other things I've tried include:

  • using panel_idx (not sure if 'pane' and 'panel' are different in this context)
  • using self.iter_panes().iter().find(|pane| pane.is_active).
  • hard coding a value (just to check)

They each result in the same error.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try this instead?:

diff --git a/mux/src/tab.rs b/mux/src/tab.rs
index 89a256878..4b5922395 100644
--- a/mux/src/tab.rs
+++ b/mux/src/tab.rs
@@ -1416,6 +1416,11 @@ impl TabInner {
         }
         if let Some(panel_idx) = self.get_pane_direction(direction, false) {
             self.set_active_idx(panel_idx);
+
+            let mux = Mux::get();
+            if let Some(window_id) = mux.window_containing_tab(self.id) {
+                mux.notify(MuxNotification::WindowInvalidated(window_id));
+            }
         }
     }

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I think panel_idx may be a typo. I don't remember writing that!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works!

@wez wez merged commit da7e29d into wez:main Mar 23, 2023
17 checks passed
wez added a commit that referenced this pull request Mar 23, 2023
@wez
Copy link
Owner

wez commented Mar 23, 2023

Thanks for confirming!

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.

Window doesn't re-render gui when 'cli activate-pane-direction' is called
2 participants