D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2020-07-05 Thread Emirald Mateli
emateli added a comment. You're completely right in the sense that for a batch rename files will be in the same folder. However, rename really is just a move operation. The current BatchRenameJob has a hardcoded logic about placeholders and extensions which make it unusable outside of its

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2020-07-05 Thread Emirald Mateli
emateli added a comment. I honestly think the GUI is mostly ready, at least what I wanted to achieve with it: Something that is advanced enough but not over the top with options as there's already tools for that. Will check at some point to finish the items left from the review here. What

D27760: WIP | Added BatchMoveJob

2020-04-03 Thread Emirald Mateli
emateli added inline comments. INLINE COMMENTS > batchmovejobtest.cpp:89 > +KIO::Job *job = KIO::batchMove(items); > + > KIO::FileUndoManager::self()->recordJob(KIO::FileUndoManager::CommandType::BatchMove, > QList{}, QUrl(), job); > +job->setUiDelegate(nullptr);

D27760: WIP | Added BatchMoveJob

2020-03-01 Thread Emirald Mateli
emateli added a comment. In that it supports moving them to different directories but as well as moving them with a different name. This operation is to allow also batch renaming of items. See this patch for more details: D14631 eg: BatchMove

D27760: WIP | Added BatchMoveJob

2020-03-01 Thread Emirald Mateli
emateli retitled this revision from "Added BatchMoveJob" to "WIP | Added BatchMoveJob". emateli edited the summary of this revision. emateli added reviewers: dfaure, ngraham. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D27760 To: emateli, dfaure, ngraham Cc:

D27760: Added BatchMoveJob

2020-03-01 Thread Emirald Mateli
emateli created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. emateli requested review of this revision. REPOSITORY R241 KIO BRANCH batchmove REVISION DETAIL https://phabricator.kde.org/D27760 AFFECTED FILES autotests/CMakeLists.txt

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2020-02-28 Thread Emirald Mateli
emateli added a comment. Well, this patch is more or less complete but it doesn't make sense to be deployed without batch operations from KIO as it would spawn a job for each file to be renamed. Like I stated in my last comment, I tried to give adding batch renaming a go but it felt

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2019-09-14 Thread Emirald Mateli
emateli added a comment. I spent some time with modifying CopyJob but didn't make much progress and it's too deep into KIO internals - a lot one has to familiarize himself with. Maybe it's best someone who already did work on KIO to add the batch stuff. Once that is done and merged we can

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2019-07-21 Thread Emirald Mateli
emateli added a comment. In D14631#499038 , @dfaure wrote: > KIO::move (implemented by CopyJob) can move N files to a single destination directory, but they get the same name at that dest. > KIO::moveAs (implemented by CopyJob too) can

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2019-07-21 Thread Emirald Mateli
emateli added a comment. Before continuing work I'd like to hear some opinions on this. 1. From @dfaure or other people at frameworks: How do we handle batch rejame jobs. Should I create a new patch to submit a basic batch move job that simply moves files around without any additional

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2019-05-28 Thread Emirald Mateli
emateli added a comment. I'll need to rewrite the BatchRenameJob part which does the batch processing without breaking binary compatibility. Been unusually busy lately between job and uni but I'll start working on this sooner than later. REPOSITORY R241 KIO REVISION DETAIL

D18563: Don't create directory tree when a new folder has a '/' in the name

2019-01-30 Thread Emirald Mateli
emateli added a comment. Agree, something similar is being proposed over at D18384 . These dialogs are getting out of hand. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18563 To: shubham, ngraham, #frameworks, #dolphin, dfaure,

D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-30 Thread Emirald Mateli
emateli added a comment. I am of the opinion that such dialogs are not very user friendly and honestly very much against them. Not commenting on the usefulness of this particular warning, but dolphin has quite a few of these kind of dialogs with the same process: `Input something -> Get

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2019-01-15 Thread Emirald Mateli
emateli planned changes to this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-12-16 Thread Emirald Mateli
emateli added a subscriber: chinmoyr. emateli added a comment. Pinging @chinmoyr as the original author of the BatchRenameJob class. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: chinmoyr, mlaurent, asensi, rkflx,

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-12-16 Thread Emirald Mateli
emateli updated this revision to Diff 47694. emateli added a comment. - convert to namespace from class - do not show first captured group - remove help button - Use d-ptr pattern - remove show/hide methods and autoshow dialog - update doc - KFileItemList -> QList; KFileItem usage

D17617: Display error instead of silently failing when asked to create folder that already exists

2018-12-16 Thread Emirald Mateli
emateli added a comment. FWIW: The patch over at D11304 changes the behavior to select the currently existing folder instead of doing nothing. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17617 To: ngraham, #dolphin,

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,

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-12-14 Thread Emirald Mateli
emateli marked 3 inline comments as done. emateli added a comment. There's still some work to do with the actual BatchRenameClass now and the d-pointer pattern. I'll update this in a few days. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli,

D16694: Improve visibility for Konsole icon when using Breeze Dark

2018-12-10 Thread Emirald Mateli
emateli added a comment. @anishgiri What is the advantage of using this outlined version of the Konsole icon(which we haven't seen in a light scheme yet) compared to using one of the monochrome icons already available? REPOSITORY R266 Breeze Icons REVISION DETAIL

D16694: Improve visibility for Konsole icon when using Breeze Dark

2018-12-07 Thread Emirald Mateli
emateli added a comment. Agree with @ndavis with respect to the icon. I would be in favor of using something similar (if not identical) to `dialog-scripts`. It's monochrome and works with dark and light backgrounds. REPOSITORY R266 Breeze Icons REVISION DETAIL

D16905: Add new generic sorting icons and rename existing alphabetic sorting icons to -name scheme

2018-11-16 Thread Emirald Mateli
emateli added a comment. @trickyricky26 I think we should stick to whatever conventions the KDE community has. I checked with Dolphin and the arrow goes up for descending and up for ascending, however the VDG makes these calls in the end. REPOSITORY R266 Breeze Icons REVISION DETAIL

D16905: Add new generic sorting icons and rename existing alphabetic sorting icons to -name scheme

2018-11-16 Thread Emirald Mateli
emateli added a comment. 1. Aren't the names in reverse? `view-sort-ascending` shows a list in descending order 2. In `view-sort-descending` the items are not even sorted. Not sure how i feel about that. REPOSITORY R266 Breeze Icons BRANCH add-sort-options-icon (branched from master)

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-11-13 Thread Emirald Mateli
emateli updated this revision to Diff 45438. emateli marked 9 inline comments as done. emateli added a comment. - create on stack - upd define - remove unused import - use qstringliteral - remove unused export - match file name - remove import prefixes - remove q_unused on used

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-11-05 Thread Emirald Mateli
emateli updated this revision to Diff 44938. emateli added a comment. - Initialize pointers to nullptr - Code style fix --- Thoughts on proceeding with the proposed changes to batchrenamejob? Also, the filenameutils namespace feels like duplicated work. Any existing solution

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-11-02 Thread Emirald Mateli
emateli added a comment. Also, thoughts on my previous comments regarding `BatchRenameJob`? INLINE COMMENTS > mlaurent wrote in batchrenamedialog.h:69 > it's a good idea to initialize to nullptr each pointer here. > So we can see problem easily. You mean to initialize them to nullptr on the

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-11-02 Thread Emirald Mateli
emateli updated this revision to Diff 44737. emateli marked 4 inline comments as done. emateli added a comment. - coding style fix REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14631?vs=44218=44737 BRANCH batchrename2 REVISION DETAIL

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-10-25 Thread Emirald Mateli
emateli marked 2 inline comments as done. emateli added a comment. Something that might need a bit of input is `BatchRenameJob`. - This was added a while back so that Dolphin doesn't do a batch rename as a series of single file renames. - This job takes care of replacing the `#`

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-10-25 Thread Emirald Mateli
emateli updated this revision to Diff 44218. emateli marked 3 inline comments as done. emateli added a comment. - Remove QtCore/ REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14631?vs=44217=44218 BRANCH batchrename2 REVISION DETAIL

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-10-25 Thread Emirald Mateli
emateli updated this revision to Diff 44217. emateli added a comment. - fix indent - remove setlayout call - Add missing parent parameters - Remove unnecessary QtCore/ prefix REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14631?vs=44189=44217 BRANCH

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-10-24 Thread Emirald Mateli
emateli updated this revision to Diff 44189. emateli added a comment. - use _p notation. Add BatchRenameDialog to installable headers --- Besides any current review, the last step for this patch would be to actually use the BatchRenameJob already in KIO. Regarding that: Should I put

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-09-25 Thread Emirald Mateli
emateli added a comment. In D14631#303886 , @aacid wrote: > As far as i can see none of these headers get installed so should they all be renamed to _p.h ? > > Also if the headers don't get installed how do you use the new classes?

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-09-25 Thread Emirald Mateli
emateli updated this revision to Diff 42324. emateli marked 15 inline comments as done. emateli added a comment. - Code style fixes - use .insert instead of [] - Indent Q_OBJECT - use isEmpty instead of len==0 - use aggregate initialization - protected -> private - added parent

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2018-08-05 Thread Emirald Mateli
emateli retitled this revision from "Migrate D10698 from Dolphin to KIO" to "Adds a new RenameDialog to KIO with more options for batch renaming". emateli edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks

D14631: Migrate D10698 from Dolphin to KIO

2018-08-05 Thread Emirald Mateli
emateli added a reviewer: Frameworks. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14631: Migrate D10698 from Dolphin to KIO

2018-08-05 Thread Emirald Mateli
emateli created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. emateli requested review of this revision. REPOSITORY R241 KIO BRANCH batchrename2 REVISION DETAIL https://phabricator.kde.org/D14631

D10937: Retouching of Screen Layout Selection OSD Icons

2018-03-16 Thread Emirald Mateli
emateli added a comment. Hi, while I do appreciate the icons I also have a question: Why the use of laptop and projector over of two simple generic screens. Wouldn't that be better for all use cases? REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D10937 To:

D8920: Fixes url navigation with relative links on KUrlNavigator

2017-12-18 Thread Emirald Mateli
emateli abandoned this revision. emateli added a comment. Resolved in https://phabricator.kde.org/D9217 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8920 To: emateli, #frameworks, dfaure Cc: #frameworks

D9217: KUriFilter: don't return an error on non-existing files.

2017-12-06 Thread Emirald Mateli
emateli added a comment. I can't seem to be able to properly run this(whatever changes I make to the kshorturifilter.cpp seem to be ignored?!) but on paper looks good to me. Do we mark https://phabricator.kde.org/D8920 as abandoned, or wait for this to commit then the new `dir` property of

D8920: Fixes url navigation with relative links on KUrlNavigator

2017-12-05 Thread Emirald Mateli
emateli updated this revision to Diff 23532. emateli added a comment. - ~/ at the start always resolves to $HOME REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8920?vs=22702=23532 BRANCH relative-files-v2 REVISION DETAIL https://phabricator.kde.org/D8920

D9100: KUriFilter: simplify data structures, fix memory leak

2017-12-02 Thread Emirald Mateli
emateli added inline comments. INLINE COMMENTS > kurifilter.cpp:597 > +if (filters.isEmpty() || filters.contains(plugin->objectName())) { > +if (plugin->filterUri(data)) { > filtered = true; Perhaps this can be merged with the if statement above. REPOSITORY

D8920: Fixes url navigation with relative links on KUrlNavigator

2017-12-01 Thread Emirald Mateli
emateli added a comment. Ping. Thoughts on the last comment? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8920 To: emateli, #frameworks, dfaure Cc: #frameworks

D8920: Fixes url navigation with relative links on KUrlNavigator

2017-11-26 Thread Emirald Mateli
emateli added a comment. @dfaure I've managed to take another look at this, added a `directory` property which controls the base dir. Works as it should, however there is one case where I'm not sure which way we want it and is probably just a matter of preference. User types

D8920: Fixes url navigation with relative links on KUrlNavigator

2017-11-21 Thread Emirald Mateli
emateli updated this revision to Diff 22702. emateli added a comment. - remove QDir::homePath + minor code style REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8920?vs=22671=22702 BRANCH relative-files REVISION DETAIL https://phabricator.kde.org/D8920

D8920: Fixes url navigation with relative links on KUrlNavigator

2017-11-20 Thread Emirald Mateli
emateli added a reviewer: Frameworks. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8920 To: emateli, #frameworks Cc: #frameworks

D8920: Fixes url navigation with relative links on KUrlNavigator

2017-11-20 Thread Emirald Mateli
emateli created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY This patch aims to address an issue with KUrlNavigator using relative paths. Upon attempting to navigate using relative paths, due to the

D7828: fix createKMessageBox focus widget inconsistency

2017-11-02 Thread Emirald Mateli
emateli added a comment. Ping @subdiff @abetts does this iteration work for you guys? It's marked as ready to land but I feel that we should get an overall opinion on this. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D7828 To: emateli, #frameworks,

D7828: fix createKMessageBox focus widget inconsistency

2017-10-30 Thread Emirald Mateli
emateli retitled this revision from "fix createKMessageBox focus button inconsistency" to "fix createKMessageBox focus widget inconsistency". REVISION DETAIL https://phabricator.kde.org/D7828 To: emateli, #frameworks, ngraham, aacid, #vdg, rkflx Cc: elvisangelaccio, rkflx, abetts, subdiff,

D7828: fix createKMessageBox focus button inconsistency

2017-10-30 Thread Emirald Mateli
emateli retitled this revision from "createKMessageBox tries to focus a default button when available" to "fix createKMessageBox focus button inconsistency". emateli edited the summary of this revision. REVISION DETAIL https://phabricator.kde.org/D7828 To: emateli, #frameworks, ngraham,

D7828: createKMessageBox tries to focus a default button when available

2017-10-27 Thread Emirald Mateli
emateli updated this revision to Diff 21453. emateli added a comment. Well, the diff of this patch took a sizeable reduction. I took a second look at this and here is what happened in the end and what caused the inconsistency with Dolphin. The issue was that the `QDialogButtonBox`

D7828: createKMessageBox tries to focus a default button when available

2017-10-25 Thread Emirald Mateli
emateli added a comment. I think this is getting a bit out of hand here. Please try to read my last message where I explain why I decided to submit this patch and why I think it's a bug (the whole parent widget thing). Given the new information on why this occurs the whole focus by

D7828: createKMessageBox tries to focus a default button when available

2017-10-25 Thread Emirald Mateli
emateli added a comment. Hey @subdiff, thanks for your input on this. Whether this patch goes in or not, I still think that this "odd" behaviour is something that the frameworks should fix or change rather than relying on the developer of each application to do this. However I never

D7828: createKMessageBox tries to focus a default button when available

2017-10-24 Thread Emirald Mateli
emateli added a comment. @aacid the issue at hand is that in this particular scenario the default button more or less equals to the one that has focus. There are only 3 types of widgets that can be created here: buttons, a list or a check box. You can't interact with the list so it

D7828: createKMessageBox tries to focus a default button when available

2017-10-23 Thread Emirald Mateli
emateli added a comment. In my opinion a "Default" widget, should also have focus. Makes little sense to have some widget focused and then pressing enter to trigger the action of another one. By focusing, the default action is also made visible and allows to invoke the it action by

D7828: createKMessageBox tries to focus a default button when available

2017-10-08 Thread Emirald Mateli
emateli updated this revision to Diff 20497. emateli added a comment. Added autotests CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7828?vs=19543=20497 REVISION DETAIL https://phabricator.kde.org/D7828 AFFECTED FILES autotests/CMakeLists.txt

D7828: createKMessageBox tries to focus a default button when available

2017-10-03 Thread Emirald Mateli
emateli added a comment. @aacid just to make clear, you're saying to add another case to the `testMessageBox` on `kmessageboxtest.cpp` right? REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D7828 To: emateli, #frameworks Cc: aacid, #frameworks

D7828: createKMessageBox tries to focus a default button when available

2017-09-14 Thread Emirald Mateli
emateli edited the summary of this revision. emateli edited the test plan for this revision. emateli added a reviewer: Frameworks. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D7828 To: emateli, #frameworks Cc: #frameworks

D7828: createKMessageBox tries to focus a default button when available

2017-09-14 Thread Emirald Mateli
emateli created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REPOSITORY R236 KWidgetsAddons BRANCH createKMessageBox-btn-focus REVISION DETAIL https://phabricator.kde.org/D7828 AFFECTED FILES