zed icon indicating copy to clipboard operation
zed copied to clipboard

Toggling on vim mode does not work

Open aeruhxi opened this issue 1 year ago • 10 comments

Check for existing issues

  • [X] Completed

Describe the bug / provide steps to reproduce it

Toggling on vim mode does not work without a restart. Toggling off vim mode does work however, for what it's worth.

If a restart is expected, I believe we should let user know about it.

Environment

Zed: v1.0.0 (Zed Dev ddaaaee9731179e2c5e29289e3ec7982ae8a55ee) OS: Linux 1.0.0 Memory: 31.3 GiB Architecture: x86_64

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

No response

aeruhxi avatar May 09 '24 07:05 aeruhxi

Huh - this is strange, I believe Vim mode has always worked for me right after the setting change, without needing a restart. Wonder what the difference is. Can you include your settings file? I dont have any hunches that there is anything in there that is causing that, just curious to see it.

JosephTLyons avatar May 09 '24 18:05 JosephTLyons

I made some changes (somewhat recently) to how vim mode activates, but the intent is that it should work without a restart.

I would also love to know a bit more detail here:

  • How are you saving your settings file?
  • Does it work if you switch to a different file?

ConradIrwin avatar May 10 '24 03:05 ConradIrwin

@ConradIrwin

I am using ctrl+shift+P to toggle vim mode, and the settings file is written as expected.

No it does not work if I switch to a different file.

FWIW, I am using this in Linux.

image The print does not execute when the value is true. It does for false.

aeruhxi avatar May 10 '24 03:05 aeruhxi

Great, thanks for the clarification – I'd missed you mentioned Linux in the original report.

(Possible duplicate of #11090)

ConradIrwin avatar May 10 '24 04:05 ConradIrwin

@ConradIrwin Oh, sorry I did not find the issue before I created this issue.

Anyway, the things I've found that are worth mentioning:

  • Turning on vim mode by manually changing in settings file and saving and changing the tab does turn on the vim mode. However, if you go to the settings file with vim mode turned on and save the settings, it disables the vim mode again.
  • Turning on vim mode by Ctrl-Shift-P and selecting toggle vim mode however does not turn it on even after changing the tab.
  • Once you toggle the vim mode with Ctrl-Shift-P action, changing the settings file and changing the tab does not turn on again. Once you've used Ctrl-Shift-P action, it's permanently broken until the restart.

This is not that big of an issue, I think, and is probably a low hanging fruit to fix. If you could point me to how observe_global works and why its callback is potentially not being called when VimModeSetting is true, I'd like to contribute to fixing it.

aeruhxi avatar May 10 '24 06:05 aeruhxi

What is supposed to happen is:

  • Updating the settings writes the new file to disk
  • That file is being watched by https://github.com/zed-industries/zed/blob/325709ffcd2b305a34ca497eec2d12ffd05be4d9/crates/zed/src/main.rs#L294
  • The watcher should cause the SettingsStore to update (The cx.update_global here: https://github.com/zed-industries/zed/blob/325709ffcd2b305a34ca497eec2d12ffd05be4d9/crates/settings/src/settings_file.rs#L80)
  • The update should push an effect to notify global observers (https://github.com/zed-industries/zed/blob/325709ffcd2b305a34ca497eec2d12ffd05be4d9/crates/gpui/src/app.rs#L968)

If I had to guess, the weak link in the chain is the file watcher, but would be good to confirm (as it sounds like it does work if you manually save the settings).

ConradIrwin avatar May 10 '24 19:05 ConradIrwin

I find the root of the problem and will try fixed.

CharlesChen0823 avatar May 12 '24 09:05 CharlesChen0823

I find the root of the problem and will try fixed.

@CharlesChen0823 Do you mean you have already found it or you are going to find it? Let me know how it goes. :)

aeruhxi avatar May 13 '24 04:05 aeruhxi

  1. When using ctrl-shift-p to toggle vim mode, current fs::atomic_write using temparory file replace old file, fs::watch will emit Modify and Delete event, when triggle Delete event, notify implementation will not watch this file any times(this is notify default behavior). So you cannot modify vim mode anymore, only restart can work.
  2. When manual modify settings.json, only when click other pane can active vim mode. currently I don't know what happened, guess might lose foucsed?
  3. In first scene, using notify::PollWatcher replaced, then ctrl-shift-p toggle vim mode or manual modify settings.json will have same behaviour. Will all need click other pane to active vim mode.

CharlesChen0823 avatar May 13 '24 08:05 CharlesChen0823

update, because current linux platform not implemented active_window, will cause second scene problem.

CharlesChen0823 avatar May 13 '24 12:05 CharlesChen0823