Skip to content

Refactor recent files list managing, splitting responsibility from KRecentFilesAction

Alvin Wong requested to merge alvinwong/krita:alvin/recent-files-refactor into master

(Follow-up on !1283 (merged))

This refactoring splits the code responsible for loading, saving and managing the recent files list from our forked copy of KRecentFilesAction into a new KisRecentFilesManager singleton.

Before: Each KisMainWindow contains its own KRecentFilesAction, which not only manages the recent files menu but also handles loading and saving of the list for that KisMainWindow. Whenever a file is opened or saved, the request to add or bump an URL in the list is sent to all KisMainWindows via KisPart, which is then sent to their child KRecentFilesAction. Each of them also saves the list on their own, so with multiple KisMainWindows the config is written multiple times. To make it more complicated, a KisMainWindow may also notify other KisMainWindows to reload the recent file list after saving to keep their lists synchronized.

After: The KisRecentFilesManager singleton handles the config read/write and any changes to the recent files list. KRecentFilesAction now only handles the recent files menu, and gets notified with any changes to the list through signals from KisRecentFilesManager. KisPart also uses KisRecentFilesManager directly and no longer needs to go through KisMainWindow for recent files handling.

The KisRecentDocumentsModelWrapper is still responsible for handling the file icons and providing the model for the welcome widget. While it has also been changed to use KisRecentFilesManager (instead of relying on its KisMainWindow to notify it of any changes to the list), there is not much changes to how it works. I'll look into refactoring it as a follow-up (I think it will be needed to implement lazy-loading of the file icons).

Targetting this change for 5.1.

Licensing Oddity

KRecentFilesAction was forked from KConfigWidgets (https://invent.kde.org/frameworks/kconfigwidgets/-/blob/master/src/krecentfilesaction.cpp) which uses LGPL 2.0 (not LGPL 2.1). In making KisRecentFilesManager I reused some code from KRecentFilesAction, but I don't feel like making new code in LGPL 2.0 and having to relicense it later, so I opted to split the implementation into two files. Though I am not sure if this is really a better choice.

The relicensing of KRecentFilesAction is tracked in teams/licensing/issues#34.

Test Plan

  • Check that the recent file list is loaded on startup
  • Check that a file is bumped to the top when opening
  • Check that a file is added to the top when saving
  • Check that the list is synchronized between multiple KisMainWindows

Formalities Checklist

  • I confirmed this builds.
  • I confirmed Krita ran and the relevant functions work.
  • I tested the relevant unit tests and can confirm they are not broken. (If not possible, don't hesitate to ask for help!)
  • I made sure my commits build individually and have good descriptions as per KDE guidelines.
  • I made sure my code conforms to the standards set in the HACKING file.
  • I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.

Merge request reports

Loading