[KDirOperator] Allow renaming files from the context menu
Needs ReviewPublic

Authored by ngraham on Dec 15 2018, 2:56 AM.

Details

Reviewers
None
Group Reviewers
Frameworks
Dolphin
Summary

BUG: 189482
FIXED-IN: 5.55

Depends on D17595

Test Plan

  • 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)

Diff Detail

Repository
R241 KIO
Branch
arcpatch-D17596
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6050
Build 6068: arc lint + arc unit
ngraham created this revision.Dec 15 2018, 2:56 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptDec 15 2018, 2:57 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
ngraham requested review of this revision.Dec 15 2018, 2:57 AM
ngraham edited the summary of this revision. (Show Details)Dec 15 2018, 2:57 AM
ngraham edited the test plan for this revision. (Show Details)
dhaumann added a subscriber: dhaumann.EditedDec 15 2018, 8:54 AM

Not sure about binary compatibility, since this is an exported class / public API.

src/filewidgets/kdiroperator.h
774

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

markuss added a subscriber: markuss.EditedDec 15 2018, 1:05 PM

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.

elvisangelaccio added inline comments.
src/filewidgets/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).

ngraham updated this revision to Diff 47624.Dec 15 2018, 4:32 PM
ngraham marked 2 inline comments as done.

Address review comments

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. :)

dhaumann added inline comments.Dec 15 2018, 5:00 PM
src/filewidgets/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).

src/filewidgets/kdiroperator.cpp
907–909

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

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

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.

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.

cfeck added a subscriber: cfeck.Jan 9 2019, 2:47 AM

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

ngraham added a comment.EditedJan 10 2019, 5:15 PM

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.55.

ngraham edited the summary of this revision. (Show Details)Jan 10 2019, 5:15 PM