D17595: Upstream Dolphin's file rename dialog

This revision was automatically updated to reflect the committed changes. Closed by commit R241:3f5341d3d7e2: Upstream Dolphins file rename dialog (authored by meven). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=72882=72922 REVISION DETAIL

D17595: Upstream Dolphin's file rename dialog

dfaure accepted this revision. dfaure added a comment. Last day before what? The world explodes soon? :) KF 5.66 is already tagged, as planned (first saturday of the month). REPOSITORY R241 KIO BRANCH arcpatch-D17595_2 REVISION DETAIL https://phabricator.kde.org/D17595 To:

D17595: Upstream Dolphin's file rename dialog

meven added a comment. last days for last minute review @dfaure REPOSITORY R241 KIO BRANCH arcpatch-D17595_2 REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham, dfaure, elvisangelaccio Cc: elvisangelaccio, lydia, dfaure, sitter,

D17595: Upstream Dolphin's file rename dialog

ngraham added a comment. This will be so nice to finally have done! REPOSITORY R241 KIO BRANCH arcpatch-D17595_2 REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham, dfaure, elvisangelaccio Cc: elvisangelaccio, lydia, dfaure, sitter,

D17595: Upstream Dolphin's file rename dialog

elvisangelaccio accepted this revision. REPOSITORY R241 KIO BRANCH arcpatch-D17595_2 REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham, dfaure, elvisangelaccio Cc: elvisangelaccio, lydia, dfaure, sitter, mitchell, emmanuelp, ltoscano,

D17595: Upstream Dolphin's file rename dialog

meven updated this revision to Diff 72882. meven added a comment. Update Copyright year REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=72881=72882 BRANCH arcpatch-D17595_2 REVISION DETAIL https://phabricator.kde.org/D17595 AFFECTED FILES

D17595: Upstream Dolphin's file rename dialog

meven updated this revision to Diff 72881. meven marked 2 inline comments as done. meven added a comment. Rebase, amend, update @since, fix typo in TODO KF6 REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=70224=72881 BRANCH arcpatch-D17595_2

D17595: Upstream Dolphin's file rename dialog

meven edited the summary of this revision. REPOSITORY R241 KIO BRANCH arcpatch-D17595_1 REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham, dfaure Cc: elvisangelaccio, lydia, dfaure, sitter, mitchell, emmanuelp, ltoscano, bruns, meven,

D17595: Upstream Dolphin's file rename dialog

elvisangelaccio added inline comments. INLINE COMMENTS > elvisangelaccio wrote in renamefiledialog.h:48 > So, what should we rename `RenameFileDialog` to? `RenameFileDialog` or > `RenameFileOverwrittenDialog` ? ;) > > Anyway, why wait for KF6 since this is new API? Can't we rename it now? Ah

D17595: Upstream Dolphin's file rename dialog

elvisangelaccio added inline comments. INLINE COMMENTS > renamefiledialog.h:48 > + */ > +// TODO KF6 : rename the class RenameFileDialog to RenameDialog and the > class RenameFileDialog to RenameFileOverwrittenDialog or similar. > +class KIOWIDGETS_EXPORT RenameFileDialog : public QDialog So,

D17595: Upstream Dolphin's file rename dialog

dfaure accepted this revision. dfaure added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > dfaure wrote in renamefiledialog.h:47 > @since 5.62 I hope this comment (between docu and class) doesn't break apidox generation. REPOSITORY R241 KIO BRANCH

D17595: Upstream Dolphin's file rename dialog

ngraham accepted this revision. ngraham added a comment. The @since values need to be 5.66 now. Works great though, and the code looks sane to me now. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham, dfaure Cc:

D17595: Upstream Dolphin's file rename dialog

meven added a comment. In D17595#580655 , @ngraham wrote: > Awesome. Does this need a rebase? It should not, the API has not changed and this is new code. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To:

D17595: Upstream Dolphin's file rename dialog

ngraham added a comment. Awesome. Does this need a rebase? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham, dfaure Cc: lydia, dfaure, sitter, mitchell, emmanuelp, ltoscano, bruns, meven, dhaumann, pino,

D17595: Upstream Dolphin's file rename dialog

meven added a comment. In D17595#569924 , @lydia wrote: > For posterity's sake: > > I confirm that we have confirmation to the board list from (hope I didn't overlook anyone): > > - Emmanuel Pescosta > - Roman Inflianskas > -

D17595: Upstream Dolphin's file rename dialog

lydia added a comment. For posterity's sake: I confirm that we have confirmation to the board list from (hope I didn't overlook anyone): - Emmanuel Pescosta - Roman Inflianskas - Alexander Richardson - David Faure - Matthias Fuchs - Artur Puzio - Kevin Funk - Aldo

D17595: Upstream Dolphin's file rename dialog

meven added a subscriber: lydia. meven added a comment. In D17595#566800 , @dhaumann wrote: > In D17595#542142 , @meven wrote: > > > I received the agreements of all contributors including the

D17595: Upstream Dolphin's file rename dialog

dhaumann added a comment. In D17595#542142 , @meven wrote: > I received the agreements of all contributors including the original code author, to relicense this code and hence upstream it. Playing the bad guy here: where are these

D17595: Upstream Dolphin's file rename dialog

meven updated this revision to Diff 70224. meven added a comment. Update copyright/licensing, update @since, add a TODO KF6 REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=63895=70224 BRANCH arcpatch-D17595_1 REVISION DETAIL

D17595: Upstream Dolphin's file rename dialog

ngraham added a comment. In D17595#542142 , @meven wrote: > I received the agreements of all contributors including the original code author, to relicense this code and hence upstream it. > It reminds of the importance of signing FLA to KDE

D17595: Upstream Dolphin's file rename dialog

meven added a comment. I received the agreements of all contributors including the original code author, to relicense this code and hence upstream it. It reminds of the importance of signing FLA to KDE e.V. So this is good to follow through the normal review process. @dfaure I

D17595: Upstream Dolphin's file rename dialog

meven marked 2 inline comments as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham, dfaure Cc: dfaure, sitter, mitchell, emmanuelp, ltoscano, bruns, meven, dhaumann, pino, kde-frameworks-devel, LeGast00n,

D17595: Upstream Dolphin's file rename dialog

meven marked an inline comment as done. meven added inline comments. INLINE COMMENTS > dfaure wrote in renamefiledialog.cpp:246 > I see you used the more complicated solution of keeping showEvent and testing > for spontaneous. Any reason? It didn't work from the constructor? After some testing

D17595: Upstream Dolphin's file rename dialog

meven updated this revision to Diff 63895. meven added a comment. Call setFocus() in dialog constructor REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=63892=63895 BRANCH arcpatch-D17595 REVISION DETAIL https://phabricator.kde.org/D17595

D17595: Upstream Dolphin's file rename dialog

meven updated this revision to Diff 63892. meven added a comment. Fix dialog where the private class was missing the renamed files items REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=63891=63892 BRANCH arcpatch-D17595 REVISION DETAIL

D17595: Upstream Dolphin's file rename dialog

meven marked 6 inline comments as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham, dfaure Cc: dfaure, sitter, mitchell, emmanuelp, ltoscano, bruns, meven, dhaumann, pino, kde-frameworks-devel, LeGast00n,

D17595: Upstream Dolphin's file rename dialog

meven updated this revision to Diff 63891. meven added a comment. Emit error correctly use name() function of KFileItem REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=63854=63891 BRANCH arcpatch-D17595 REVISION DETAIL

D17595: Upstream Dolphin's file rename dialog

bruns added inline comments. INLINE COMMENTS > dfaure wrote in renamefiledialog.cpp:119 > Oops yes I meant items.first().url().path(). OK for fileName(), path() is > faster but this code path isn't speed critical. anything wrong with `KFileItem::name()`? REPOSITORY R241 KIO REVISION DETAIL

D17595: Upstream Dolphin's file rename dialog

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > renamefiledialog.cpp:215 > +connect(job, ::result, this, ::deleteLater); > +connect(job, ::error, this, ::slotError); > + This can't possibly work,

D17595: Upstream Dolphin's file rename dialog

meven marked 4 inline comments as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham, dfaure Cc: dfaure, sitter, mitchell, emmanuelp, ltoscano, bruns, meven, dhaumann, pino, kde-frameworks-devel, LeGast00n,

D17595: Upstream Dolphin's file rename dialog

meven marked 4 inline comments as done. meven added a comment. Btw the licensing issue is not resolved yet. I will update the license once this has cleared. INLINE COMMENTS > dfaure wrote in renamefiledialog.cpp:119 > items.first().path() would be simpler and faster. > (that doesn't give

D17595: Upstream Dolphin's file rename dialog

meven updated this revision to Diff 63854. meven marked 14 inline comments as done. meven added a comment. uncrustify file, typo fixes, add error slot REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=63796=63854 BRANCH arcpatch-D17595 REVISION

D17595: Upstream Dolphin's file rename dialog

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > renamefiledialog.cpp:67 > +{ > +const QSize minSize = minimumSize(); > +setMinimumSize(QSize(320, minSize.height())); This returns an invalid size

D17595: Upstream Dolphin's file rename dialog

meven marked an inline comment as done. meven added inline comments. INLINE COMMENTS > dfaure wrote in renamefiledialog.cpp:60 > Just run `uncrustify-kf5` from the repo kde-dev-scripts. I am very happy to learn about this one. This script cannot work with a single file it seems and most of the

D17595: Upstream Dolphin's file rename dialog

meven updated this revision to Diff 63796. meven added a comment. Group boolean fields together REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=62026=63796 BRANCH arcpatch-D17595 REVISION DETAIL https://phabricator.kde.org/D17595 AFFECTED FILES

D17595: Upstream Dolphin's file rename dialog

dfaure added inline comments. INLINE COMMENTS > renamefiledialog.cpp:59 > +KFileItemList items; > +bool allExtensionsDifferent; > +QSpinBox* spinBox; group the bools together for less padding > meven wrote in renamefiledialog.cpp:60 > I really wish we would enforce use of a code

D17595: Upstream Dolphin's file rename dialog

meven marked 4 inline comments as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham Cc: sitter, mitchell, emmanuelp, ltoscano, bruns, meven, dhaumann, pino, kde-frameworks-devel, LeGast00n, sbergeron, michaelh,

D17595: Upstream Dolphin's file rename dialog

meven marked 4 inline comments as done. meven added inline comments. INLINE COMMENTS > sitter wrote in renamefiledialog.h:72 > You'll also want to override `event` as per > https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B Thanks This is really arcane, I am really not

D17595: Upstream Dolphin's file rename dialog

sitter added inline comments. INLINE COMMENTS > renamefiledialog.h:72 > +protected: > +void showEvent(QShowEvent* event) override; > + You'll also want to override `event` as per https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B > renamefiledialog.h:75 >

D17595: Upstream Dolphin's file rename dialog

meven updated this revision to Diff 62026. meven added a comment. Add context about filename # REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=62005=62026 BRANCH arcpatch-D17595 REVISION DETAIL https://phabricator.kde.org/D17595 AFFECTED FILES

D17595: Upstream Dolphin's file rename dialog

meven updated this revision to Diff 62005. meven added a comment. Fix email in copyright declaration REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=59292=62005 BRANCH arcpatch-D17595 REVISION DETAIL https://phabricator.kde.org/D17595 AFFECTED

D17595: Upstream Dolphin's file rename dialog

meven marked 3 inline comments as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham Cc: mitchell, emmanuelp, ltoscano, bruns, meven, dhaumann, pino, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham

D17595: Upstream Dolphin's file rename dialog

meven added a subscriber: mitchell. meven added a comment. In D17595#481526 , @ngraham wrote: > Who's left? Only three now : Jeff Mitchell @mitchell , Frank Reininghaus and Peter Penz. But Peter Penz is the most important one as

D17595: Upstream Dolphin's file rename dialog

ngraham added a comment. Who's left? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham Cc: emmanuelp, ltoscano, bruns, meven, dhaumann, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham

D17595: Upstream Dolphin's file rename dialog

meven planned changes to this revision. meven added a comment. Still need 4 agreements from copyrights holders to make the necessary license change. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham Cc: emmanuelp,

D17595: Upstream Dolphin's file rename dialog

meven added a comment. In D17595#478067 , @emmanuelp wrote: > In D17595#473825 , @meven wrote: > > > I guess we need now to reach to schwarz...@gmail.com romaninflianskas mfuchs chinmoyr mitchell

D17595: Upstream Dolphin's file rename dialog

emmanuelp added a comment. In D17595#473825 , @meven wrote: > I guess we need now to reach to schwarz...@gmail.com romaninflianskas mfuchs chinmoyr mitchell emateli emmanuelp freininghaus to have lgplv2+ approval. Added myself to the

D17595: Upstream Dolphin's file rename dialog

meven added inline comments. INLINE COMMENTS > bruns wrote in renamefiledialog.cpp:105 > The code assumes the "#" is the very last character ("Don't select the # > character"), so this requires a translation comment. Is that better line 105 ? REPOSITORY R241 KIO REVISION DETAIL

D17595: Upstream Dolphin's file rename dialog

meven updated this revision to Diff 59292. meven marked an inline comment as done. meven added a comment. Improve localization context REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=59255=59292 BRANCH arcpatch-D17595_1 REVISION DETAIL

D17595: Upstream Dolphin's file rename dialog

bruns added inline comments. INLINE COMMENTS > renamefiledialog.cpp:105 > +} else { > +newName = i18nc("@info:status", "New name #"); > +editLabel = new QLabel(i18ncp("@label:textbox", The code assumes the "#" is the very last character ("Don't select the # character"), so

D17595: Upstream Dolphin's file rename dialog

meven marked 7 inline comments as done. meven added inline comments. INLINE COMMENTS > bruns wrote in renamefiledialog.cpp:126 > better name - `m_extensionsAreUnique` The current name seems fine to me. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: meven,

D17595: Upstream Dolphin's file rename dialog

meven updated this revision to Diff 59255. meven added a comment. Add a Private class to RenameFileDialog, code fixes REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17595?vs=47606=59255 BRANCH arcpatch-D17595 REVISION DETAIL

D17595: Upstream Dolphin's file rename dialog

meven commandeered this revision. meven added a reviewer: ngraham. meven added inline comments. INLINE COMMENTS > pino wrote in renamefiledialog.cpp:50-51 > why a minimum size is enforced? For files whose name is short, I think the dialog would not look nice. This look fine in dolphin. Was

D17595: Upstream Dolphin's file rename dialog

ngraham added a comment. Thanks so much! \o/ REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: ngraham, #frameworks, #dolphin, broulik Cc: ltoscano, bruns, meven, dhaumann, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham

D17595: Upstream Dolphin's file rename dialog

meven added a comment. In D17595#473826 , @ngraham wrote: > Would you be interested in doing that and taking over this patch? I'm afraid I don't think I have the time right now for the kind of deep dive that it requires. If not, that's okay. :)

D17595: Upstream Dolphin's file rename dialog

ngraham added a comment. Would you be interested in doing that and taking over this patch? I'm afraid I don't think I have the time right now for the kind of deep dive that it requires. If not, that's okay. :) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To:

D17595: Upstream Dolphin's file rename dialog

meven added a comment. In dolphin repo relicensecheck.pl views/renamedialog.h The following emails do not appear in the accounts file: schwarz...@gmail.com Need permission for licensing: - gplv2+: schwarz...@gmail.com mfuchs romaninflianskas mitchell

D17595: Upstream Dolphin's file rename dialog

meven added inline comments. INLINE COMMENTS > ltoscano wrote in renamefiledialog.h:5 > We do, but people sign it on a vuluntary basis: > > https://ev.kde.org/rules/fla.php > > Also, there is this which pre-dates the FLA but it is useful too (and still > in use): > >

D17595: Upstream Dolphin's file rename dialog

dhaumann added a comment. Yes, please ask Peter Penz and CC a public mailing list and/or use the relicense check script. I am sure Peter would agree to a license change. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: ngraham, #frameworks, #dolphin, broulik

D17595: Upstream Dolphin's file rename dialog

ltoscano added inline comments. INLINE COMMENTS > meven wrote in renamefiledialog.h:5 > We can email Peter Penz to ask about doing a license change. > > I wish we would ask its contributors to allow the kde foundation to update > code distribution license on their behalf, provided the license

D17595: Upstream Dolphin's file rename dialog

bruns added inline comments. INLINE COMMENTS > renamefiledialog.cpp:60 > + i18nc("@title:window", "Rename Items")); > +QDialogButtonBox *buttonBox = new > QDialogButtonBox(QDialogButtonBox::Ok|QDialogButtonBox::Cancel); > +QVBoxLayout *mainLayout = new QVBoxLayout; space

D17595: Upstream Dolphin's file rename dialog

meven added inline comments. INLINE COMMENTS > pino wrote in renamefiledialog.h:5 > > The KIO repo's COPYING.LIB file says LGPL 2.1. Is that a mismatch? > > Considering this new code is GPL: yes, it's a mismatch, and it would make the > whole kiowidgets as GPL. We can email Peter Penz to ask

D17595: Upstream Dolphin's file rename dialog

pino added inline comments. INLINE COMMENTS > ngraham wrote in renamefiledialog.h:5 > The KIO repo's COPYING.LIB file says LGPL 2.1. Is that a mismatch? > The KIO repo's COPYING.LIB file says LGPL 2.1. Is that a mismatch? Considering this new code is GPL: yes, it's a mismatch, and it would

D17595: Upstream Dolphin's file rename dialog

ngraham added inline comments. INLINE COMMENTS > dhaumann wrote in renamefiledialog.h:5 > Is the rest of KIO LGPL or GPL? If we have a license mismatch, we'd have to > ask the authors for an OK to relicense. The KIO repo's COPYING.LIB file says LGPL 2.1. Is that a mismatch? REPOSITORY R241

D17595: Upstream Dolphin's file rename dialog

dhaumann added inline comments. INLINE COMMENTS > renamefiledialog.h:5 > + * This program is free software; you can redistribute it and/or modify * > + * it under the terms of the GNU General Public License as published by * > + * the Free Software Foundation; either version 2 of the

D17595: Upstream Dolphin's file rename dialog

pino added inline comments. INLINE COMMENTS > renamefiledialog.cpp:50-51 > +{ > +const QSize minSize = minimumSize(); > +setMinimumSize(QSize(320, minSize.height())); > + why a minimum size is enforced? > renamefiledialog.cpp:54 > +const int itemCount = items.count(); > +

D17595: Upstream Dolphin's file rename dialog

ngraham added a dependent revision: D17597: Use newly-upstreamed rename dialog from KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: ngraham, #frameworks, #dolphin, broulik Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17595: Upstream Dolphin's file rename dialog

ngraham added a dependent revision: D17596: [KDirOperator] Allow renaming files from the context menu. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: ngraham, #frameworks, #dolphin, broulik Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17595: Upstream Dolphin's file rename dialog

ngraham created this revision. ngraham added reviewers: Frameworks, Dolphin, broulik. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ngraham requested review of this revision. REVISION SUMMARY KIO didn't actually have its own rename dialog, which is