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

Update pane index when moving panes in same tab #6029

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

scauligi
Copy link

Fixes #6028.
Not sure if this is the best solution, but it works.

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 this!

I have a suggestion that I think makes the flow clearer overall, and which will be more robust against other changes that might mutate the tab contents in the future

mux/src/domain.rs Outdated Show resolved Hide resolved
mux/src/domain.rs Outdated Show resolved Hide resolved
@scauligi
Copy link
Author

I think this change is in line with what you're suggesting? There's a minor complication in that the pre-removal pane_index is needed to calculate split_size, but the post-removal pane_index is what we need for the final tab.split_and_insert call.

@wez
Copy link
Owner

wez commented Sep 22, 2024

Yeah, this is what I was thinking. The complication you mentioned just sounds like how this works now, or did you mean that phrasing the code this way introduces another issue?

@scauligi
Copy link
Author

scauligi commented Oct 6, 2024

Whoops! Lost track of this and forgot to reply.
The "complication" was in reference to the fact that the new pane index is needed for the tab.split_and_insert call, so recalculating it inside the nested scope wouldn't work since we need the new index in the outer scope (which my latest change should be taking care of).

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.

split-pane --move-pane-id can kill panes
2 participants