SVN commit dialog now supports diff, add and revert actions.
ClosedPublic

Authored by nikolaik on Feb 25 2020, 9:48 AM.

Details

Summary

Added SVN commit dialog actions for diff file, add file and revert file.
This is done by commit dialog context menu actions.
SVN commit dialog is now a separate class and moved to a separate file.
Also added SVNCommands class with static methods to access basic SVN actions like geting revision or geting remote URL path for a file.
Also added ItemVersion::MissingVersion control for SVN plugin.

Test Plan

Run SVN commit dialog and check diff file works, revert works and add works.

Diff Detail

Repository
R449 Plugins for Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
nikolaik requested review of this revision.Feb 25 2020, 9:48 AM
nikolaik created this revision.

This is a huge one guys, sorry for this.
Found this update useful for my everyday needs.
As i think SVNCommands class should be a class to process all SVN commands in the future (if I'll do it now this will be a realy big commit).
FileViewSvnPlugin should be a class for plugin UI (call other dialogs, update items states).

I am ready for comments and updates.

nikolaik updated this revision to Diff 76349.Feb 25 2020, 10:14 AM

Updated comments.

nikolaik updated this revision to Diff 76438.Feb 26 2020, 7:42 AM

Added icons for context menu actions based on default VCS plugin actions.

meven requested changes to this revision.Feb 26 2020, 8:44 AM

Just a couple nitpick one regarding cmake and the other about a refresh button.

The point regarding supporting multiple files is out of scope here.

This is some nice code, nice separation of logic and UI, Enum and documentation, makes it easy to review.

svn/CMakeLists.txt
10

Do you really need a set for HDRS ?
Usually in KDE projects we only have a comprising cpp files, they should include the headers anyway.

svn/fileviewsvnplugin.h
70

Eventually we woulde need those functions to receive several files QList<QString> and rename them to revertFiles, to make the API future proof and the usage more flexible.

svn/svncommitdialog.cpp
239

Add a button so the user can trigger this himself using the UI and discover this shortcut.
For instance if he did modifications from a Shell.
Left of "commit" button as a default.

This revision now requires changes to proceed.Feb 26 2020, 8:44 AM
nikolaik updated this revision to Diff 76521.Feb 27 2020, 9:05 AM

Thanks for advices!

Changes:

  1. No more headers in the CMakeLists.txt
  2. Added QStringList for addFiles, revertFiles and so on.
  3. Added "Refresh" button.

Also added possibility for user to commit just a few files or just one directory, thanks for this idea, great feature.
Added more comments and explanations.

@meven What do you think about using an UI files?

nikolaik marked 3 inline comments as done.Feb 27 2020, 9:06 AM
nikolaik edited the summary of this revision. (Show Details)Feb 27 2020, 9:11 AM
meven added a comment.Feb 27 2020, 9:39 AM

Thanks for advices!

You are welcome. This is some nice work.

Changes:

  1. No more headers in the CMakeLists.txt

:)

  1. Added QStringList for addFiles, revertFiles and so on.

I didn't think this would be easy to add here, nice !

  1. Added "Refresh" button.

Add an icon to it and associate the shortcut rather than implement SvnCommitDialog::keyPressEvent

Also added possibility for user to commit just a few files or just one directory, thanks for this idea, great feature.
Added more comments and explanations.

@meven What do you think about using an UI files?

You mean .ui Qt file ? Personally I like those, you can reduce C++ code extracting UI setup code to dedicated xml.
But here since you have already done the work, up to you to add some more work.

svn/fileviewsvnplugin.cpp
459

Todos are more commonly written //TODO within our codebase.

491

Well now is a good time ;)

svn/svncommitdialog.cpp
115

You can probably find an appropriate icon and associate the shortcut to this action/button so that it is shown in the tooltip.

nikolaik updated this revision to Diff 76540.Feb 27 2020, 1:01 PM

Updated.

  1. Added shortcut for a refresh action in the commit dialog (standard system hotkey).
  2. Added icon for a refresh button.
  3. Added comments.

Also fixed commit action in a separate SVN transactions for each file: now it is done in one operation.

nikolaik marked 3 inline comments as done.Feb 27 2020, 1:02 PM
nikolaik edited the summary of this revision. (Show Details)Feb 27 2020, 1:06 PM
nikolaik updated this revision to Diff 76542.Feb 27 2020, 1:43 PM

Added missing QLatin1String() for SVN commands.

meven added a subscriber: ngraham.Feb 27 2020, 3:40 PM

I wish we could have a diff on a side panel, but this is already a nice improvement !

I am thinking the context menu actions are not very discoverable, buttons for actions could be added later.

Please wait for @elvisangelaccio or @ngraham review, but IMHO it is ready for merging.

meven accepted this revision.Mar 9 2020, 7:31 AM

I meant to leave some time for other reviewers, but IMO this is ready for merging.

This revision is now accepted and ready to land.Mar 9 2020, 7:31 AM
ngraham accepted this revision.Mar 9 2020, 4:55 PM

@meven, wanna land this?

This revision was automatically updated to reflect the committed changes.