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

Update documentation "Save Document" do not work anymore #222

Open
dubreuia opened this issue Jan 30, 2019 · 17 comments
Open

Update documentation "Save Document" do not work anymore #222

dubreuia opened this issue Jan 30, 2019 · 17 comments

Comments

@dubreuia
Copy link
Owner

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?

@bborchard
Copy link

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.

@dubreuia
Copy link
Owner Author

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)

@bborchard
Copy link

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.

@dubreuia dubreuia changed the title Single Ctrl+S has been broken "Save Document" do not work anymore Mar 23, 2019
@dubreuia
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

dubreuia commented Mar 23, 2019

We are left with two solutions:

@snaggen
Copy link

snaggen commented Apr 18, 2019

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

@dubreuia
Copy link
Owner Author

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
Copy link
Owner Author

I cannot use the action "Save Document" anymore. Like not at all, at least for now.

That means:

For the IdeaVim plugin, my recommended workaround is to call :wa to trigger the plugin.

@webberwang
Copy link

webberwang commented Apr 25, 2019

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

@dubreuia
Copy link
Owner Author

Save all support only

@dubreuia dubreuia removed the bug label Dec 13, 2019
@dubreuia
Copy link
Owner Author

doc vim + single save

@dubreuia
Copy link
Owner Author

dubreuia commented Feb 1, 2020

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

@dubreuia dubreuia reopened this Feb 1, 2020
@coderjz
Copy link

coderjz commented Feb 4, 2020

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.

@dubreuia
Copy link
Owner Author

dubreuia commented Feb 4, 2020

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.

@yaeuge
Copy link

yaeuge commented Feb 6, 2020

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

@dubreuia dubreuia changed the title "Save Document" do not work anymore Update "Save Document" do not work anymore Sep 11, 2020
@dubreuia dubreuia changed the title Update "Save Document" do not work anymore Update documentation "Save Document" do not work anymore Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants