`addWorkspaceFolder` / `removeWorkspaceFolder` are not effective API

[alexdima]

Testing #37133

fyi @jrieken .

---

• observe that a WorkspaceFolder has three fields: uri, name and index.
• observe the reading API: export let workspaceFolders: WorkspaceFolder[] | undefined;

---

IMHO, the editing API seems out of place:

• removeWorkspaceFolder takes as argument a single WorkspaceFolder; why a single one ?
• addWorkspaceFolder also takes in a single WorkspaceFolder; why ? It also ignores the index property; how to add something at the beginning of the array ?

e.g. of difficulty:

• how to implement replace folder at i with another folder. i.e. workspaceFolders[i] = newWorkspaceFolder:
  • remove N folders where folder.index >= i, i.e. call removeWorkspaceFolder N times
  • add N folders back again i.e. call addWorkspaceFolder N times

IMHO a much simpler API would be a setWorkspaceFolders(newWorkspaceFolders: WorkspaceFolder[])

[bpasero]

@egamma one idea is to put our new API to proposed API first to better understand the requirements from other extensions. Would that be ok?

[bpasero]

@jrieken @alexandrudima without having this coded up yet, what about this:

export function setWorkspaceFolders(folders: (WorkspaceFolder | { uri: Uri; name?: string; })[]): Thenable<boolean>;

Where a folder can either be an existing WorkspaceFolder or a new one.

It avoids having to introduce a separate createWorkspaceFolder method with all the associated issues and it is still long running as before. The downside I see is that we need to tackle the corner cases, e.g. someone could pass in the URI of an existing WorkspaceFolder but with a different name so we should update the name of that workspace folder I guess.

Another downside is that the simple case of adding or removing a single WorkspaceFolder is a lot more work for an extension author...

One interesting (new) challenge with this approach is that it would then be possible to go from 1 single-folder workspace to a multi-root workspace where the first folder is different, which is a transition that we do not support today without window reload because some extensions could be disabled in the single-folder case and no longer for the new first root folder. In that case we would still need to do a window reload.

[jrieken]

If full power is needed we should go for something splice'ish. Maybe updateWorkspaceFolders(index, deleteCount, add:{uri, name}[])

[bpasero]

@jrieken yeah I like that for the reason of not having the ugly or-type between WorkspaceFolder and {uri, name}