feature: allow configuring default browser

[SimonSiefke]

Helps with #96132.

This adds a new setting workbench.defaultBrowser for configuring which browser to open:

default-browser.mp4

The value for workbench.defaultBrowser is a string and can either be an application name like google-chrome or an absolute path like /usr/bin/google-chrome.

[bpasero]

Nice approach 👏

[SimonSiefke]

Thanks for the feedback! Changed the setting name to workbench.browser and the setting scope to application.

Also more variants are support now, for example:

• chrome, google-chrome, /usr/bin/chrome
• edge, microsoft-edge, /usr/bin/microsoft-edge
• firefox, /usr/bin/firefox

In case of error, it now falls back to shell.openExternal.

[bpasero]

@SimonSiefke thanks, did some follow up cleanup, please check. I renamed the setting to workbench.externalBrowser.

Its sad we cannot use the latest open module which switched to ESM (yet).

I wonder if people in the future want more options, such as controlling arguments to the browser. In that case maybe the setting should already make room for other settings? Like workbench.externalBrowser.app?

[bpasero]

Pushed some more changes to validate invalid inputs and missing paths, I think this is good to go as it is.

[gjsjohnmurray]

I see you decided on application scope for the setting, #96132 (comment) gave a use case for workspace-specific settings. I think you considered allowing this for workspaces but only respecting it if the workspace has been trusted by the user. What was the reason for not doing that?

[bpasero]

If we want to make this a workspace setting, the change will become larger: in nativeHostMainService we only have access to the application scoped settings, so the setting value would have to be passed down from the workbench window that has access to the workspace configuration.

I am fine making this a workspace supported setting if its untrusted.

[SimonSiefke]

I wonder if people in the future want more options, such as controlling arguments to the browser. In that case maybe the setting should already make room for other settings? Like workbench.externalBrowser.app?

I like that idea!

The workspace setting also sounds good to me! It seems it might be a larger change, so maybe it could be a separate pull request.

[bpasero]

Ideally we do all the changes in one PR when we think its done. Maybe @gjsjohnmurray wants to join forces?

[SimonSiefke]

Alright then, I can try to make the workspace setting changes. :)

Is there any standard way of handling multiple workspaces? E.g. when two workspaces are open (a and b) and the user clicks a link from the terminal, would it use the settings from a, b or the global application settings?

[bpasero]

Alright then, I can try to make the workspace setting changes. :)

Is there any standard way of handling multiple workspaces? E.g. when two workspaces are open (a and b) and the user clicks a link from the terminal, would it use the settings from a, b or the global application settings?

The workspace settings are bound to the window you are in where you click from, so we do typically not support workspace settings in another folder, even if you are in that context.

I would suggest to read the setting here where we open links from a window:

vscode/src/vs/workbench/electron-sandbox/window.ts

Line 897 in f8b1bef

const success = await this.nativeHostService.openExternal(href);

and send it over + provide a fallback for the other usages where we are not in a workspace context. in these cases I think its fine to do the config reading as we do today only supporting app scope.

[bpasero]

I had some slack time and went ahead implementing what we discussed.

[bpasero]

Waiting for #220273 for the dependencies.

[SimonSiefke]

Awesome, thanks! :)