Theia: ctrl+f defaults to the browser search

[akosyakov]

Description

Reproduction Steps

• open SIW view
• press ctrl+f to retrigger search
• native browser search is shown
• search is not retriggered

OS and Theia version:

Diagnostics:

[kittaakos]

• native browser search is shown

FYI: previously we throw an exception: https://github.com/eclipse-theia/theia/issues/7474

[kittaakos]

Related conversation: https://github.com/eclipse-theia/theia/pull/7481#issuecomment-609692711

[kittaakos]

Linked issue: https://github.com/eclipse-theia/theia/issues/7346

[akosyakov]

@kittaakos you are right about #7346

I'm still not sure about falling back to the native search.

[kittaakos]

We have to re-introduce a NOOP core.find then.

[akosyakov]

What about core.replace? We used to prevent it as well.

[kittaakos]

What about core.replace?

Yes. Obviously, it belongs here.

We used to prevent it as well.

"prevent" 😄 yes.

---

I think Select All, Find, and Replace should be generalized all over the application; the active widget should handle it if it can, otherwise we have to fall-back to the default NOOP. Most likely that's a bigger task.

[akosyakov]

I think Select All, Find, and Replace should be generalized all over the application; the active widget should handle it if it can, otherwise we have to fall-back to the default NOOP

Strangely the same issue present in VS Code, try with the terminal for instance:

• if you use a find keybinding then it shows find widget in the terminal
• but if you use Edit -> Find it does nothing.

I don't think that's nice though.

[akosyakov]

Actually before it was working like in VS Code: if you have an opened editor which is not focused, Find and Replace will still applied to this editor. It does not work only in the case if a widget, e.g. terminal, has own command. I think we should align again with it.

[kittaakos]

I think we should align again with it.

I don't like the idea because then we have to handle monaco commands differently, one command (find, replace) would require an editor, it does not necessarily have to have the focus. Other commands, such as select all, undo would require a focused editor.

But please go ahead if you think it is better.

[akosyakov]

I don't like the idea because then we have to handle monaco commands differently, one command (find, replace) would require an editor, it does not necessarily have to have the focus. Other commands, such as select all, undo would require a focused editor.

I afraid that it will be surprising to users, since it used to work like that in Theia for other commands except undo, redo, select all and work like that in VS Code.

[kittaakos]

since it used to work like that in Theia

But it threw an error.

[lmcbout]

Side effect of "Select All" for the "Problem" view. When using the cmd+a to selct all, it also select all line ranges in the open editor.

[akosyakov]

But it threw an error.

yes, this thing we can fix by no-op core find/replace handlers which are always enabled, since we have reverse handlers Monaco and terminal extensions will win

[akosyakov]

@lmcbout please file a separate issue. The problems view should have Select All command similar to the debug console.

[akosyakov]

@kittaakos I'm trying to figure out where do we check when closures and preconditions of Monaco commands/actions with the new approach. We used to do it via MonacEditor.isActionSupported. Do we completely ignore them now?

[kittaakos]

Do we completely ignore them now

For the built-in Monaco commands, yes.

We use these for the enablement checks:

• https://github.com/eclipse-theia/theia/blob/master/packages/monaco/src/browser/monaco-command.ts#L168
• https://github.com/eclipse-theia/theia/blob/master/packages/monaco/src/browser/monaco-command.ts#L177

[akosyakov]

For the built-in Monaco commands, yes.

Why is it better? Don't we ignore internal Monaco logic, i.e. we call a command when Monaco does not expect it. Should not we evaluate them instead in isEnabled callback?

[kittaakos]

Why is it better?

Otherwise, you never execute anything from VS Code's coreCommands.

Update: fixed the module name, added link.

[akosyakov]

Otherwise, you never execute anything from VS Code's coreCommand.

You mean for undo, redo, select all? I thought Monaco does the same what our core command doing for such cases, i.e. document.execCommand? For other I don't think they are enabled always, it should depend on when closure as before.

[akosyakov]

I wonder also why do we need ElectronUndoHandler and other handlers? If it is based on Chromium and calls at the end document.execCommand. VS Code does not seem to do it.