[svn] Added SVN Checkout dialog.
ClosedPublic

Authored by nikolaik on May 6 2020, 5:45 PM.

Details

Summary


Added SVN Checkout dialog with basic and most commonly used SVN Checkout functionality:

  • set repo URL;
  • change working copy path;
  • omit externals.

Additional features:

  • automatically pasting repo URL on dialog creation (if its valid);
  • automatically forming working copy path based on repo URL (trying to predict repository name);
  • user can pick an output folder instead of typing in;
  • few error-controls to prevent running 'svn co' with invalid params (disabling 'OK' button).

SVN Checkout can be time consuming.

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.May 6 2020, 5:45 PM
nikolaik created this revision.
nikolaik retitled this revision from Added SVN Checkout dialog. to [svn] Added SVN Checkout dialog..May 6 2020, 5:50 PM
nikolaik edited the summary of this revision. (Show Details)
anthonyfieroni added inline comments.
svn/fileviewsvnplugin.cpp
127–130

This action is shown when plugin is loaded, but that's when it has folder .svn

nikolaik marked an inline comment as done.May 6 2020, 6:36 PM
nikolaik added inline comments.
svn/fileviewsvnplugin.cpp
127–130

There is a new public virtual function for out-of-version-control folders, https://phabricator.kde.org/D29041

meven added a comment.May 7 2020, 12:31 PM

A couple of minor points, but this looks great already.

svn/svncheckoutdialog.cpp
73

Check that repoPath looks like a remote svn repo url with schemes svn:/ https:/ or http:/ (can svn use others ?)

74

Don't need to else here.
A user might have the url in the clipboard and the target dir selected.

91

Can reuse variable stripped

meven added inline comments.May 7 2020, 12:32 PM
svn/svncommands.cpp
420

If we could add some progress dialog of some kind, allowing the user to kill the process.

nikolaik edited the summary of this revision. (Show Details)May 7 2020, 12:57 PM
nikolaik marked 2 inline comments as done.
nikolaik marked an inline comment as done.May 7 2020, 1:22 PM
nikolaik added inline comments.
svn/svncheckoutdialog.cpp
74

Yes, that is correct, this is the most common use strategy.
The idea was "if clipboard contains correct URL set it through on_leRepository_textChanged() which sets work directory path, if no URL just set the work directory path to the current dir".
Do you think about something better?

svn/svncommands.cpp
420

This is a huge problem now and it is in todo list.
SvnCommands should become a singleton with signal-slots connection to run svn process and not block Dolphin main thread.
I think this is a separate commit change.
If we can't accept this without a normal multithreading it's ok i will try to implement it first.

nikolaik updated this revision to Diff 82198.May 7 2020, 1:24 PM
nikolaik marked an inline comment as done.

Update in response to review comments: added more reuse of 'stripped' variable.

nikolaik marked an inline comment as done.May 7 2020, 1:25 PM
nikolaik added inline comments.May 7 2020, 1:46 PM
svn/svncheckoutdialog.cpp
73

http://svnbook.red-bean.com/en/1.2/svn-book.html#svn.basic.in-action.wc.tbl-1
Ok, we can check probably with regex. Some basic checks: this is URL (QUrl().isValid()) and starts correct.

nikolaik updated this revision to Diff 82225.May 7 2020, 5:18 PM

More appropriate repo URL validation.

nikolaik marked 2 inline comments as done and an inline comment as not done.May 7 2020, 5:19 PM
meven accepted this revision as: meven.May 7 2020, 5:44 PM

All in all this is good.

I hope you'll do a git dialog ;)
Unless I do first.

svn/svncheckoutdialog.cpp
51

I would have reversed this, first QUrl::fromUserInput, then isvalid() then .scheme() in [listof svn repo url scheme], but this is fine

74

I missed textChanged edited leCheckoutDir.
Your idea is nice.

This revision is now accepted and ready to land.May 7 2020, 5:44 PM
nikolaik updated this revision to Diff 82229.May 7 2020, 6:46 PM

Improved URL checks.

nikolaik marked 4 inline comments as done.May 7 2020, 6:47 PM
meven accepted this revision.May 11 2020, 6:41 AM
meven added inline comments.
svn/svncheckoutdialog.cpp
54

You could return here.

nikolaik updated this revision to Diff 82496.May 11 2020, 8:38 AM

Update in response to review comments.

nikolaik marked an inline comment as done.May 11 2020, 8:39 AM
meven added a comment.May 11 2020, 1:08 PM

I think you can land it, code is nice and changes should be consensual.

svn/svncheckoutdialog.cpp
68

Do you really need QObject:: here ?

70

And here ?

elvisangelaccio accepted this revision.May 11 2020, 5:40 PM
elvisangelaccio added inline comments.
svn/svncheckoutdialog.h
36–37

Please use camelCase.

nikolaik updated this revision to Diff 82605.May 11 2020, 6:38 PM

Moved from QObject::connect to connect.

nikolaik marked 5 inline comments as done.May 11 2020, 6:42 PM
nikolaik added inline comments.
svn/svncheckoutdialog.h
36–37

This is for slots autoconnection: https://doc.qt.io/qt-5/qmetaobject.html#connectSlotsByName
I can manually connect if needed.

elvisangelaccio accepted this revision.May 11 2020, 8:48 PM
elvisangelaccio added inline comments.
svn/svncheckoutdialog.h
36–37

Ah sorry, didn't know about this feature :)

nikolaik marked 3 inline comments as done.May 12 2020, 5:44 AM
This revision was automatically updated to reflect the committed changes.