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.

[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.

[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]

At least for me, both mac and Linux worked with docker compose up in the root of the project, and cargo nextest run --workspace --no-fail-fast (as the CI runs it) in a separate tab.

[eth0net]

Interesting, I'll try that, the docs say cargo test --workspace ^^

[eth0net]

Apologies, thought I'd made it draft again! ^^

I'm working on the DB changes to persist it, but need to decide on the best way to do so.

[eth0net]

Trying to figure out where to store it, but there's a lot going on in here 😅

If I'm reading this all correctly, this will only need to be stored in the DB for local workspaces, so I could include it in the LocalPaths struct potentially, do you think this sounds like a good approach? Alternatively, I could store it in the SerializedWorkspaceLocation::Local struct, which may be better.

[eth0net]

Never mind, going to add it to the SerializedWorkspaceLocation::Local struct ^^ just working on it and all the required methods for serialisation.

[eth0net]

I have made a start on the saving and loading though it does not yet work, but my brain is too fried tonight to see why so I shall continue after a break 👀

[eth0net]

I have made a start on the saving and loading though it does not yet work, but my brain is too fried tonight to see why so I shall continue after a break 👀

I stand corrected, it did work, it just didn't refresh the project panel after loading the order 👀

[mikayla-maki]

Awesome! When you're feeling like it's ready for us to check in, hit that 'ready for review' button below :)

[eth0net]

I need to work on some more tests for it all of course but the feature is now in place ^^ I'll mark it ready once I've got those tests in place!

[eth0net]

Any suggestions on testing the drag and drop events? Not seeing how they're tested for the existing move_entry code.

[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 🚀

[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.

[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.