D20838: Allow to drop one file or one folder on KDirOperator

2019-07-23 Thread Méven Car 
meven added a comment.


  ping @dfaure

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham, #frameworks
Cc: dfaure, elvisangelaccio, apol, kde-frameworks-devel, LeGast00n, sbergeron, 
michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-07-17 Thread Méven Car 
meven added a comment.


  In D20838#492751 , @dfaure wrote:
  
  > Strange, I'm pretty sure I was able to reproduce it locally before I wrote 
my comment here. But now I can't anymore. OK, to be further investigated when 
either of us have time.
  
  
  I have an hypothesis concerning the issue, and why we have a hard time 
reproducing it ourselves.
  
  I had to add a wait in the test to make it predictable, which I am not proud 
of :
  
  line 490 in kfilewidgettest.cpp
  
// once we drop a file the dirlister scans the dir
// wait a bit to the dirlister time to finish
QTest::qWait(100);
  
  So this may work well on fast systems like my owns with SSD but on CI with 
less resources this might not be long enough.
  Either we can extend a bit this wait and hope for the best or maybe if you 
see a way to actually get an IO signal when the dirlister is done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham, #frameworks
Cc: dfaure, elvisangelaccio, apol, kde-frameworks-devel, LeGast00n, sbergeron, 
michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-07-09 Thread David Faure 
dfaure added a comment.


  Strange, I'm pretty sure I was able to reproduce it locally before I wrote my 
comment here. But now I can't anymore. OK, to be further investigated when 
either of us have time.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham, #frameworks
Cc: dfaure, elvisangelaccio, apol, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-07-09 Thread Méven Car 
meven added a comment.


  In D20838#491992 , @dfaure wrote:
  
  > The test fails in CI, please check.
  >
  > 
https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.12/job/kio/job/kf5-qt5%20SUSEQt5.12/139/testReport/junit/projectroot/autotests/kiofilewidgets_kfilewidgettest/
  >  
https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.13/job/kio/job/kf5-qt5%20SUSEQt5.13/7/testReport/projectroot/autotests/kiofilewidgets_kfilewidgettest/
  >
  >   PASS   : KFileWidgetTest::testDropFile(some.txt)
  >   FAIL!  : KFileWidgetTest::testDropFile(subdir/some.txt) Compared values 
are not the same
  > Actual   (fileWidget.locationEdit()->currentText()): ""
  > Expected (expectedCurrentText) : "some.txt"
  > Loc: [/home/jenkins/workspace/Frameworks/kio/kf5-qt5 
SUSEQt5.12/autotests/kfilewidgettest.cpp(496)]
  
  
  I can't reproduce this test failure locally.
  
$ ctest -R kfilewidget
Test project /home/meven/kde/build/kio
Start 48: kiofilewidgets-kfilewidgettest
1/1 Test #48: kiofilewidgets-kfilewidgettest ...   Passed9.06 sec

100% tests passed, 0 tests failed out of 1
  
  It may depend on the specific versions used, or environment.
  
  My system uses :
  KDE Plasma Version: 5.16.80
  KDE Frameworks Version: 5.60.0
  Qt Version: 5.12.2
  Kernel Version: 5.0.0-20-generic
  
  In any case this needs some investigation, that I will dig into later, I 
don't have much time right now.
  But without reproducing it, it might be challenging.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham, #frameworks
Cc: dfaure, elvisangelaccio, apol, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-07-07 Thread David Faure 
dfaure added a comment.


  The test fails in CI, please check.
  
  
https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.12/job/kio/job/kf5-qt5%20SUSEQt5.12/139/testReport/junit/projectroot/autotests/kiofilewidgets_kfilewidgettest/
  
https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.13/job/kio/job/kf5-qt5%20SUSEQt5.13/7/testReport/projectroot/autotests/kiofilewidgets_kfilewidgettest/
  
  PASS   : KFileWidgetTest::testDropFile(some.txt)
  FAIL!  : KFileWidgetTest::testDropFile(subdir/some.txt) Compared values are 
not the same
  
Actual   (fileWidget.locationEdit()->currentText()): ""
Expected (expectedCurrentText) : "some.txt"
Loc: [/home/jenkins/workspace/Frameworks/kio/kf5-qt5 
SUSEQt5.12/autotests/kfilewidgettest.cpp(496)]

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham, #frameworks
Cc: dfaure, elvisangelaccio, apol, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-18 Thread Méven Car 
This revision was automatically updated to reflect the committed changes.
meven marked an inline comment as done.
Closed by commit R241:25eb17ffa666: Allow to drop one file or one folder on 
KDirOperator (authored by meven).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=58000=58242

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  autotests/kfilewidgettest.cpp
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/widgets/kdirmodel.cpp
  src/widgets/kdirmodel.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham, #frameworks
Cc: elvisangelaccio, apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-17 Thread Nathaniel Graham 
ngraham accepted this revision.
ngraham added a comment.


  I think this can land.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham, #frameworks
Cc: elvisangelaccio, apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-13 Thread Méven Car 
meven updated this revision to Diff 58000.
meven added a comment.


  Review, removed a dead variable in test, use QRegularExpression

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57707=58000

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  autotests/kfilewidgettest.cpp
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/widgets/kdirmodel.cpp
  src/widgets/kdirmodel.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham, #frameworks
Cc: elvisangelaccio, apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-13 Thread Méven Car 
meven marked 8 inline comments as done.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham, #frameworks
Cc: elvisangelaccio, apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-11 Thread Elvis Angelaccio 
elvisangelaccio added inline comments.

INLINE COMMENTS

> kfilewidgettest.cpp:38
> +#include 
> +#include 
> +

Unused?

> kfilewidgettest.cpp:407-413
> +QDir f(tempDir.filePath(dir));
> +
> +KFileWidget fw(QUrl::fromLocalFile(tempDir.path()));
> +fw.setOperationMode(KFileWidget::Opening);
> +fw.setMode(KFile::File);
> +fw.show();
> +fw.activateWindow();

Please use descriptive variable names instead of `f` and `fw`

> kfilewidgettest.cpp:415
> +
> +QList list_urls = {fileUrl};
> +QMimeData *mimeData = new QMimeData();

Missing camelCase

> kdiroperator.cpp:1404-1405
> +
> +QMimeDatabase md;
> +QMimeType mt = md.mimeTypeForUrl(url);
> +

Please use descriptive variable names also here.

> kdiroperator.cpp:1407
> +
> +QRegExp r;
> +r.setPatternSyntax(QRegExp::Wildcard);   // the "mimetype" 
> can be "image/*"

QRegExp should be avoid in new code, can you try to use QRegularExpression 
instead?

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20838_1

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham, #frameworks
Cc: elvisangelaccio, apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-08 Thread Méven Car 
meven added a reviewer: Frameworks.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20838_1

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham, #frameworks
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-08 Thread Nathaniel Graham 
ngraham added a comment.


  Given that nobody else from #frameworks 
 has offered any review comments, 
I say let's get it in early in this Frameworks cycle (i.e. now-ish) so we have 
almost a month of testing before release.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20838_1

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-08 Thread Méven Car 
meven added a comment.


  @ngraham Should I wait for some more review/testing, or LGTM means let's get 
this merge ASAP ?

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20838_1

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-07 Thread Méven Car 
meven updated this revision to Diff 57707.
meven added a comment.


  update since comment kio version

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57636=57707

BRANCH
  arcpatch-D20838_1

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  autotests/kfilewidgettest.cpp
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/widgets/kdirmodel.cpp
  src/widgets/kdirmodel.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-06 Thread Nathaniel Graham 
ngraham edited the summary of this revision.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-06 Thread Nathaniel Graham 
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  LGTM!

INLINE COMMENTS

> meven wrote in kdiroperator.cpp:418
> Any idea where I should add this information ?
> An example would suffice.

You already did, in the inline function documentation in 
`src/filewidgets/kdiroperator.h`

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-06 Thread Méven Car 
meven updated this revision to Diff 57636.
meven edited the test plan for this revision.
meven added a comment.


  Fix drag^Ciltering, add mime filtering to the drag filtering

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57625=57636

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  autotests/kfilewidgettest.cpp
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/widgets/kdirmodel.cpp
  src/widgets/kdirmodel.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-06 Thread Méven Car 
meven planned changes to this revision.
meven added a comment.


  An eventFilter must return true when the event has been processed and prevent 
further event handling.
  This prevent the drag filtering to not work (multiple files for instance).
  The code does not do that currently, I am on it.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-06 Thread Méven Car 
meven edited the summary of this revision.
meven edited the test plan for this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-05 Thread Méven Car 
meven updated this revision to Diff 57625.
meven added a comment.


  review feedback

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57599=57625

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  autotests/kfilewidgettest.cpp
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/widgets/kdirmodel.cpp
  src/widgets/kdirmodel.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-05 Thread Méven Car 
meven marked 5 inline comments as done.
meven added inline comments.

INLINE COMMENTS

> ngraham wrote in kdiroperator.cpp:418
> Yep, I'd say so now.

Any idea where I should add this information ?
An example would suffice.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-05 Thread Nathaniel Graham 
ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  Still works great, and I have just a few code change requests. Then let's aim 
to land this early in the 5.59 cycle so it gets lots of testing.

INLINE COMMENTS

> meven wrote in kdiroperator.cpp:418
> Not sure I have done this correctly.

Yep, I'd say so now.

> kdiroperator.cpp:1404
> +
> +this->setFocus();
> +

`this->` not needed

> kdiroperator.cpp:1418
> +auto urlSetterClosure = [this, url](){
> +this->setCurrentItem(url);
> +QObject::disconnect(d->m_connection);

`this->` not needed

> kdiroperator.cpp:1423
> +} else {
> +this->setCurrentItem(url);
> +}

`this->` not needed

> kdiroperator.cpp:1558
>  
> +if (this->acceptDrops()) {
> +newView->setAcceptDrops(true);

`this->` not needed

> kfilewidgettest_gui.cpp:36
>  }
> -

Unnecessary whitespace change.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-05 Thread Méven Car 
meven updated this revision to Diff 57599.
meven added a comment.


  oops

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57597=57599

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  autotests/kfilewidgettest.cpp
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/widgets/kdirmodel.cpp
  src/widgets/kdirmodel.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-05 Thread Méven Car 
meven updated this revision to Diff 57597.
meven added a comment.


  Clean up test code

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57589=57597

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  autotests/kfilewidgettest.cpp
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/widgets/kdirmodel.cpp
  src/widgets/kdirmodel.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-05 Thread Méven Car 
meven updated this revision to Diff 57589.
meven added a comment.


  Clean up and fix test

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57442=57589

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  autotests/kfilewidgettest.cpp
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/widgets/kdirmodel.cpp
  src/widgets/kdirmodel.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-03 Thread Nathaniel Graham 
ngraham added a task: T8552: Polish Open/Save dialogs.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-03 Thread Méven Car 
meven updated this revision to Diff 57442.
meven added a comment.


  Grab the focus so that the currentItem becomes the kfilewidget selected item, 
otherwise the user needs to give the kdiroperator the focus first

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57441=57442

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  autotests/kfilewidgettest.cpp
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/widgets/kdirmodel.cpp
  src/widgets/kdirmodel.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-05-03 Thread Méven Car 
meven updated this revision to Diff 57441.
meven added a comment.


  Fix an issue where the closuser would be called for each finished event once 
set

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57124=57441

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  autotests/kfilewidgettest.cpp
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/widgets/kdirmodel.cpp
  src/widgets/kdirmodel.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-29 Thread Nathaniel Graham 
ngraham added a comment.


  In D20838#457395 , @meven wrote:
  
  > I have fixed those two issues :
  >
  > - the path are now translated from kde url to mostlocal urls (dropping from 
desktop:/ works fine) (not from trash:/ though, but it currently does not work 
currently in the filename field either)
  > - the drop action moveAction needed to be allowed in the KDirModel for the 
drop from the folder view to work.
  
  
  Works perfectly!
  
  > - To test: what if the filewidget has a mime filter ?
  
  In this case, if the dropped file has a mimetype not permitted by the 
dialog's filter, it should be marked as an invalid drop.
  
  > - To test: what if the filewidget is in folder mode ?
  
  I would say that dropping a file should enter its parent path (and remove the 
filename part of the path). Dropping a folder should add the folder's path as 
normal

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-28 Thread Méven Car 
meven added a comment.


  In D20838#456847 , @ngraham wrote:
  
  > In D20838#456831 , @meven wrote:
  >
  > > I have tested on my side, I don't understand why it does not work.
  > >  From dolphin desktop:/ you can drag'n drop fine but not from the folder 
view.
  > >  I receive events and desktop:/ urls but the drag is not accepted 
whatever I do, like always calling event->accept() in the "case 
QEvent::DragEnter".
  > >  Could it be a because of the folder view filtering where it accepts to 
get dragged for instance ?
  >
  >
  > Sounds like it. If it works from `desktop:/` in Dolphin, but not from 
Folder view, I bet the drag isn't being sent the right way.
  >
  > However, testing with a file in `desktop:/`, the drag and drop happens 
successfully, but the path listed in the filename field is invalid (e.g. 
`desktop:IMG_0713.JPG` and the file can't actually be opened:
  >
  > F6795316: Screenshot_20190426_111318.png 

  >
  > Looks like the protocol is missing a trailing slash before the file path 
part. Interestingly, I notice that if I drag the same file from 
`desktop:/` the filename field, the path is listed as 
`file:///home/dev/Desktop/IMG_0713.JPG` which is different, but also correct. 
Perhaps the path just needs to be sanitized in the same way when dragged to the 
file view?
  
  
  I have fixed those two issues :
  
  - the path are now translated from kde url to mostlocal urls (dropping from 
desktop:/ works fine) (not from trash:/ though, but it currently does not work 
currently in the filename field either)
  - the drop action moveAction needed to be allowed in the KDirModel for the 
drop from the folder view to work.
  
  Todo :
  
  - add an automated test
  - check the KDirModel change is sane and does not introduce weird behavior
  - To test: what if the filewidget has a mime filter ?
  - To test: what if the filewidget is in folder mode ?
  
  Thoughts ?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-28 Thread Méven Car 
meven updated this revision to Diff 57124.
meven added a comment.


  Allow KDirModel to accept more actions when dropping on the model, simplify 
implementation, translates kde url to mostlocalurls when dropping

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57101=57124

BRANCH
  drop-kdiroperator

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/widgets/kdirmodel.cpp
  src/widgets/kdirmodel.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-27 Thread Méven Car 
meven updated this revision to Diff 57101.
meven added a comment.


  Have to keep a reference to the connection in the object, or it gets deleted 
before the closure is run

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57099=57101

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  tests/kfilewidgettest_gui.cpp
  tests/kfilewidgettest_saving_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-27 Thread Méven Car 
meven updated this revision to Diff 57099.
meven added a comment.


  Fix dropping directory

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57098=57099

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  tests/kfilewidgettest_gui.cpp
  tests/kfilewidgettest_saving_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-27 Thread Méven Car 
meven updated this revision to Diff 57098.
meven added a comment.


  Use mostLocalUrls when resolving the dropped url, bettor organize code

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57056=57098

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  tests/kfilewidgettest_gui.cpp
  tests/kfilewidgettest_saving_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Nathaniel Graham 
ngraham added a comment.


  In D20838#456831 , @meven wrote:
  
  > I have tested on my side, I don't understand why it does not work.
  >  From dolphin desktop:/ you can drag'n drop fine but not from the folder 
view.
  >  I receive events and desktop:/ urls but the drag is not accepted whatever 
I do, like always calling event->accept() in the "case QEvent::DragEnter".
  >  Could it be a because of the folder view filtering where it accepts to get 
dragged for instance ?
  
  
  Sounds like it. If it works from `desktop:/` in Dolphin, but not from Folder 
view, I bet the drag isn't being sent the right way.
  
  However, testing with a file in `desktop:/`, the drag and drop happens 
successfully, but the path listed in the filename field is invalid (e.g. 
`desktop:IMG_0713.JPG` and the file can't actually be opened:
  
  F6795316: Screenshot_20190426_111318.png 

  
  Looks like the protocol is missing a trailing slash before the file path 
part. Interestingly, I notice that if I drag the same file from 
`desktop:/` the filename field, the path is listed as 
`file:///home/dev/Desktop/IMG_0713.JPG` which is different, but also correct. 
Perhaps the path just needs to be sanitized in the same way when dragged to the 
file view?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Méven Car 
meven added a comment.


  In D20838#456820 , @ngraham wrote:
  
  > Wow, it works and it's so nice. What an improvement!
  
  
  :)
  
  > Drop options are not needed here; this isn't a move/copy/symlink UI; it's 
simply a quick way to choose a file that you happen to have visible in Dolphin 
or on the desktop.
  >  On that subject, I notice that drops from the desktop don't work. Not sure 
if that needs work here or in Folder View.
  
  I have tested on my side, I don't understand why it does not work.
  From dolphin desktop:/ you can drag'n drop fine but not from the folder view.
  I receive events and desktop:/ urls but the drag is not accepted whatever I 
do, like always calling event->accept() in the "case QEvent::DragEnter".
  Could it be a because of the folder view filtering where it accepts to get 
dragged for instance ?
  I am a bit stuck here. I am missing something.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Méven Car 
meven updated this revision to Diff 57056.
meven added a comment.


  Remove unnecessary comment

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57027=57056

BRANCH
  arcpatch-D20838

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Méven Car 
meven marked an inline comment as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Nathaniel Graham 
ngraham added a comment.


  Wow, it works and it's so nice. What an improvement!
  
  Drop options are not needed here; this isn't a move/copy/symlink UI; it's 
simply a quick way to choose a file that you happen to have visible in Dolphin 
or on the desktop. On that subject, I notice that drops from the desktop don't 
work. Not sure if that needs work here or in Folder View.

INLINE COMMENTS

> kdiroperator.cpp:1410
> +if (entry.isDir()) {
> +// if this was a directory
> +setUrl(url, false);

Comment seems unnecessary; the code is clear enough

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Méven Car 
meven added inline comments.

INLINE COMMENTS

> apol wrote in kdiroperator.cpp:418
> This default change should be documented.

Not sure I have done this correctly.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Méven Car 
meven updated this revision to Diff 57027.
meven added a comment.


  Add a since in documentation

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57026=57027

BRANCH
  drop-kdiroperator

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Méven Car 
meven marked an inline comment as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Méven Car 
meven marked 4 inline comments as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Méven Car 
meven updated this revision to Diff 57026.
meven added a comment.


  Review feedback

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57024=57026

BRANCH
  drop-kdiroperator

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Aleix Pol Gonzalez 
apol added inline comments.

INLINE COMMENTS

> kdiroperator.cpp:418
>  setFocusPolicy(Qt::WheelFocus);
> +setAcceptDrops(true);
>  }

This default change should be documented.

> kdiroperator.cpp:1401
> +} else {
> +QUrl url = urls.at(0);
> +

.constFirst() maybe reads better?

> kdiroperator.cpp:1419
> +QMetaObject::Connection connection;
> +auto fun = [this, url, connection](){
> +this->setCurrentItem(url);

maybe call the function urlSetter?

> kdiroperator.cpp:1523
> +if (acceptsDrops) {
> +this->view()->installEventFilter(this);
> +} else {

Initial this-> isn't necessary, just `view()`.

> kfilewidgettest_gui.cpp:34
>  
> +app.connect(fileWidget, ::destroyed, fileWidget, []() {
> +app.exit();

No need to lambda:
`QObject::connect(fileWidget, ::destroyed, , 
::exit);`

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Méven Car 
meven added a reviewer: ngraham.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20838

To: meven, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

2019-04-26 Thread Méven Car 
meven created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
meven requested review of this revision.

REVISION SUMMARY
  Open questions:
  
  - drop options
  
  Todo:
  
  - Add automated tests
  
  BUG: 45154
  FIXED-IN: 5.58

TEST PLAN
  Manual testing using kfilewidgettestgui.cpp and kfilewidgettest_saving_gui.cpp

REPOSITORY
  R241 KIO

BRANCH
  drop-kdiroperator

REVISION DETAIL
  https://phabricator.kde.org/D20838

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  tests/kfilewidgettest_gui.cpp

To: meven
Cc: kde-frameworks-devel, michaelh, ngraham, bruns