D17596: [KDirOperator] Allow renaming files from the context menu

2020-01-06 Thread Nathaniel Graham 
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:5ee3e7bb4797: [KDirOperator] Allow renaming files from 
the context menu (authored by ngraham).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17596?vs=72897&id=72933

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

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

To: ngraham, #frameworks, #dolphin, meven
Cc: meven, cfeck, emateli, elvisangelaccio, markuss, dhaumann, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2020-01-06 Thread Méven Car 
meven added a comment.


  D17595  just landed, just tested this 
patch

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, meven
Cc: meven, cfeck, emateli, elvisangelaccio, markuss, dhaumann, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2020-01-06 Thread Méven Car 
meven accepted this revision as: meven.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, meven
Cc: meven, cfeck, emateli, elvisangelaccio, markuss, dhaumann, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2020-01-06 Thread Nathaniel Graham 
ngraham marked 2 inline comments as done.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, meven
Cc: meven, cfeck, emateli, elvisangelaccio, markuss, dhaumann, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2020-01-06 Thread Nathaniel Graham 
ngraham updated this revision to Diff 72897.
ngraham marked an inline comment as done.
ngraham added a comment.


  Fix @since value

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17596?vs=72641&id=72897

BRANCH
  arcpatch-D17596

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

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

To: ngraham, #frameworks, #dolphin, meven
Cc: meven, cfeck, emateli, elvisangelaccio, markuss, dhaumann, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2020-01-06 Thread Nathaniel Graham 
ngraham added a comment.


  @since value needs to be updated.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, meven
Cc: meven, cfeck, emateli, elvisangelaccio, markuss, dhaumann, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2020-01-06 Thread Méven Car 
meven requested changes to this revision.
meven added a comment.
This revision now requires changes to proceed.


  Don't forget to amend commit message (I have update the FIXED-IN: value).

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, meven
Cc: meven, cfeck, emateli, elvisangelaccio, markuss, dhaumann, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2020-01-06 Thread Méven Car 
meven edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: meven, cfeck, emateli, elvisangelaccio, markuss, dhaumann, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2020-01-06 Thread Méven Car 
meven added inline comments.

INLINE COMMENTS

> kdiroperator.h:800
> + * prompting the user to choose a new name(s) for the currently selected 
> items
> + * @since 5.56
> + */

Update to @since 5.67

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: meven, cfeck, emateli, elvisangelaccio, markuss, dhaumann, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2020-01-02 Thread Nathaniel Graham 
ngraham updated this revision to Diff 72641.
ngraham added a comment.


  Update @since

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17596?vs=47624&id=72641

BRANCH
  arcpatch-D17596

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

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

To: ngraham, #frameworks, #dolphin
Cc: cfeck, emateli, elvisangelaccio, markuss, dhaumann, kde-frameworks-devel, 
LeGast00n, GB_2, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2019-01-10 Thread Nathaniel Graham 
ngraham edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: cfeck, emateli, elvisangelaccio, markuss, dhaumann, kde-frameworks-devel, 
michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2019-01-10 Thread Nathaniel Graham 
ngraham added a comment.


  Sorry, I lost track of this after large changes were requested for the 
dependent patch D17595 . I'll try to get 
this in for 5.54.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: cfeck, emateli, elvisangelaccio, markuss, dhaumann, kde-frameworks-devel, 
michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2019-01-08 Thread Christoph Feck 
cfeck added a comment.


  What is the status of this patch? It missed the 5.54 deadline, so the version 
would need to be adjusted.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: cfeck, emateli, elvisangelaccio, markuss, dhaumann, kde-frameworks-devel, 
michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-15 Thread Markus Slopianka 
markuss added a comment.


  In D17596#377668 , @ngraham wrote:
  
  > That's currently an option in Dolphin. Maybe we should just read that 
setting to see if we should do it here too?
  
  
  Sounds like a good idea.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: emateli, elvisangelaccio, markuss, dhaumann, kde-frameworks-devel, 
michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-15 Thread Nathaniel Graham 
ngraham added a comment.


  That's currently an option in Dolphin. Maybe we should just read that setting 
to see if we should do it here too?
  
  Either way I'd prefer to do that in a separate patch.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: emateli, elvisangelaccio, markuss, dhaumann, kde-frameworks-devel, 
michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-15 Thread Emirald Mateli 
emateli added a comment.


  IMO it would be great if this would just rename inline rather than opening a 
dialog.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: emateli, elvisangelaccio, markuss, dhaumann, kde-frameworks-devel, 
michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-15 Thread Elvis Angelaccio 
elvisangelaccio added inline comments.

INLINE COMMENTS

> dhaumann wrote in kdiroperator.cpp:907-909
> Does dialog->open() block? If so, do we have to new the dialog at all? (Only 
> reason I can think of is some DBus crash, see 
> https://blogs.kde.org/2009/03/26/how-crash-almost-every-qtkde-application-and-how-fix-it-0
>  but that would require a better fix).

Nope, `dialog->open()` does not block.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: elvisangelaccio, markuss, dhaumann, kde-frameworks-devel, michaelh, 
ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-15 Thread Dominik Haumann 
dhaumann added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in kdiroperator.cpp:907-909
> I think we should just use `dialog->open()` here. I see you've copied these 3 
> lines from Dolphin, but we should also fix it there (the rename dialog should 
> really be modal).

Does dialog->open() block? If so, do we have to new the dialog at all? (Only 
reason I can think of is some DBus crash, see 
https://blogs.kde.org/2009/03/26/how-crash-almost-every-qtkde-application-and-how-fix-it-0
 but that would require a better fix).

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: elvisangelaccio, markuss, dhaumann, kde-frameworks-devel, michaelh, 
ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-15 Thread Nathaniel Graham 
ngraham added a comment.


  In D17596#377423 , @markuss wrote:
  
  > Before the feature was removed, the window had all file operations, incl. 
move to trash and IIRC even the ability to cut/copy/paste and move (via D'n'D) 
files.
  
  
  Move to Trash is already there. Cut/Copy/Paste shouldn't be too hard to 
add--in another patch of course. :)

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: elvisangelaccio, markuss, dhaumann, kde-frameworks-devel, michaelh, 
ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-15 Thread Nathaniel Graham 
ngraham updated this revision to Diff 47624.
ngraham marked 2 inline comments as done.
ngraham added a comment.


  Address review comments

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17596?vs=47607&id=47624

BRANCH
  arcpatch-D17596

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

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

To: ngraham, #frameworks, #dolphin
Cc: elvisangelaccio, markuss, dhaumann, kde-frameworks-devel, michaelh, 
ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-15 Thread Elvis Angelaccio 
elvisangelaccio added inline comments.

INLINE COMMENTS

> kdiroperator.cpp:907-909
> +dialog->show();
> +dialog->raise();
> +dialog->activateWindow();

I think we should just use `dialog->open()` here. I see you've copied these 3 
lines from Dolphin, but we should also fix it there (the rename dialog should 
really be modal).

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: elvisangelaccio, markuss, dhaumann, kde-frameworks-devel, michaelh, 
ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-15 Thread Markus Slopianka 
markuss added a comment.


  Before the feature was removed, the window had all file operations, incl. 
move to trash and IIRC even the ability to cut/copy/paste and move (via D'n'D) 
files.
  
  What's that "ShowDeleteCommand", false? Can it be true and file deletion will 
be possible again as well?

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: markuss, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-15 Thread Dominik Haumann 
dhaumann added a comment.


  Not sure about binary compatibility...

INLINE COMMENTS

> kdiroperator.h:774
> + */
> +virtual void renameSelected();
> +

Adding a virtual function changes the vtable, so this is BIC (binary 
incompatible). Does the function need to be virtual?

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-14 Thread Nathaniel Graham 
ngraham created this revision.
ngraham added reviewers: Frameworks, Dolphin.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  BUG: 189482
  FIXED-IN: 5.54

TEST PLAN
  [image goes here]
  
  - Using the menu item works to rename one or more items
  - The menu item is disables when there's no selection
  - This menu item is inappropriately enabled for items that cannot be deleted, 
just like the trash/delete menu item (pre-existing bug, will fix in another 
patch)

REPOSITORY
  R241 KIO

BRANCH
  master

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

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

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

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-14 Thread Nathaniel Graham 
ngraham edited the summary of this revision.
ngraham edited the test plan for this revision.
ngraham added a dependency: D17595: Upstream Dolphin's file rename dialog.

REPOSITORY
  R241 KIO

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

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