Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keys don't work on find wedget #137

Closed
makotoshimazu opened this issue Aug 6, 2019 · 10 comments · Fixed by #665
Closed

Keys don't work on find wedget #137

makotoshimazu opened this issue Aug 6, 2019 · 10 comments · Fixed by #665

Comments

@makotoshimazu
Copy link

Cursor operations like Ctrl+F/B/A/E and delete like Ctrl+H don't work on Find widget, and moreover, they close the widget.
Is it possible to use them on the widget?

@ganaware
Copy link
Contributor

ganaware commented Aug 6, 2019

There is no way to customize keybindings of input fields:

(FYI: #55)

@makotoshimazu
Copy link
Author

Thanks for the pointers!
Now I can understand that this is blocked by issues on vscode. I'm going to watch them.

@makotoshimazu
Copy link
Author

makotoshimazu commented Aug 6, 2019

And I also found that 7da035c is a patch to close the widget.
Now I can understand what's the problem I'm facing with - it closes the widget even when it's replace (not find). I don't have a way to move cursor except for clicking the small input area.

@ganaware Do you have any thoughts?
I personally prefer not closing the widget because currently we can close the find widget by C-g, but we don't have a way to move the cursor during replacing words. (= probably it means reverting the patch)

@ganaware
Copy link
Contributor

ganaware commented Aug 6, 2019

I understand that what is the problem.

The patch was introduced by #71 to emulate original i-search behavior, and I think it does good job.

But unwillingly it also affects the replace widget, because there is no difference between the replace widget and the find widgets.

We have no way to know which widget is visible.
For 'when' clause contexts, VSCode offers "findWidgetVisible" that cannot distinguish them.
( https://code.visualstudio.com/docs/getstarted/keybindings )

So, which is better ?

  1. more emacs-like i-search, but poor replace-widget experience. (current)
  2. less emacs-like i-search, but cursor movement works.
  3. or ...?

@makotoshimazu
Copy link
Author

Given that currently we don't have any keyboard shortcuts to move the cursor, I prefer 2.
I like more emacs-like i-search, but personally bad experience on replace widget seems more problematic than C-g to stop i-search.
What do you think?

@ganaware
Copy link
Contributor

ganaware commented Aug 9, 2019

@tuttieee any ideas?

@whitphx
Copy link
Owner

whitphx commented Mar 16, 2020

Sorry, no idea. What @ganaware said is all.
Please override the keybindings by your user-level setting like below as a workaround.

{
    "key": "ctrl+f",
    "command": "-emacs-mcx.executeCommands",
    "when": "editorFocus && findWidgetVisible"
},
{
    "key": "ctrl+b",
    "command": "-emacs-mcx.executeCommands",
    "when": "editorFocus && findWidgetVisible"
},
{
    "key": "ctrl+p",
    "command": "-emacs-mcx.executeCommands",
    "when": "editorFocus && findWidgetVisible"
},
{
    "key": "ctrl+n",
    "command": "-emacs-mcx.executeCommands",
    "when": "editorFocus && findWidgetVisible"
},
{
    "key": "ctrl+a",
    "command": "-emacs-mcx.executeCommands",
    "when": "editorFocus && findWidgetVisible"
},
{
    "key": "ctrl+e",
    "command": "-emacs-mcx.executeCommands",
    "when": "editorFocus && findWidgetVisible"
}

@makotoshimazu
Copy link
Author

Ah sorry I missed your comments last month.
I have those in my local keybindings.json and I no longer have any problems, but I'd suggest that adding those settings into the default keybindings so that the behavior would be easier to understand (at least for me).
What do you think?

@whitphx whitphx reopened this Apr 28, 2020
@whitphx
Copy link
Owner

whitphx commented Apr 28, 2020

The workaround I provided was to disable closing the find widget - but the disabled behavior here was an intended behavior introduced in #71 as @ganaware said.
So, the workaround here cannot be installed into the default keybindings - it conflicts with the "correct" behavior.

However, there are some options and things to do.

  • To write document about this.
  • To add the "disabled" version keybindings as an option switchable by config.

Thank you for your suggestion. I will do these things.

@whitphx
Copy link
Owner

whitphx commented Feb 4, 2021

Now you can use emacs-mcx.cursorMoveOnFindWidget option with v0.26.0 or later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants