intellij-plugin-save-actions icon indicating copy to clipboard operation
intellij-plugin-save-actions copied to clipboard

Update documentation "Save Document" do not work anymore

Open dubreuia opened this issue 6 years ago • 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?

dubreuia avatar Jan 30 '19 18:01 dubreuia

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.

bborchard avatar Mar 08 '19 22:03 bborchard

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)

dubreuia avatar Mar 14 '19 15:03 dubreuia

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.

bborchard avatar Mar 14 '19 16:03 bborchard

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.

dubreuia avatar Mar 23 '19 22:03 dubreuia

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

dubreuia avatar Mar 23 '19 23:03 dubreuia

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)

dubreuia avatar Mar 23 '19 23:03 dubreuia

None of these two solutions work from the vim plugin, right?

snaggen avatar Apr 18 '19 11:04 snaggen

Unfortunately, I haven't figured a way of making the vim plugin work without breaking the fix I've done for the general case.

dubreuia avatar Apr 18 '19 16:04 dubreuia

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().saveDocument won'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.

dubreuia avatar Apr 19 '19 12:04 dubreuia

Save files automatically if application is idle for x sec does trigger the format though, which actually is quite nice.

webberwang avatar Apr 25 '19 04:04 webberwang

Save all support only

dubreuia avatar Dec 13 '19 09:12 dubreuia

doc vim + single save

dubreuia avatar Dec 13 '19 09:12 dubreuia

Reopening check "issue_222_document_single_save" branch I forgot about this: document limitations and event lifecycle

dubreuia avatar Feb 01 '20 18:02 dubreuia

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.

coderjz avatar Feb 04 '20 16:02 coderjz

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.

dubreuia avatar Feb 04 '20 21:02 dubreuia

@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 :(

yaeuge avatar Feb 06 '20 11:02 yaeuge

https://github.com/dubreuia/intellij-plugin-save-actions/tree/issue_222_document_single_save

dubreuia avatar Apr 23 '20 10:04 dubreuia