Enable manual worktree organization

[eth0net]

Release Notes:

• Preserve order of worktrees in project (#10883).
• Enable drag-and-drop reordering for project worktrees

Note: worktree order is not synced during collaboration but guests can reorder their own project panels.

[SomeoneToIgnore]

General note: if we reset all ordering and sort the workspace alphabetically on restart, should we just do that sorting instantly when new worktrees are added?

[eth0net]

General note: if we reset all ordering and sort the workspace alphabetically on restart, should we just do that sorting instantly when new worktrees are added?

Probably, yes. Though I'd prefer to have the option as I'm trying to introduce here, to not sort them. But I agree on that behaviour when the editor is configured to sort them.

[SomeoneToIgnore]

Though I'd prefer to have the option as I'm trying to introduce here, to not sort them

Agree that it would be needed on a more general case described in the issue, where one would be able to drag worktrees around and form a different order out of them.
But even with this scenario, the only reason to have the action is to restore the original order of worktrees. Will it be popular, given that people do not open dozens of worktree items and could drag them in a different order already?

So, honestly, not sure if that's needed later even.
Currently, some issues mention complain on the order of the files in the project panel — I suspect we'll end with a dynamic toggle for various sorting there, and might just re-sort the worktree roots along the way.
Would it be a better way to have this action implemented?

Meanwhile, even with this action added, it seems odd that the new worktree items reshuffle after a restart, and since

But I agree on that behaviour when the editor is configured to sort them.

let's have it in the PR regardless of the action presence?

[eth0net]

Though I'd prefer to have the option as I'm trying to introduce here, to not sort them

Agree that it would be needed on a more general case described in the issue, where one would be able to drag worktrees around and form a different order out of them. But even with this scenario, the only reason to have the action is to restore the original order of worktrees. Will it be popular, given that people do not open dozens of worktree items and could drag them in a different order already?

So, honestly, not sure if that's needed later even. Currently, some issues mention complain on the order of the files in the project panel — I suspect we'll end with a dynamic toggle for various sorting there, and might just re-sort the worktree roots along the way. Would it be a better way to have this action implemented?

Meanwhile, even with this action added, it seems odd that the new worktree items reshuffle after a restart, and since

That's a good point, and I was considering working on the drag-and-drop reordering as a separate PR. Perhaps I should revisit it here and attempt to implement that instead.

I could also look into implementing actions for different sorting methods, though perhaps it would be better to just drop the action all together? I was aiming to provide that support in case people wanted it, but with manual reordering that wouldn't be an issue! ^^

But I agree on that behaviour when the editor is configured to sort them.

let's have it in the PR regardless of the action presence?

I'll look into adding it!

[eth0net]

Working on tests for the new logic 👾

[eth0net]

So one thing I've realised is that with the roots not being sorted at serialisation we consider two workspaces with a different path order as different, which I'm not sure I like but might not be much of an issue in reality.

I see two options:

• Sort the roots as before (either on serialisation or other time) and have a separate field in the workspace to control the sort order.
• Treat two identical workspaces with a different order of roots as a different workspace.

Thoughts?

[SomeoneToIgnore]

If I've understood the root issue right, we do not anyhow persist the roots' order between restarts?
And the real question would be how to store the new information? I would go with another column in the SQLite workspaces table, but let's recheck with more people (@\ConradIrwin maybe) when it's implemented to be on a safer side.

[eth0net]

we do not anyhow persist the roots' order between restarts

Correct for main, however I removed the sort in the serialisation leading to the persistence. Though this now treats ["one", "two"] and ["two", "one"] as different workspaces, unlike before.

And the real question would be how to store the new information?

Exactly, we could accept that different orders result in different workspaces, or add another field. I think another field might be better, even though it likely means a lot of work, it feels like a better solution.

I would go with another column in the SQLite workspaces table, but let's recheck with more people (@\ConradIrwin maybe) when it's implemented to be on a safer side.

I agree, let's see what other people think, but I think it's likely the better solution so I'll start working on it in the meantime.

[eth0net]

Also it seems my cargo test --workspace is not happy and can't connect to the database even though I started the docker compose environment... will have to figure that out at some point to get local testing working properly

[SomeoneToIgnore]

There's no such test functionality exposed in a convenient way (if any), so testing the action invocations is enough at this point.

[eth0net]

I've added tests now and I'm quite happy with it, but I'm having trouble running the zed-local script to test collaboration. So far I haven't added support for syncing order from collaboration or remote projects, but neither can I test it 👀

./script/zed-local -2
/Users/elliot.thomas/code/zed/script/zed-local:83
  throw new Error("Could not parse screen resolution");
  ^

Error: Could not parse screen resolution
    at Object.<anonymous> (/Users/elliot.thomas/code/zed/script/zed-local:83:9)
    at Module._compile (node:internal/modules/cjs/loader:1358:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1416:10)
    at Module.load (node:internal/modules/cjs/loader:1208:32)
    at Module._load (node:internal/modules/cjs/loader:1024:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:174:12)
    at node:internal/main/run_main_module:28:49

Node.js v20.13.1

[SomeoneToIgnore]

First time I see something like this, but you could try to debug that node script and see what's wrong.

I think we can omit the collab for now: not even sure if it's needed?
In the multiplayer mode, people can arrange their projects however they like and we might want to save such order locally.
And if we get that model working, single player remote project's item order will be also saved.
I'm not sure if a remote project is even supporting dropping files on it (should not, actually), so it's not very widespread now and might require more work.

[eth0net]

That's fair, the main issue I was thinking of is that users joining might see projects in alphabetical and the host would see their custom order, but I'd like to test it once I can figure that out ^^

[eth0net]

Ahhh it's because my macbook has a dual GPU 👀 the script indexes SPDisplaysDataType[0] and gets my intel GPU which isn't actively being used.

[eth0net]

Okay I managed to test it, it does indeed organise them alphabetically on the client but they can also drag and drop to reorder them so not a major issue for now I guess since it all works :)

[eth0net]

It took me way too long to figure out how to get the collab server running though as all the instructions rely on foreman and only loosely mention docker, which I was using. Might be worth revisiting those docs outside of this. I did include the zed-local fix though 🚀

[SomeoneToIgnore]

Thank you for all the adjustments — I have also planned to adjust the collab docs and add the remote project development guide, but will wait a bit then, in case you submit a PR there, to avoid the conflicts.

I have tested it and it works really nice!
Let's change the data structures a bit to avoid n^2 and we're good to merge.

[SomeoneToIgnore]

I know that there's hard to fit hundreds of worktrees inside one workspace, but I am concerned still: this looks like an n^2 asymptotic complexity here?

Could we maybe allocate a HashMap or maybe change worktree_order: Vec<WorktreeId> to that HashMap instead?

Two methods below have a similar issue, also with id's involved.

[eth0net]

I'll see what I can do! It would be nice to have it as efficient as possible, but I'm wary of how effective a HashMap might be for ordered values 🤔 it would make the worktree lookup quicker but would be more steps to shift them around if we use it to store the order in the project itself. Though using it as an intermediary may work nicely.

[eth0net]

Well, it's not exactly pretty, but it works and I believe it avoids n^2 issues! 🎉

[SomeoneToIgnore]

I have looked at the whole PR after it changes and got somewhat upset by the amount of things we start exposing with pub around the order.

Overall, it started to feel quite complex too, so I have tried to simplify it with https://github.com/zed-industries/zed/compare/kb/test?expand=1

It has one large commit that removes all extra DB and other stuff, instead trying to maintain the worktrees: Vec<WorktreeHandle> collection in a specific order:

• alphanumeric if the worktrees were added if none of those were reordered before
• to the end of the list, otherwise

And a small commit that showcases an odd state that I failed to understand now:

[crates/project/src/project.rs:7138:9] self.worktrees().map(|w| w.read(cx).abs_path()).collect::<Vec<_>>() = [
    "path_2",
    "path_1",
    "path_3",
]

[crates/workspace/src/workspace.rs:3645:57] local_paths = LocalPaths(
    [
        "path_1",
        "path_2",
        "path_3",
    ],
)

so, the code successfully re-orders the entries, but somehow fails to emit the same correct order when serializing this to the database.

If we manage to fix that — seems that we've solved most of the task with even less changes?
One caveat would be the new worktrees_reordered field not restored from the DB now: on deserialize from the DB, we can check whether the paths are sorted or not which does not seem hard.
At worst case, deserializing that as true always is fine too, given that there will be a way to re-order elements later.

What do you think of the whole approach proposed? Should we pursue that instead, looking for what's breaking the order before the serialization?

[eth0net]

I have looked at the whole PR after it changes and got somewhat upset by the amount of things we start exposing with pub around the order.

While I agree it would be nice to keep it as simple as possible and expose less, we do need to keep in mind other implications, such as workspaces with identical paths but different orders.

Overall, it started to feel quite complex too, so I have tried to simplify it with https://github.com/zed-industries/zed/compare/kb/test?expand=1

It has one large commit that removes all extra DB and other stuff, instead trying to maintain the worktrees: Vec<WorktreeHandle> collection in a specific order:

• alphanumeric if the worktrees were added if none of those were reordered before
• to the end of the list, otherwise

I love the simpler approach, it's like a more fleshed out version of my first implementation, though it does not solve the issue I had of the workspaces with identical paths in a different order. Is this something we want to solve, or would we accept these near identical workspaces existing?

One reason I moved the order into a separate field was to allow the sorting in the serialised data to avoid the duplication of workspaces with the same paths, meaning that if you create a new workspace that matches the paths of an old one you'll clear that out and use the new order of paths. Either from launching the CLI with the paths in a different order, or by adding/removing folders in a workspace making it identical to another one.

And a small commit that showcases an odd state that I failed to understand now:

[crates/project/src/project.rs:7138:9] self.worktrees().map(|w| w.read(cx).abs_path()).collect::<Vec<_>>() = [
    "path_2",
    "path_1",
    "path_3",
]

[crates/workspace/src/workspace.rs:3645:57] local_paths = LocalPaths(
    [
        "path_1",
        "path_2",
        "path_3",
    ],
)

so, the code successfully re-orders the entries, but somehow fails to emit the same correct order when serializing this to the database.

If we manage to fix that — seems that we've solved most of the task with even less changes? One caveat would be the new worktrees_reordered field not restored from the DB now: on deserialize from the DB, we can check whether the paths are sorted or not which does not seem hard. At worst case, deserializing that as true always is fine too, given that there will be a way to re-order elements later.

What do you think of the whole approach proposed? Should we pursue that instead, looking for what's breaking the order before the serialization?

I'll take a deeper look at it, since it would be nicer to have this simpler, assuming we accept we can get multiple serialised workspaces with the same paths in different orders :)

[SomeoneToIgnore]

At least what we can do is return the DB layer with the extra sorting field back — it seems that we can still keep the in-memory data as just a Vec based on the order taken from the DB, and get the deduplication back?

[eth0net]

So I've been going through merging your branch into mine and going crazy over the serialisation, doesn't help I'm sick right now 💀 But it finally works now and I can finally relax 🤣

Please lmk if you have any thoughts on the latest changes, I'd love to get this finished but want it done right of course 🚀

[SomeoneToIgnore]

Oh, get well, please — no rush here.

[SomeoneToIgnore]

Sorry, takes me a while to understand the very last question: why cannot we remove LocalPathsOrder and stop sorting LocalPaths instead, saving the right order into the database.

I had to deal with my other tasks first, but now I'm ready to explore and try to fix that on top.
Otherwise, the PR is ready and fine as it looks, so let me take care of a few things during this or next week, and get it merged.

[SomeoneToIgnore]

Finally, took me an embarrassingly long time to connect the dots and understand the persistence deal more — that corner needs more refactorings tbh.

Had pushed a few style fixes, but overall looks nice, thank you for bearing with me for a while.

[eth0net]

Thanks for approving, sorry it was confusing!

I've also realised the recent projects aren't sorted so I'll put another PR in for that shortly, now that the project panel and persistence is out of the way.