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

workspace: Fix set last_active_center_pane error #22604

Closed

Conversation

CharlesChen0823
Copy link
Contributor

Fix an bug set last_active_center_pane error.

Release Notes:

  • N/A

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jan 3, 2025
@SomeoneToIgnore
Copy link
Contributor

Again, a PR with zero context.

Which bug? How to reproduce this so people can test that you've fixed it?

Let's try again.

@CharlesChen0823
Copy link
Contributor Author

emmm, with no context, because IMO, there only three line in this function, and this logic is straightforward.

    fn set_active_pane(&mut self, pane: &View<Pane>, cx: &mut ViewContext<Self>) {
+     self.last_active_center_pane = Some(self.active_pane.downgrade());
        self.active_pane = pane.clone();
        self.active_item_path_changed(cx);
-      self.last_active_center_pane = Some(pane.downgrade());

    }

set last_active_center_pane with same with active_pane?

I found this problem, when I try fix and edge case in pr #22563

@SomeoneToIgnore

@SomeoneToIgnore
Copy link
Contributor

And while altering this "straightforward logic", 4 multiplayer tests have failed:
https://github.com/zed-industries/zed/actions/runs/12594612720/job/35102417600

so, I do not think it's that simple.

@SomeoneToIgnore
Copy link
Contributor

Here's an example of a "trivial" change that's well-described: #22608

While I do not insist you writing that much, 0 elaboration, repeated, coupled with failing tests, is repulsive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants