Always open editor even in error cases

[kieferrm]

1. open a not empty repo folder
2. open a file in an editor (not preview mode)
3. create a new untitled file
4. add a couple of new lines
5. save it as an ipynb file
6. close the saved file
7. right click the ipynb file in the explorer and select "Open with..." > Jupyter
   -> error notification
   -> the explorer immediately selects the file from step 2. In a small workspace this is not an issue but in a larger on it is. The ipynb file might scroll out of the view port.

For me, the error notification here is the wrong tool. This should be a modal dialog.

Recording of step 7 and the consequences:

[bpasero]

Any customer of IEditorService.openEditor is able to pass in context: EditorOpenContext.USER as part of the options for opening to signal that the action to open an editor was done as a user action. If you do so, the notification will instead be a modal dialog:

As such, moving to respective component owners whether this option should apply or not.

It is currently used only in a very few selected places because it is quite a heavy interrupt of the user flow, but may make sense in certain contexts.

[bpasero]

To clarify: I assigned Jackson for whether any normal click on an entry in the explorer should trigger this and Logan for the "Open with.." flow.

[lramos15]

I went ahead and added the modal flow for Open With... but this applies to Reopen with... too. I believe @rebornix plans to work on better error handling for Jupyter so it's not an issue but this seems like a bad solution for failed editors in general. @bpasero Is there a reason we can't render a lightweight "error editor" like we do when we cannot open a file on disk. Or maybe even just gracefully fallback to the text editor? The reason for the workbench hop is because it opens and closes so quickly that the next editor in the group gets workbench focus.

[JacksonKearl]

Makes good sense to me that errors encountered when opening from the explorer should show a blocking dialog, given the strong user intention to open the file.

[bpasero]

@bpasero Is there a reason we can't render a lightweight "error editor" like we do when we cannot open a file on disk

The error editor only appears for editors that have been opened before and are thus restored in order to preserve the users view state:

vscode/src/vs/workbench/browser/parts/editor/editorPanes.ts

Line 120 in 4a130c4

if (!context.newInGroup) {

I am not entirely convinced we should change this condition to always open an error editor, I would actually prefer the modal dialog in this case.

Nevertheless, the more I think about it the more we are getting slightly inconsistent:

• sometimes we show a modal dialog on error
• sometimes we show a notification on error
• sometimes we show an error editor

The original reason for having EditorOpenContext.USER was for this case where the user would retry to open an editor from an error editor where it felt somewhat bad to show a notification instead of a modal dialog given the explicit intent:

@misolori @daviddossett maybe you have thoughts. #130971 is slightly related.

[lramos15]

I am not entirely convinced we should change this condition to always open an error editor, I would actually prefer the modal dialog in this case.

I'm fine with the modal but it doesn't fix the problem of your explorer position getting messed up as Kai demonstrates in the recording above.

[bpasero]

I think the explorer just automatically reveals the editor when it changes and in this case the editor changes briefly to the faulty one and then back.

[rebornix]

For notebooks, especially a Jupyter notebook, common errors during file opening is corrupted JSON. Showing a modal dialog in this case is better than a notification but users still have no way out until they learn about "Reopen editor with ..." from the context menu to open it in text editor and fix the corruption.

Considering this, a "Retry" on the error editor doesn't help too much as it retries with the same editor type and mostly you get the same error message. Should we add "Try another editor" in the warning text or modal dialog so users can find a way out to get the data fixed?

[daviddossett]

Should we add "Try another editor" in the warning text or modal dialog so users can find a way out to get the data fixed?

I like this idea. Could also reuse the "Reopen editor with..." text as the button text to be consistent with the context menu option?

[lramos15]

Considering this, a "Retry" on the error editor doesn't help too much as it retries with the same editor type and mostly you get the same error message. Should we add "Try another editor" in the warning text or modal dialog so users can find a way out to get the data fixed?

The downside is most editor errors are issues regardless of error type. This is just a special case it seems

[rebornix]

The downside is most editor errors are issues regardless of error type. This is just a special case it seems

@lramos15 we can offer different warning text and buttons based on scenarios, in notebook opening in a text editor is a good way out (agree it's a special case) but we don't want to build our own modal dialog or error editor. Instead this should be the same error dialog/editor users see, but with different context.

For example, we can try catch in notebook land and return

{
  error: Error,
  suggestion: 'ReopenWithAnother'
}

[bpasero]

You can actually control the buttons shown in the error dialog when opening, we do that to offer a "Create File" action in case you open a missing file and text editor does this by creating a IErrorWithActions:

vscode/src/vs/workbench/contrib/files/browser/editors/textFileEditor.ts

Lines 193 to 209 in ddf1226

	const fileNotFoundError: FileOperationError & IErrorWithActions = new FileOperationError(localize('fileNotFoundError', "File not found"), FileOperationResult.FILE_NOT_FOUND);
	fileNotFoundError.actions = [
	toAction({
	id: 'workbench.files.action.createMissingFile', label: localize('createFile', "Create File"), run: async () => {
	await this.textFileService.create([{ resource: input.preferredResource }]);
	return this.editorService.openEditor({
	resource: input.preferredResource,
	options: {
	pinned: true // new file gets pinned by default
	}
	});
	}
	})
	];
	throw fileNotFoundError;

Maybe worth exploring this for notebooks?

[rebornix]

@bpasero thanks Ben, it's exactly what I was looking for. I adopted the IErrorWithActions in notebook and then it's up to the editor service to decide how it handles the error and actions (either in modal or error editor).

[dbaeumer]

Moving to April.

[miguelsolorio]

We can always try out the editor error for now, since we already use a similar pattern, and then if it's not enough we can move to trying out the modal dialog.

[bpasero]

Will see if I can get to it, otherwise Backlog or out of scope.

[bpasero]

Exploring this a bit, the change is pretty straight forward: instead of showing the error pane only when restoring an editor, also show it in all other cases, such as opening an editor.

The example below demonstrates the flow, assuming there is a link to a file that does not exist. This would normally only bring up a modal dialog asking to create the file, but would now open the error pane.

Unclear to me though what to do with the error toast or even error dialog:

• should we only show error message and actions in the error editor pane
• should we still support modal dialogs but avoid notifications?

[lramos15]

• should we only show error message and actions in the error editor pane

I vote for this one. It will prevent the error from blocking the user and then I think in the future when the error pane is cleaned up a bit more with @misolori's mockups it will be even easier for the user to take action within the pane. For now I would just expect Create File or Try again to be links within the pane.

[bpasero]

I actually find the flow for "Create File" specifically quite nice because I heard from users that work like this:

• create a HTML file
• add links to CSS, JS etc. even though the files don't exist
• follow the links
• just hit enter to create the missing files

If we stop showing the dialog, this flow becomes harder.

My vote would be: show modal dialog in selected cases where we have meaningful actions but never show the notification toast error that only conveys the error message but no action other than retry.

[bpasero]

This landed and will be tweaked.

[sbatten]

@bpasero in the demo case that you show (creating a file from a link to a non-existent file), why do we show the editor area error message if they cancel? We are now showing a modal dialog and they have been told the error and opted to cancel.

[bpasero]

I think we still do the right thing showing the placeholder, because the user cancelled the dialog that asks for creating the file. The dialog is not asking about the error placeholder.

[sbatten]

I'm just not sure what I'm ever going to do with the placeholder in this case. After clicking cancel, I've already chosen not to take the only action that the placeholder provides.

[bpasero]

The original motivation for having an editor visible is coming from Kai's original steps: when you carefully select a file in the explorer in some deep hierarchy and an editor opens but then closes due to error, we by default select the previous file in the explorer so you lost your selection. The error editor placeholder tries to help for that case mainly, preserve UI state and stability.