Add changes list for SVN commit dialog

Authored by nikolaik on Jan 25 2020, 6:46 PM.



SVN commit dialog box contains no list of changes that will be commited. This is often confusing.
This commit adds basic changes list for SVN commit action.
Also contains workaround for QTBUG-40584 to preserve dialog sizes, see KWindowConfig::restoreWindowSize() docs.

Diff Detail

R449 Plugins for Dolphin
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
nikolaik requested review of this revision.Jan 25 2020, 6:46 PM
nikolaik created this revision.
nikolaik edited the summary of this revision. (Show Details)Jan 26 2020, 6:46 AM
meven added a comment.Jan 26 2020, 7:55 PM

Small nitpick, looks good


You probably can use Q_ASSERT

nikolaik updated this revision to Diff 74408.Jan 27 2020, 8:42 AM

Updated in response to review comments: added Q_ASSERT, added UpdateRequiredVersion because this status is possible.

nikolaik marked an inline comment as done.Jan 27 2020, 8:43 AM
nikolaik added inline comments.Jan 27 2020, 8:59 AM

We should be really careful with this asserts because there is no place documenting for example "There should be no NormalVersion". One can change beginRetrieval() and because we doesn't have test units for everything this could damage SVN plugin in some rare cases.
In my opinion better to continue working without plugin crash with empty status and qWarning() instead. It is also worth noting that commitFiles() can do its job anyway.

meven accepted this revision as: meven.Jan 27 2020, 9:11 AM
meven added inline comments.

I am fine with either solution.

This revision is now accepted and ready to land.Jan 27 2020, 9:11 AM
nikolaik updated this revision to Diff 74421.Jan 27 2020, 1:26 PM

Moved from Q_ASSERT to qWarning().

nikolaik marked 2 inline comments as done.Jan 27 2020, 1:26 PM
ngraham accepted this revision as: VDG, ngraham.Jan 27 2020, 4:37 PM
ngraham added a subscriber: ngraham.

LGTM too. @elvisangelaccio?

meven added inline comments.Feb 18 2020, 8:15 AM

Can we use some context here ?

nikolaik updated this revision to Diff 75888.Feb 18 2020, 8:48 AM

Added context.

nikolaik marked an inline comment as done.Feb 18 2020, 8:49 AM
ngraham accepted this revision.Feb 18 2020, 8:13 PM

This has sat under review for long enough and it works fine. Landing it. :)

This revision was automatically updated to reflect the committed changes.