Vscode: Diff tabs opened from the `Source Control` panel no longer in preview mode

[cateyes99]

• VSCode Version: 1.12.1
• OS Version: OS X EI Capitan 10.11.3

Steps to Reproduce:

1. Open some project from GitHub.
2. Make some changes to a file A.cs, and save it.

3.a Stay on the editor of A.cs, and click the button of Open Changes on the top right corner above the editor.
4.a A diff tab opened to show the difference between the changed A.cs and the version in GitHub. Before version 1.12, this diff tab will be open in the preview mode (the tab title font is in __italic__), now the new diff tab is just a normal tab.

3.b Open the Source Control panel, click A.cs.
4.b A diff tab opened to show the difference between the changed A.cs and the version in GitHub. Before version 1.12, this diff tab will be open in the preview mode (the tab title font is in __italic__), now the new diff tab is just a normal tab.

1. If I have 50 files changed, and iterate the changes in the 50 files from Source Control panel, it will open 50 diff tabs for me! This new behaviour isn't very desired. The old behaviour in the previous versions is much more convenient and consistent with opening files from the Explorer panel.

[eamodio]

FYI, this changed here: https://github.com/Microsoft/vscode/commit/9f01642853d797978b62714b31edc8d8f1acd18b#diff-5d0a5d79d21dddf1284ed8ca92298d24R384

[joaomoreno]

cc @jrieken already 4 users mentioned this

[jrieken]

This was to align the behaviour of our various open-calls. @joaomoreno you do have an option tho, simply set preview to true and everything is back to normal

[bpasero]

I think the extension should not decide to set the preview flag unless it really wants to make sure that something does not open as preview (e.g. a use case I can see is to open a playground to the side of the current editor that should not go away too easily).

The workbench should deal with the preview flag, otherwise settings like "workbench.editor.enablePreview": false are ignored, which is bad.

[bdinesh]

@jrieken I have set "workbench.editor.enablePreview": true in user settings but no luck. Is there any other setting to be set explicitly?

[jrieken]

This isn't about the setting @bdinesh. Foremost extensions can express their intent when showing a text document (preview or not). Unsure how the setting plays into that, but I have a feeling that the editor service should use it and simply overrule the API choice. Correct @bpasero or are there editors that must open as preview despite have that setting which blocks putting this into the service?

[bpasero]

@jrieken @joaomoreno today we have two settings that play a role to decide if an editor should open as preview or not:

• workbench.editor.enablePreview (default: true)
• workbench.editor.enablePreviewFromQuickOpen (default: true)

If these settings are configured by default, in most cases we open files in preview (e.g. click in explorer, open from quick open).

However, there are still some user interactions that can lead to not using preview after all independent from the setting:

• double click in the explorer
• opening a dirty file
• moving a file from the explorer

If the workbench.editor.enablePreview setting is configured to be false you will never see an editor as preview no matter how the user configured it from the API.

If the workbench.editor.enablePreviewFromQuickOpen setting is configured to be false, it only impacts editors that open from quick open but nothing else.

My understanding is that we added new API for TextDocumentShowOptions to control if a document opens as preview or not. However, we are not having this option from our vscode.diff command, and as such there we now hardcode pinned: true. Wouldn't it best we do not set pinned at all for any editor we open from extensions and let the extension decide this? For example, we could add support for the vscode.diff command to define similar TextDocumentShowOptions and then the GIT extension can actually decide to open pinned or not based on user interaction (e.g. double click vs. single click).

[jrieken]

However, we are not having this option from our vscode.diff command, and as such there we now hardcode pinned: true.

No, that isn't correct. We have added that option when adding the option for showing a text document. The change that happened is that both are now aligned wrt the default, which is 'pinned'. The diff-command used to be different here and because it is less official got aligned with the show-function. To get to old behaviour you need to pass {preview: true} when calling the diff-command.

[eamodio]

Before these changes extensions could do the following (at least as I've observed them):

commands.executeCommand('vscode.open' ...) - would always open the document in preview (not pinned) and steal focus
commands.executeCommand('vscode.diff' ...) - would always open the diff in preview (not pinned) and steal focus
window.showTextDocument(...) - would always open the document not in preview (pinned) and you could control the focus

After these changes:
commands.executeCommand('vscode.open' ...) - unchanged since it is deprecated
commands.executeCommand('vscode.diff' ...) - opens the diff in not in preview (pinned) by default (to align with the default of showTextDocument) and takes the new TextDocumentShowOptions to control preview, focus, column, etc
window.showTextDocument(...) - opens the document in not in preview (pinned) by default and takes the new TextDocumentShowOptions to control preview, focus, column, etc

So now the default behavior of both is aligned and both provide the same options.

Hope that helps and makes sense.

[bpasero]

@jrieken @eamodio to me the only right way of fixing this is to not set pinned option in showTextDocument and vscode.diff unless the caller is explicitly setting this option. Otherwise there is no way to get the default behaviour we have everywhere else where the settings of the user are being looked at.

Otherwise, the only way of fixing it for the git extension is to read the user settings (specifically workbench.editor.enablePreview) and then make the decision of setting pinned or not based on that. But basically any extension that uses the vscode.diff command or the showTextDocument would have to do that if they wanted the default behaviour. I would change it and let those extensions adopt the pinned option that really want the editor to open pinned.

[eamodio]

@bpasero You are proposing to change both showTextDocument & vscode.diff to default the preview parameter (if undefined) to the value of workbench.editor.enablePreview right?

If so, I agree that sounds like the best way, but it will likely come with some short-term pain -- as the preview default of showTextDocument was always true and vscode.diff was always false. IMO at least it sounds like it might be worth it to actually honor the workbench.editor.enablePreview by default, but still allow extensions to override that depending on their needs.

[bpasero]

@eamodio yes, the only time the workbench explicitly sets the pinned option is when there is a specific user action that makes it worth opening as pinned, for example a double click on a file.

I think if we communicate this change properly, extensions can adopt it, especially since we provide this option for both the command and the method.

[glen-84]

Isn't this a regression? I was hoping that it might be fixed in a patch release.

It's quite annoying. (=

[glen-84]

@joaomoreno Will you please consider adding this to the milestone for May?

[joaomoreno]

Likely not happening for May, sorry man.

[joaomoreno]

Turns out I was wrong.

[eamodio]

@joaomoreno while that commit fixed it for the SCM/Git case, it didn't address @bpasero's concerns above -- so should this be re-opened or should that be moved into a separate issue?