Update documentation "Save Document" do not work anymore
dubreuia opened this issue · 17 comments
From the plugin page but I don't know what that means https://plugins.jetbrains.com/plugin/7642-save-actions.
In new release (1.4.0) actions on single Ctrl+S yet has been broken :( (Android Studio 3.3.0 on Ubuntu 18.04)
Does CTRL+S do something?
It looks like the save actions only trigger when the 'Save All' action is triggered. It looks like 'Save All' is standard save action on ctrl+s (it is also the only save action in the file menu), so it is fine for most users, but 'Save Document' can be mapped to ctrl+s or any other binding using the keymap, and if a user has done this, the actions in this plugin won't trigger. This is also the case for the vim plugin (see #220).
I'm guessing the solution is as simple as overriding the method FileDocumentManagerListener.beforeDocumentSaving in addition to FileDocumentManagerListener.beforeAllDocumentsSaving in the SaveActionManager and having it only process the document that is passed in on that hook
That being said I have only really glanced at this code and definitely haven't tested it. Just a thought.
Hey @bborchard, overriding beforeDocumentSaving is definitely part of the solution, but often both method will get called (first beforeAllDocumentsSaving then beforeDocumentSaving for each file) so I need to make sure it doesn't trigger twice for a file (maybe it doesn't matter that much I don't know)
Ah, that makes sense. I suppose I shouldn't be surprised that this is more complicated than it looks. If I have some time later this week, maybe I'll try to set up a dev environment for this and see if there is some non-disruptive way to get this to work for 'Save Document' as well as 'Save All' - seems like there are a decent number of vim users affected.
Current state is: since refactoring, only beforeAllDocumentsSaving is working, when the plugin is called from beforeDocumentSaving nothing happens, because I've removed all the code there. The problem I have: if I re-add the code, it ALWAYS results in a "Attempt to modify PSI for non-committed Document!` stack from #210 for whatever reason.
This affects vim plugin, and some users that seems to use only single save, also some IDE that seems to be configured that way (PHPStorm, see #224, needs testing).
I have no idea how to fix this.
Ok I found the problem but I don't know how to fix it.
The beforeAllDocumentSaving is called here: com.intellij.openapi.fileEditor.impl.FileDocumentManagerImpl.java:298, and guard is down from the call just above ((TransactionGuardImpl)TransactionGuard.getInstance()).assertWriteActionAllowed(); which makes it possible to modify the documents in the save actions.
The beforeDocumentSaving is called here: com/intellij/openapi/fileEditor/impl/FileDocumentManagerImpl.java:419 and is guarded by PomModelImpl.guardPsiModificationsIn(lambda) which makes it impossible to modify this specific document.
I don't actually understand how this could work before, but I surely don't know how to fix it. I'm trying to find a workaround
We are left with two solutions:
- Anybody someone wanting to use the plugin must remap it's "Save Document (CTRL-S)" to "Save All" (save all is default in intellij idea)
- Dynamically override the "Save Document" action at startup http://www.jetbrains.org/intellij/sdk/docs/basics/action_system.html (I trying doing it in the plugin.xml but it doesn't work)
None of these two solutions work from the vim plugin, right?
Unfortunately, I haven't figured a way of making the vim plugin work without breaking the fix I've done for the general case.
I cannot use the action "Save Document" anymore. Like not at all, at least for now.
That means:
- Anybody using it directly with CTRL-S, for example, needs to rebind that shortcut to "Save All", I think it's the default in non-java IDEs like WebStorm (I need to check that)
- Any plugin calling directly
FileDocumentManager.getInstance().saveDocumentwon't work, this is the case of the IdeaVim plugin (see https://github.com/JetBrains/ideavim/blob/38a4fd5fbcf4b37990391d68c67e16d84f427c33/src/com/maddyhome/idea/vim/group/FileGroup.java#L167)
For the IdeaVim plugin, my recommended workaround is to call :wa to trigger the plugin.
Save files automatically if application is idle for x sec does trigger the format though, which actually is quite nice.
Save all support only
doc vim + single save
Reopening check "issue_222_document_single_save" branch I forgot about this: document limitations and event lifecycle
I have found a workaround that is working better for me. When I tried :wa I noticed that it would take several seconds on a single file, and would take much longer on multiple files. This was unacceptably slow.
However, when the plugin setting "Activate Save Actions on Shortcut" is enabled, then the plugin can be executed by doing :action com.dubreuia.core.action.ShortcutAction.
In my .ideavimrc I've remapped :w to :action com.dubreuia.core.action.ShortcutAction with the command nnoremap :w :action com.dubreuia.core.action.ShortcutAction. This remapping works well because I have automatic save setup in IntelliJ, so I don't actually need to do :w to save the file.
Hey @coderjz, thanks for pointing this. I'll be adding this to the readme documentation for other users. I really don't understand why :wa takes that long thought, I might try to profile that and see what is the problem.
@coderjz , thanks for the hint!
I have autosave disabled, so I came up with a bit ugly
nnoremap :w :action com.dubreuia.core.action.ShortcutAction<CR>:action SaveDocument
Unfortunately, IdeaVim doesn't support chaining commands ( https://youtrack.jetbrains.com/issue/VIM-748 ), so
nnoremap :w :action com.dubreuia.core.action.ShortcutAction <bar> action SaveDocument
won't work :(