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

Fix(VIM-2933): Reloading/sourcing .ideavimrc does not initialize new plugins #736

Merged
merged 1 commit into from Nov 3, 2023

Conversation

chylex
Copy link
Contributor

@chylex chylex commented Oct 28, 2023

Found two issues related to VIM-2933:

  1. Clicking the Reload button did not call VimExtensionRegistrar.enableDelayedExtensions(), so new extensions were not getting initialized.

    At this point, there are 3 places in the code that follow the pattern of setting the vimscriptExecutor.executingVimscript flag, executing a script, and then initializing new extensions. On the other hand, the :source command does not appear to do this at all, it just executes the script and initializes extensions immediately.

    I don't know why extensions have to be delayed, but it seems like the inconsistency and duplication of this behavior should be refactored. If this behavior exists at all, isn't it always necessary? - In which case the delayed initialization + setting of the flag should perhaps be done directly in Executor.execute, rather than having to remember to do it manually anywhere scripts are being executed.

  2. It was possible to edit .ideavimrc and run :source ~/.ideavimrc before the file was actually saved. The Reload button ensures the file is saved first, but :source does not.

    This PR ensures that, before executing any script file, if the script file is currently opened in an editor and dirty, it will be saved first. This also means the Reload button itself doesn't need to save the file anymore.

    As I said earlier, :source does (eagerly) initialize new extensions, so this is a slightly different issue. I'm guessing the author of VIM-2933 edited the file, tried sourcing it, and it did not initialize new extensions just because IntelliJ hadn't saved the file yet, so they assumed the initialization was broken entirely; that's what happened to me while I was trying to reproduce the issue.

    Since this fix affects all script file executions, let me know if you'd like me to separate it into 2 commits and/or PRs.

@AlexPl292 AlexPl292 merged commit 4191607 into JetBrains:master Nov 3, 2023
4 checks passed
AlexPl292 added a commit that referenced this pull request Nov 3, 2023
…command

This includes updating the "ReloadIdeaVimRc" button and setting the correct mapping owner

Previously, the `source` command loaded ~/.ideavimrc file as a regular file, thus several features didn't work properly.
This refactoring was caused by this PR: #736
@AlexPl292
Copy link
Member

Hi there! Thank you for this update.
Actually, after reviewing your change, I decided to revamp this area a bit. This started to be a bit outdated after all the changes in the plugin.
Also, I added documentation to explain why we need to initialize plugins after the full execution of .ideavimrc.
And I took your suggestion about setting the flag directly in the "execute" function, thank you for that.

60c27b1
44c8a97
288c66d

@chylex chylex deleted the pr/vim-2933 branch November 3, 2023 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants