Allow empty splits on panes by iff · Pull Request #40245 · zed-industries/zed

iff · 2025-10-15T11:09:06Z

Draft as a base for continuing the discussion in #8008 : adds a SplitOperation enum to support bindings like ["pane::SplitLeft", {"operation": "Clear"}]

To be discussed @MrSubidubi and others:

Naming: Generally not happy with names yet and specifically Empty is unclear, e.g., what does this mean for terminal panes? Added placeholder code to split without cloning, but unsure what users would expect in this case.
~~I removed SplitAndMoveXyz actions but I guess we should keep them for backwards compatibility?~~
May have missed details in the move implementation. Will check the code again for opportunities to refactor more code after we agree on the approach.
~~Tests should go to crates/collab/src/tests/integration_tests.rs?~~

Closes #8008

Release Notes:

Add pane::Split mode ({ClonePane,EmptyPane,MovePane}) to allow creating an empty buffer.

- adding an enum for SplitBehavior (naming not final) to support bindings like `["pane::SplitLeft", {"behavior": "Empty"}]` - subsuming SplitAndMoveDirection actions but probably have to keep them (?) - empty behavior for terminal panes: unclear what this means. Currently split but does not clone.

zelenenka · 2025-11-13T09:16:22Z

@iff thanks for the PR! are you still working on this?

iff · 2025-11-13T09:21:19Z

Yup. Just waiting for feedback.

zelenenka · 2025-11-13T09:31:37Z

@iff got it, thanks. i'll ping people internally!

MrSubidubi · 2025-11-17T08:01:56Z

Sorry for the long delay from my side here:

Naming: Do see the issue, but lack much better ideas currently. We could perhaps instead of "behavior" name it "operation", which might make things more clear, but other than that, lack good ideas currently as well. That said, we can also be more verbose with the names if that helps, just a thought.
We very much should keep these around, sadly, because we need these for the command palette to show up. Fine to just keep them as aliases for the specific variants possible then (e.g., on SplitAndMoveRight, you just invoke your method with that enum variant`.
Regarding tests: Would be best to not have them there - best case they would go into pane.rs as well. Feel free to copy anything you need for that into these tests from elsewhere and feel free to ping me if there are any other questions.

Other than that, sorry again for the long delay in review and thank you for picking this up!

iff · 2025-11-17T11:47:52Z

@MrSubidubi no worries at all and completely understandable. Thanks for the feedback - I will work on integrating the feedback and finishing the implementation this week.

iff · 2025-11-20T11:47:03Z

@MrSubidubi Maybe it is a good time for a first round of feedback? I could use some guidance with respects to unittests and coverage you expect, and there are still some minor comments in the code that I can resolve after getting feedback.

MrSubidubi · 2025-11-26T15:39:16Z

@iff Sorry for not getting to this thus far! Am only working part-time and could not find the time this week yet. Will try to get to this later this week, next Monday at latest. Sorry!

iff · 2025-11-26T17:37:16Z

@MrSubidubi No worries at all! I completely understand.. I am also juggling part‑time work, so I know how quickly things can fill up. Take the time you need - there’s absolutely no rush. Looking forward to hearing from you whenever you’re able to get to it.

MrSubidubi

Two days delayed. sorry. Some thoughts, please ping me if you feel I am missing something.

crates/terminal_view/src/terminal_panel.rs

MrSubidubi · 2025-12-03T17:36:32Z

crates/workspace/src/pane.rs

+// TODO naming
+#[derive(Clone, PartialEq, Debug, Deserialize, JsonSchema, Default)]
+#[serde(deny_unknown_fields)]
+pub enum SplitOperation {


I see the TODO, but still lack better ideas

SplitKind is meh IMO but

SplitOperationKind is a bit long

Do you have a better idea?

I will spend some time trying to come up with alternatives..

@MrSubidubi It is hard to escape previous ideas. SplitOperationKind is long, but is not excessively so (e.g., ResourceOperationKind is also used). If we want to be short, the only options I can think of are SplitEffect or SplitMode?

The Clear enum variant is still bothering me. Fundamentally, the issue stems from terminal and editor panes not sharing the same behavior for opening a "new and empty" pane (terminal pane is never empty). I do not have a good idea for a term that captures the behavior for both panes - aside from the very generic New..

I think SplitMode might also be good, still captures this quite well I think.

We could also do something like

enum SplitMode { CloneItem, MoveItem, EmptyPane // or NewPane }

but then, not sure whether that is actually better. Happy to go with Clear if you think the above is worse, I'll also think a bit more about it

Sorry I missed this earlier, but I actually prefer your suggested naming. I would make all variants end with Pane (e.g., ClonePane). Or is Item a term you use in connection with panes? EmptyPane still feels a bit odd for a terminal pane, since it’s never truly empty (there’s always a prompt), but it’s probably good enough.

@MrSubidubi pushed a commit changing the name to SplitMode (and mode in SplitXYZ)

crates/workspace/src/pane.rs

Co-authored-by: Finn Evers <finn.evers@outlook.de>

iff · 2025-12-11T13:09:30Z

@MrSubidubi Do you want me to resolve the comments I have addressed, or should I keep them open as a reminder to check those changes?

MrSubidubi · 2025-12-15T08:18:05Z

I can resolve them one by one, good idea.

One thing before the last review and before I get this in (thank you so much already for being persistent here and doing awesome work): We desperately need the comments for the action structs - we use them throughout the app to provide some sort of context on these actions, so we should definitely not remove them (e.g. you can test this by searching for the pane actions in the keymap editor and hovering on the action name).

Sorry for mentioning this only now, could you find a somewhat functioning way to re-add these?

iff · 2025-12-15T11:05:40Z

Ah, yeah - that makes sense. I’ll fix it later today and resolve the merge conflicts in terminal_panel.rs.

iff · 2025-12-15T15:05:04Z

@MrSubidubi I initially missed that the Action derive macro requires a literal. I think the change above is the minimal fix to make it work (I get the docstring on hover in the keymap editor). Let me know if you’d prefer another solution (for example, adapting the Action macro)

MrSubidubi

Very much like the solution you came up with, thank you very much!

One last question that is not necessarily blocking and I think we are good to merge

MrSubidubi · 2025-12-10T09:38:30Z

crates/workspace/src/pane.rs

+// TODO naming
+#[derive(Clone, PartialEq, Debug, Deserialize, JsonSchema, Default)]
+#[serde(deny_unknown_fields)]
+pub enum SplitOperation {


I think SplitMode might also be good, still captures this quite well I think.

We could also do something like

enum SplitMode { CloneItem, MoveItem, EmptyPane // or NewPane }

but then, not sure whether that is actually better. Happy to go with Clear if you think the above is worse, I'll also think a bit more about it

crates/workspace/src/pane.rs

MrSubidubi · 2025-12-18T21:33:20Z

Sigh, sorry for the merge conflict coming in. Could you resolve that? No need for it to be today, but I'll try to get back to you over the weekend/next week at latest

iff · 2025-12-19T06:03:32Z

No problem. I’ll try to finish it up by the end of today.

- user intended to add an empty panel where the current item is and move the item to the other direction - still needs testing and one check in the unit tests is missing

MrSubidubi

Nice. Thank you so much for bearing with me, absolutely awesome job on this. Good improvement, good code and a pleasure to discuss this with you.

Sorry for me being a bit unresponsive at the start and inbetween and thank you for being fine with it. Hope to see you again in another PR perhaps, other than that, happy holidays!

@MrSubidubi

Draft as a base for continuing the discussion in #8008 : adds a `SplitOperation` enum to support bindings like `["pane::SplitLeft", {"operation": "Clear"}]` To be discussed @MrSubidubi and others: - Naming: Generally not happy with names yet and specifically `Empty` is unclear, e.g., what does this mean for terminal panes? Added placeholder code to split without cloning, but unsure what users would expect in this case. - ~~I removed `SplitAndMoveXyz` actions but I guess we should keep them for backwards compatibility?~~ - May have missed details in the move implementation. Will check the code again for opportunities to refactor more code after we agree on the approach. - ~~Tests should go to `crates/collab/src/tests/integration_tests.rs`?~~ Closes #8008 Release Notes: - Add `pane::Split` mode (`{ClonePane,EmptyPane,MovePane}`) to allow creating an empty buffer. --------- Co-authored-by: Finn Evers <finn.evers@outlook.de> Co-authored-by: MrSubidubi <finn@zed.dev>

@MrSubidubi

Draft as a base for continuing the discussion in zed-industries#8008 : adds a `SplitOperation` enum to support bindings like `["pane::SplitLeft", {"operation": "Clear"}]` To be discussed @MrSubidubi and others: - Naming: Generally not happy with names yet and specifically `Empty` is unclear, e.g., what does this mean for terminal panes? Added placeholder code to split without cloning, but unsure what users would expect in this case. - ~~I removed `SplitAndMoveXyz` actions but I guess we should keep them for backwards compatibility?~~ - May have missed details in the move implementation. Will check the code again for opportunities to refactor more code after we agree on the approach. - ~~Tests should go to `crates/collab/src/tests/integration_tests.rs`?~~ Closes zed-industries#8008 Release Notes: - Add `pane::Split` mode (`{ClonePane,EmptyPane,MovePane}`) to allow creating an empty buffer. --------- Co-authored-by: Finn Evers <finn.evers@outlook.de> Co-authored-by: MrSubidubi <finn@zed.dev>

@MrSubidubi

Draft as a base for continuing the discussion in zed-industries#8008 : adds a `SplitOperation` enum to support bindings like `["pane::SplitLeft", {"operation": "Clear"}]` To be discussed @MrSubidubi and others: - Naming: Generally not happy with names yet and specifically `Empty` is unclear, e.g., what does this mean for terminal panes? Added placeholder code to split without cloning, but unsure what users would expect in this case. - ~~I removed `SplitAndMoveXyz` actions but I guess we should keep them for backwards compatibility?~~ - May have missed details in the move implementation. Will check the code again for opportunities to refactor more code after we agree on the approach. - ~~Tests should go to `crates/collab/src/tests/integration_tests.rs`?~~ Closes zed-industries#8008 Release Notes: - Add `pane::Split` mode (`{ClonePane,EmptyPane,MovePane}`) to allow creating an empty buffer. --------- Co-authored-by: Finn Evers <finn.evers@outlook.de> Co-authored-by: MrSubidubi <finn@zed.dev>

@MrSubidubi

Draft as a base for continuing the discussion in zed-industries#8008 : adds a `SplitOperation` enum to support bindings like `["pane::SplitLeft", {"operation": "Clear"}]` To be discussed @MrSubidubi and others: - Naming: Generally not happy with names yet and specifically `Empty` is unclear, e.g., what does this mean for terminal panes? Added placeholder code to split without cloning, but unsure what users would expect in this case. - ~~I removed `SplitAndMoveXyz` actions but I guess we should keep them for backwards compatibility?~~ - May have missed details in the move implementation. Will check the code again for opportunities to refactor more code after we agree on the approach. - ~~Tests should go to `crates/collab/src/tests/integration_tests.rs`?~~ Closes zed-industries#8008 Release Notes: - Add `pane::Split` mode (`{ClonePane,EmptyPane,MovePane}`) to allow creating an empty buffer. --------- Co-authored-by: Finn Evers <finn.evers@outlook.de> Co-authored-by: MrSubidubi <finn@zed.dev>

cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Oct 15, 2025

iff marked this pull request as draft October 15, 2025 11:09

MrSubidubi self-assigned this Oct 15, 2025

iff added 9 commits November 17, 2025 12:57

Merge remote-tracking branch 'upstream/main' into split-behavior

69805f6

cargo fmt

6ecca3c

fix merge and re-add split and move

9e48210

pane: sketching some tests

23ececd

pane: add test helper

b40fdd4

pane: add a more generic test helper

d815940

rename behavior to operation

772b156

rename empty split operation to clear

e2b045c

cleanup

94af766

iff marked this pull request as ready for review November 20, 2025 11:27

zelenenka added this to Quality Week – December 2025 Nov 27, 2025

github-project-automation bot moved this to Community PRs in Quality Week – December 2025 Nov 27, 2025

MrSubidubi reviewed Dec 3, 2025

View reviewed changes

iff and others added 5 commits December 4, 2025 08:01

terminal_panel: matches instead of match/unreachable

fe5c7a8

Co-authored-by: Finn Evers <finn.evers@outlook.de>

terminal_pane: convention and naming

427745a

pane: make SplitOperation Copy

245e30a

pane: remove split operations impls

cd76c31

Merge branch 'main' into split-behavior

8103d97

pane: macro for split structs

70d82a2

iff added 3 commits December 15, 2025 14:30

Merge remote-tracking branch 'upstream/main' into split-behavior

c96e6be

pane: doc strings for SplitOperation and macro

f131db1

doc needs to be literal

7d8501a

MrSubidubi approved these changes Dec 15, 2025

View reviewed changes

iff added 2 commits December 18, 2025 09:02

rename SplitOperation -> SplitMode

95a7d86

rename split struct field to mode

aa23d63

iff added 3 commits December 19, 2025 11:53

Merge remote-tracking branch 'upstream/main' into split-behavior

5580b31

MovePane on single pane results in EmptyPane in opposite direction

4f87c96

- user intended to add an empty panel where the current item is and move the item to the other direction - still needs testing and one check in the unit tests is missing

pane: cleanup tests

70a3566

iff requested a review from MrSubidubi December 19, 2025 21:18

MrSubidubi added 3 commits December 21, 2025 23:14

Merge branch 'main' into split-behavior

3c5cb11

Formatting

2a2984c

Format and tidy a bit

f32e333

MrSubidubi approved these changes Dec 21, 2025

View reviewed changes

And now imagine I could type...

16e3e20

MrSubidubi enabled auto-merge (squash) December 21, 2025 23:41

MrSubidubi merged commit 3b626c8 into zed-industries:main Dec 21, 2025
23 checks passed

github-project-automation bot moved this from Community PRs to Done in Quality Week – December 2025 Dec 21, 2025

iff deleted the split-behavior branch December 22, 2025 11:46

Uh oh!

Conversation

iff commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zelenenka commented Nov 13, 2025

Uh oh!

iff commented Nov 13, 2025

Uh oh!

zelenenka commented Nov 13, 2025

Uh oh!

MrSubidubi commented Nov 17, 2025

Uh oh!

iff commented Nov 17, 2025

Uh oh!

iff commented Nov 20, 2025

Uh oh!

MrSubidubi commented Nov 26, 2025

Uh oh!

iff commented Nov 26, 2025

Uh oh!

MrSubidubi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MrSubidubi Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

iff Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

iff Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MrSubidubi Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

iff Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

iff Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iff commented Dec 11, 2025

Uh oh!

MrSubidubi commented Dec 15, 2025

Uh oh!

iff commented Dec 15, 2025

Uh oh!

iff commented Dec 15, 2025

Uh oh!

MrSubidubi left a comment

Choose a reason for hiding this comment

Uh oh!

MrSubidubi Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MrSubidubi commented Dec 18, 2025

Uh oh!

iff commented Dec 19, 2025

Uh oh!

MrSubidubi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

iff commented Oct 15, 2025 •

edited

Loading

iff Dec 9, 2025 •

edited

Loading