Refactor BaseAction in RightClickMenu

[calixtus]

I started by bugfixing some missing executablebindings. It totally escalated. Sorry.
I believe this could be the beginning of an approach to get rid of the deprecated BaseAction and OldCommandWrapper stuff.

I stop here, because this PR is becoming quite complex even though I tried to keep my modifications short and simple (e.g. I did not extract BasePanel or Globals.prefs, if I did not had to). This PR is theoretically mergable, but we should talk about it first.

I tested everything by hand, it should all work like before.

L10n yet missing, I'm doing this in the very end.

• Change in CHANGELOG.md described (if applicable)
• Tests created for changes (if applicable)
• Manually tested changed features in running JabRef (always required)
• Screenshots added in PR description (for bigger UI changes)
• Checked documentation: Is the information available and up to date? If not: Issue created at https://github.com/JabRef/user-documentation/issues.

[Siedlerchr]

Wow, thanks for your huge work! So far I could not see anything which is breaking

[koppor]

Wow 🤩. Good to see that the GUI code gets better. That always causes much changes. I am really happy that you accepted the challange to do so.

You can also start working on bigger refactorings. We need to reduce Globals.prefs. This will cause a huge diff. Similar to the entry editor refactorings.

So, I would propose that Could not open link, External viewer called, No URL defined is added to the localization and we merge.

[calixtus]

Made some final corrections. Should be mergeable now, but I'm not sure if it's best to merge this PR before the next major release. Maybe there are some bugs I did not spot and there would be not much time for the latest dev users to test the changes...

[koppor]

@tobiasdiez If possible, could you have a quick look?

We would merge it to use the current energy of @calixtus and to avoid huge merge conflicts.

[calixtus]

Just to be clear, there is still a long way to go with the BaseActions, I could not yet take all of them out of the RightClickMenu. Especially the SpecialFields and the EditActions are closely interlinked with the BaseActions.

[tobiasdiez]

Very good work! I haven't found any major issues and as usual the code quality is really good.

I echo the concern of @calixtus and would recommend waiting with the merge after the release this weekend. There is no need to risk that half of the menu is not working (although the risk for this is really low).

[calixtus]

Sorry, wrong commit title.

[calixtus]

You can also start working on bigger refactorings. We need to reduce Globals.prefs. This will cause a huge diff. Similar to the entry editor refactorings.

I think the most sensible way to proceed is, after completely extracting BaseAction, to untangle BasePanel and BibDatabaseContext a bit. At the same time, the remaining Globals.prefs calls can be broken down a bit into individual preferences classes. When this is done, it should not be a big problem anymore to reduce the unnecessary references to JabRefFrame and BasePanel.

But what will need some discussion is how to proceed with the UndoManager (Partly it doesn't work, because some actions are commented out). If I understand it correctly, it can be distinguished whether it should create an 'UndoStream' for each library or unify it for all libraries.

[tobiasdiez]

Sounds like a reasonable program. 👍

The undo manager needs to be completely rewritten (we are still using the Swing one). My idea was that one needs to only register what action is performed and the undo manager automatically listens to all changes in the open databases.
Thus in pseudo-code:

// Somewhere global
UndoManager.listenForChanges(currently open database);

// If an action is performed
try (UndoManager.newOperation("Action name")) {
   entry.setField(blaba);
}

In this way, the logic package can be completely independent of the undo manager and does not need to care to return change information (e.g. as it's done right now in the setField method).

Some links that might be helpful:
§ https://github.com/TomasMikula/UndoFX
§ https://github.com/FXMisc/UndoFX
§ http://torgen-engineering.blogspot.de/2016/05/how-to-bring-undoredo-features-to-your.html
§ https://stackoverflow.com/questions/42841258/how-to-undo-redo-a-command-with-javafx
https://stackoverflow.com/questions/40638845/implementing-undo-redo-in-javafx

[calixtus]

The RightClickMenu should now be completely clean of any BaseAction stuff. No more hidden BaseAction in SpecialFields or so. Next step (definitely in another PR) will be to extract BaseAction out of JabRefFrame (main menu and toolbar). But that shouldn't be a big deal anymore (except maybe SaveAction), but most of the funny stuff is already done.

[koppor]

The ADR on using the Swing Undomanager was in short: Why reimplementing things already industry proven only because the package name is wrong.

Maybe with the new module system, we can skip shipping the swing module so we reduce the binary size when reimplementing the UndoManager.