D29525: Make Previews devicePixelRatio aware

2020-12-28 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  Superseded by https://invent.kde.org/system/dolphin/-/merge_requests/147
  
  @meven Can you abandon this one?

REPOSITORY
  R318 Dolphin

BRANCH
  arcpatch-D29525

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

To: meven, #dolphin, #frameworks, dfaure, ngraham, elvisangelaccio
Cc: kfm-devel, badbunny, waitquietly, azyx, nikolaik, pberestov, iasensio, 
aprcela, fprice, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, 
navarromorales, firef, ngraham, andrebarros, emmanuelp, rdieter, mikesomov


D23694: Add support for sshfs to the fstab backend

2020-10-23 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D23694#676619 , 
@elvisangelaccio wrote:
  
  > In D23694#676610 , @lbeltrame 
wrote:
  >
  > > @elvisangelaccio Thanks, first of all. If that's the case, do you think 
it's better to touch KIO first before landing this?
  >
  >
  > Yeah this should probably wait for the KIO patch, which I just submitted: 
https://invent.kde.org/frameworks/kio/-/merge_requests/175
  
  
  The KIO patch has been merged.
  
  +1 for merging this one as well.

REPOSITORY
  R245 Solid

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

To: lbeltrame, bruns, broulik, fvogt, #kde_connect
Cc: elvisangelaccio, albertvaka, ngraham, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, bruns


D23694: Add support for sshfs to the fstab backend

2020-10-18 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D23694#676610 , @lbeltrame 
wrote:
  
  > @elvisangelaccio Thanks, first of all. If that's the case, do you think 
it's better to touch KIO first before landing this?
  
  
  Yeah this should probably wait for the KIO patch, which I just submitted: 
https://invent.kde.org/frameworks/kio/-/merge_requests/175

REPOSITORY
  R245 Solid

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

To: lbeltrame, bruns, broulik, fvogt, #kde_connect
Cc: elvisangelaccio, albertvaka, ngraham, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, bruns


D23694: Add support for sshfs to the fstab backend

2020-10-15 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D23694#676607 , @lbeltrame 
wrote:
  
  > Would this change be needed on this side or kdeconnect side? I don't mind 
landing this, but I need formal approval by someone. (I won't have any time to 
do additional changes elsewhere).
  
  
  The change I was talking about would be in KIO (in the places model). I can 
do that.
  
  In D23694#676608 , @bruns wrote:
  
  > One possibility to hide it would be the `x-gvfs-*` options, though I am not 
sure if it is possible to pass these to fuse.
  
  
  Doesn't seem to work :(
  
sshfs -o x-gvfs-hide nuc:/data/ /home/elvis/DATA/
fuse: unknown option(s): `-o x-gvfs-hide'

sshfs -o comments=x-gvfs-hide nuc:/data/ /home/elvis/DATA/
fuse: unknown option(s): `-o comments=x-gvfs-hide'

REPOSITORY
  R245 Solid

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

To: lbeltrame, bruns, broulik, fvogt, #kde_connect
Cc: elvisangelaccio, albertvaka, ngraham, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, bruns


D23694: Add support for sshfs to the fstab backend

2020-10-14 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  Ping? Can we get this in?
  
  In D23694#525220 , @albertvaka 
wrote:
  
  > If it's a problem for kdeconnect mounts to appear there, how can we hide 
it? It's an sshfs mountpoint like any other, only that it is mounted 
programmatically.
  
  
  We should be able to easily hide it from KIO, because we know that the mount 
point of the place URL will be `kdeconnect@X.X.X.X`

REPOSITORY
  R245 Solid

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

To: lbeltrame, bruns, broulik, fvogt, #kde_connect
Cc: elvisangelaccio, albertvaka, ngraham, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, bruns


D21695: Add FindTaglib.cmake

2020-06-21 Thread Elvis Angelaccio
elvisangelaccio commandeered this revision.
elvisangelaccio added a reviewer: heikobecker.
elvisangelaccio added a comment.


  In D21695#675056 , 
@elvisangelaccio wrote:
  
  > New attempt at 
https://invent.kde.org/frameworks/extra-cmake-modules/-/merge_requests/6
  
  
  Merged, closing.

REPOSITORY
  R240 Extra CMake Modules

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

To: elvisangelaccio, kde-buildsystem, kde-frameworks-devel, dfaure, heikobecker
Cc: asturmlechner, ngraham, elvisangelaccio, cgiboudeaux, dfaure, LeGast00n, 
cblack, bencreasy, michaelh, bruns


D21695: Add FindTaglib.cmake

2020-06-21 Thread Elvis Angelaccio
elvisangelaccio abandoned this revision.

REPOSITORY
  R240 Extra CMake Modules

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

To: elvisangelaccio, kde-buildsystem, kde-frameworks-devel, dfaure, heikobecker
Cc: asturmlechner, ngraham, elvisangelaccio, cgiboudeaux, dfaure, LeGast00n, 
cblack, bencreasy, michaelh, bruns


D21695: Add FindTaglib.cmake

2020-06-11 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  New attempt at 
https://invent.kde.org/frameworks/extra-cmake-modules/-/merge_requests/6

REPOSITORY
  R240 Extra CMake Modules

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

To: heikobecker, kde-buildsystem, kde-frameworks-devel, dfaure
Cc: asturmlechner, ngraham, elvisangelaccio, cgiboudeaux, dfaure, LeGast00n, 
cblack, bencreasy, michaelh, bruns


D29525: Make Previews devicePixelRatio aware

2020-06-07 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.

REPOSITORY
  R318 Dolphin

BRANCH
  arcpatch-D29525

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

To: meven, #dolphin, #frameworks, dfaure, ngraham, elvisangelaccio
Cc: kfm-devel, waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, 
fprice, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, 
navarromorales, firef, ngraham, andrebarros, emmanuelp, rdieter, mikesomov


D29397: KPreviewJob : Support for DeviceRatioPixel

2020-05-23 Thread Elvis Angelaccio
elvisangelaccio added inline comments.

INLINE COMMENTS

> previewjob.cpp:587
> +if 
> (thumb.textKeys().contains(QStringLiteral("Thumb::DevicePixelRatio"))) {
> +qreal dpr = 
> thumb.text(QStringLiteral("Thumb::DevicePixelRatio")).toDouble();
> +thumb.setDevicePixelRatio(dpr);

Missing `const`

> previewjob.h:192
> +/**
> + * Request preview to use the device pixel ratio @p.
> + * The returned thumbnail may not respect the device pixel ratio 
> returned.

Did you mean `@p dpr`?

(unless doxygen is smart enough to figure out the name of the parameter?)

> previewjob.h:268
> + */
> +static void setDefaultDevicePixelRatio(qreal devicePixerRatio);
>  };

Typo: `devicePixelRatio`.

REPOSITORY
  R241 KIO

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

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: elvisangelaccio, kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns


D29397: KPreviewJob : Support for DeviceRatioPixel

2020-05-17 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  Would it make sense to initialize `devicePixelRatio` to 
`QGuiApplication::devicePixelRatio()` and add an API to change it (if desired) ?
  
  This way we wouldn't need to call a static method in the `main` of every 
client app (i.e. D29525  wouldn't be 
needed).

REPOSITORY
  R241 KIO

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

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: elvisangelaccio, kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns


D29197: filenamesearch: Implement stat to display metainfo

2020-05-03 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

To: meven, #dolphin, #frameworks, ngraham, elvisangelaccio
Cc: kde-frameworks-devel, kfm-devel, azyx, nikolaik, pberestov, iasensio, 
aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, rdieter, mikesomov


D29198: filenamesearch:/ define a title for the query

2020-04-26 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
elvisangelaccio added a subscriber: iasensio.
elvisangelaccio added a comment.
This revision is now accepted and ready to land.


  LGTM but I'd like input from @iasensio too.

INLINE COMMENTS

> dolphinsearchbox.h:164
>  private:
> +QString getQueryTitle(const QString& text) const;
> +

Please drop the `get` prefix.

REPOSITORY
  R318 Dolphin

BRANCH
  arcpatch-D29198

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

To: meven, ngraham, elvisangelaccio, #dolphin, #frameworks
Cc: iasensio, kfm-devel, azyx, nikolaik, pberestov, aprcela, fprice, 
fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, 
firef, ngraham, andrebarros, emmanuelp, rdieter, mikesomov


D7563: Add privilegeExecution field to file protocol description

2020-04-20 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D7563#650117 , @ngraham wrote:
  
  > [insert I-have-no-idea-what-I'm-doing dog meme here]
  >
  > When trying to create items in root-owned locations, I'm getting an errors 
saying "The process for the file protocol died unexpectedly." or else Dolphin 
simply crashes with a totally unhelpful backtrace.
  
  
  If it's still happening, please have a look at 
https://community.kde.org/Guidelines_and_HOWTOs/Debugging/Debugging_IOSlaves

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, dfaure, chinmoyr
Cc: kde-frameworks-devel, feverfew, mreeves, mati865, ngraham, elvisangelaccio, 
LeGast00n, cblack, michaelh, bruns


D28802: Add standard shortcut for "Show/Hide Hidden Files"

2020-04-20 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  Does it make sense to also create a standard action in kconfigwidgets?

REPOSITORY
  R237 KConfig

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

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


D28802: Add standard shortcut for "Show/Hide Hidden Files"

2020-04-20 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  > and F8 seems kind of random.
  
  I was asking myself why F8 was chosen and yes, it looks like it was totally 
random: 
https://github.com/KDE/kdelibs/commit/8e26f7410a9280b504d97cdfbe5d3568cfa7b9d4

REPOSITORY
  R237 KConfig

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

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


D28535: [KIO-MTP] Fix null pointer dereference

2020-04-12 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D28535#642298 , @feverfew wrote:
  
  > In D28535#642289 , 
@elvisangelaccio wrote:
  >
  > > In D28535#640833 , @fvogt 
wrote:
  > >
  > > > I assume there is a reason why `MTPDevice::getDevice()` has code for 
handling this very specific case, so I wouldn't just remove it without figuring 
out why: https://i.redd.it/hfnl7xo8yovy.gif
  > > >
  > > > If not, that would indeed be the best option.
  > >
  > >
  > > Unfortunately git blame doesn't seem to help us here.
  > >
  > > I suggest to push this fix to master only and see what happens.
  >
  >
  > By pushing this to master would we still be able to throw it up to 20.04.* 
if we decide it's stable enough? (also need to know to know what to put down as 
the FIXED-IN in the commit message)?
  
  
  Sure, we can cherry-pick that commit later.

REPOSITORY
  R320 KIO Extras

BRANCH
  fixNullPtr (branched from master)

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

To: feverfew, akrutzler, dfaure, elvisangelaccio
Cc: anthonyfieroni, kde-frameworks-devel, fvogt, kfm-devel, ngraham, azyx, 
nikolaik, pberestov, iasensio, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov


D7563: Add privilegeExecution field to file protocol description

2020-04-12 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D7563#645172 , @ngraham wrote:
  
  > The outstanding security issues have been resolved (see T8075 
). We have requested a re-review from the 
SUSE security team, but have not received it yet. Given that the original 
schedule for this feature has already slipped by almost two years, I would like 
to propose that we land this patch and turn it on, and resolve any 
newly-discovered issues in follow-up work.
  >
  > Any objections?
  
  
  +1

BRANCH
  master

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

To: chinmoyr, #frameworks, dfaure
Cc: feverfew, mreeves, mati865, ngraham, elvisangelaccio


D28535: [KIO-MTP] Fix null pointer dereference

2020-04-05 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
elvisangelaccio added a comment.
This revision is now accepted and ready to land.


  In D28535#640833 , @fvogt wrote:
  
  > I assume there is a reason why `MTPDevice::getDevice()` has code for 
handling this very specific case, so I wouldn't just remove it without figuring 
out why: https://i.redd.it/hfnl7xo8yovy.gif
  >
  > If not, that would indeed be the best option.
  
  
  Unfortunately git blame doesn't seem to help us here.
  
  I suggest to push this fix to master only and see what happens.

REPOSITORY
  R320 KIO Extras

BRANCH
  fixNullPtr (branched from master)

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

To: feverfew, akrutzler, dfaure, elvisangelaccio
Cc: anthonyfieroni, kde-frameworks-devel, fvogt, kfm-devel, ngraham, nikolaik, 
pberestov, iasensio, fprice, LeGast00n, cblack, fbampaloukas, alexde, GB_2, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov


D28520: Fix lifetime of slot in KIO-MTP

2020-04-05 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
elvisangelaccio added a comment.


  Please push to 20.04, there is still time.

REPOSITORY
  R320 KIO Extras

BRANCH
  slotLifetime (branched from master)

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

To: feverfew, akrutzler, dfaure, elvisangelaccio, apol, meven
Cc: apol, kde-frameworks-devel, kfm-devel, fvogt, nikolaik, pberestov, 
iasensio, fprice, LeGast00n, cblack, fbampaloukas, alexde, GB_2, Codezela, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov


D27951: Allow users to change dropAction to MoveAction through kdeglobals

2020-03-15 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  > With dndToMove=true in kdeglobals, drag files will move them without 
showing the menu. (holding Shift shows it)
  
  Are we going to expose this setting in Plasma?

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns


D28060: Fix exitcode from kioexec when executable doesn't exist (and --tempfiles is set)

2020-03-15 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  +1

REPOSITORY
  R241 KIO

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

To: dfaure, apol, davidedmundson, elvisangelaccio
Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns


D27591: KAbstractFileItemActionPlugin: Add missing quotes in code example

2020-03-14 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: alex, elvisangelaccio, #frameworks, mlaurent, apol
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D26067: [KFilePlacesView] Add missing functionality required in order to be used by Dolphin again

2020-01-19 Thread Elvis Angelaccio
elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  Haven't tried the dolphin branch yet, just a comment on the API for now.
  From a quick look the patch looks good though.

INLINE COMMENTS

> kfileplacesview.h:68
> + */
> +bool showAll() const;
> +

This seems to be the name of a method that makes all items visible, not the 
name of a bool getter.

I know we already have `setShowAll()`, but we can deprecate it and find a 
better name. How about `setAllPlacesVisible()` / `allPlacesVisible()` / 
`allPlacesVisibleChanged()` ?

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, #vdg, #dolphin, elvisangelaccio
Cc: elvisangelaccio, meven, ngraham, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns


D26635: GSCreator: Fix hang due to calling exit() after fork()

2020-01-19 Thread Elvis Angelaccio
This revision was automatically updated to reflect the committed changes.
Closed by commit R373:3e2ea6e924d0: GSCreator: Fix hang due to calling exit() 
after fork() (authored by abizjak, committed by elvisangelaccio).

REPOSITORY
  R373 Image Thumbnailers

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26635?vs=73441=73898

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

AFFECTED FILES
  ps/gscreator.cpp

To: abizjak, #frameworks, elvisangelaccio
Cc: elvisangelaccio


D26635: GSCreator: Fix hang due to calling exit() after fork()

2020-01-19 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
elvisangelaccio added a comment.
This revision is now accepted and ready to land.


  Looks safe enough to me.
  
  Can you provide an email address for the git authorship? Thanks!

REPOSITORY
  R373 Image Thumbnailers

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

To: abizjak, #frameworks, elvisangelaccio
Cc: elvisangelaccio


Re: Updating our coding conventions and coding style for C++11

2020-01-19 Thread Elvis Angelaccio
Hi,

On 16/01/20 18:46, Kai Uwe Broulik wrote:
> Hi,
> 
> for "auto" I think we should always annotate it with const, *, and/or &
> where appropriate:
> 
> auto *something = new MyCustomType;
> auto *keyEvent = static_cast(event);

IMHO the * here is redundant and only adds noise. It's clear that
`something` is a MyCustomType* and that `keyEvent` is a QKeyEvent*.

> auto  = foo[bar];

This could be helpful indeed.

> Cheers
> Kai Uwe

Cheers,
Elvis


D26484: Popup menu again to reposition it

2020-01-11 Thread Elvis Angelaccio
elvisangelaccio added inline comments.

INLINE COMMENTS

> dropjob.cpp:415
> +// popup again to update position. BUG: 415917
> +menu->popup(window ? window->mapToGlobal(m_relativePos) : 
> QCursor::pos());
>  }

`QCursor::pos()` should be avoided as it's not reliable on wayland.

The whole "popup() again" here looks like a workaround to me. Can't this be 
fixed on the plasma side?

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns


D17595: Upstream Dolphin's file rename dialog

2020-01-06 Thread Elvis Angelaccio
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, 
bruns, meven, dhaumann, pino, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham


D26277: KFilePlaceEditDialog: move logic into isIconEditable()

2020-01-06 Thread Elvis Angelaccio
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:bdc2df9c1735: KFilePlaceEditDialog: move logic into 
isIconEditable() (authored by elvisangelaccio).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26277?vs=72341=72883

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

AFFECTED FILES
  src/filewidgets/kfileplaceeditdialog.cpp

To: elvisangelaccio, #frameworks, ngraham
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D26448: Add KRecentFilesMenu to replace KRecentFileAction

2020-01-06 Thread Elvis Angelaccio
elvisangelaccio added inline comments.

INLINE COMMENTS

> krecentfilesmenu.cpp:31
> +QSettings *m_settings;
> +size_t m_maximumItems = 10;
> +};

Why not `int` since it's what we expose in the API anyway?

> krecentfilesmenu.cpp:53
> +setIcon(QIcon::fromTheme(QStringLiteral("document-open-recent")));
> +const QString fileName = 
> QStandardPaths::writableLocation(QStandardPaths::AppDataLocation) + 
> QLatin1String("/") + QCoreApplication::applicationName() + 
> QLatin1String("staterc");
> +d->m_settings = new QSettings(fileName, QSettings::Format::IniFormat, 
> this);

`QStringLiteral("...").arg()` would be more readable imho.

> krecentfilesmenu.h:32
> + *
> + * Replaces KRecentFilesAction from KConfigWidgets.
> + *

This is a porting guide, not sure it belongs in the main apidox of the class. 
(users of this new API may not know/care about KRecentFilesAction).

REPOSITORY
  R236 KWidgetsAddons

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

To: nicolasfella, #frameworks, dfaure
Cc: elvisangelaccio, cfeck, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns


D26358: KIO/SMB convert kio protocol declaration to json format

2020-01-06 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  +1

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, ngraham
Cc: elvisangelaccio, apol, kde-frameworks-devel, kfm-devel, pberestov, 
iasensio, fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D17595: Upstream Dolphin's file rename dialog

2020-01-06 Thread Elvis Angelaccio
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 sorry, I've read now the commit message. Which means there is a typo in this 
TODO:

"... and the class RenameDialog to RenameFileOverwrittenDialog or similar."

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, dhaumann, pino, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham


D17595: Upstream Dolphin's file rename dialog

2020-01-06 Thread Elvis Angelaccio
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, 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?

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, dhaumann, pino, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham


D26277: KFilePlaceEditDialog: move logic into isIconEditable()

2020-01-02 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  Ping?

REPOSITORY
  R241 KIO

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

To: elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D26277: KFilePlaceEditDialog: move logic into isIconEditable()

2019-12-29 Thread Elvis Angelaccio
elvisangelaccio created this revision.
elvisangelaccio added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
elvisangelaccio requested review of this revision.

REVISION SUMMARY
  Instead of relying on whether `m_iconButton` has been initialized
  somewhere else in the code.

TEST PLAN
  Make sure that editing icons is still allowed only for non-Trash places.

REPOSITORY
  R241 KIO

BRANCH
  isIconEditable (branched from master)

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

AFFECTED FILES
  src/filewidgets/kfileplaceeditdialog.cpp

To: elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D26276: KFilePlaceEditDialog: fix crash when editing the Trash place

2019-12-29 Thread Elvis Angelaccio
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:a9ea007cf87c: KFilePlaceEditDialog: fix crash when 
editing the Trash place (authored by elvisangelaccio).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26276?vs=72339=72340

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

AFFECTED FILES
  src/filewidgets/kfileplaceeditdialog.cpp

To: elvisangelaccio, #frameworks, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns


D26276: KFilePlaceEditDialog: fix crash when editing the Trash place

2019-12-29 Thread Elvis Angelaccio
elvisangelaccio created this revision.
elvisangelaccio added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
elvisangelaccio requested review of this revision.

REVISION SUMMARY
  `m_iconButton` is initialized only when the url scheme of the place being
  edited is not `trash`.
  
  BUG: 415676
  FIXED-IN: 5.66

TEST PLAN
  Right-click the Trash place in dolphin and change the name.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/filewidgets/kfileplaceeditdialog.cpp

To: elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D21695: Add FindTaglib.cmake

2019-12-29 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  @heikobecker are you still interested in this patch? I can take over 
otherwise.

REPOSITORY
  R240 Extra CMake Modules

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

To: heikobecker, kde-buildsystem, kde-frameworks-devel, dfaure
Cc: ngraham, elvisangelaccio, cgiboudeaux, dfaure, LeGast00n, GB_2, bencreasy, 
michaelh, bruns


D26273: cmake: don't use taglib-config if we are cross compiling

2019-12-29 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D26273#584234 , @vkrause wrote:
  
  > Longer term we probably either want to convince upstream to install cmake 
config files, or at least have this in ECM, a quick local grep find a handful 
of copies of this.
  
  
  There was an attempt with D21695  but it 
got stuck.

REPOSITORY
  R286 KFileMetaData

BRANCH
  bshah/cross

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

To: bshah, vkrause
Cc: elvisangelaccio, pino, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, 
LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams


D26197: Display fully qualified class/namespace name as page header

2019-12-24 Thread Elvis Angelaccio
elvisangelaccio added a reviewer: ochurlaud.

REPOSITORY
  R264 KApiDox

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

To: davidre, ochurlaud
Cc: ngraham, aacid, jucato, kde-frameworks-devel, kde-doc-english, LeGast00n, 
gennad, fbampaloukas, GB_2, michaelh, bruns, skadinna


D25683: KDirOperator: Use a fixed line height for scroll speed

2019-12-08 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.

REPOSITORY
  R241 KIO

BRANCH
  diroperator_scrollspeed

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

To: ahiemstra, ngraham, elvisangelaccio
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns


D25720: Fix shortcut conflict between Cut and Delete File

2019-12-08 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D25720#572291 , @ngraham wrote:
  
  > Though Dolphin seems to have some kind of local hack to make Shift+delete 
work for file deletion. But it doesn't work for Cut. It's all pretty messy.
  
  
  Yes it's messy but it's Microsoft fault since they chose to use an already 
used shortcut for another thing and we are doomed.
  
  Some years ago I submitted the very same patch and it eventually led to 
8eabbf6725386e716b7536c71a9181dfe5d959f0 
 in 
kxmlgui, which automatically handles the conflict for those few apps that need 
both actions (dolphin, gwenview, etc.)
  
  (Too bad reviewboard was shutdown, I had to dig in my mail to find it :/)
  
  So imho #414799 should be closed as intentional.

REPOSITORY
  R237 KConfig

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

To: ngraham, #frameworks, cfeck
Cc: elvisangelaccio, aspotashev, bcooksley, davidedmundson, aacid, apol, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D25527: [activities/fileitemaction] Port plugin metadata to JSON

2019-11-25 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R320 KIO Extras

BRANCH
  json

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

To: nicolasfella, #frameworks, elvisangelaccio
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov


Re: Bulk replacement of projects.kde.org on Frameworks modules

2019-11-11 Thread Elvis Angelaccio



On 11/11/19 22:52, Luigi Toscano wrote:
> Hi,
> basically all Frameworks components reference the ECM website
> using the old projects.kde.org URL, which is long gone and
> it is just a (partial) redirect.
> 
> See for example:
> 
> set_package_properties(ECM PROPERTIES TYPE REQUIRED
> DESCRIPTION "Extra CMake Modules."
> URL "https://projects.kde.org/projects/kdesupport/extra-cmake-modules;)
> 
> Can I go around a bulk-replace all the URLs with
> https://commits.kde.org/extra-cmake-modules, so that it would look like:
> 
> set_package_properties(ECM PROPERTIES TYPE REQUIRED
> DESCRIPTION "Extra CMake Modules."
> URL "https://commits.kde.org/extra-cmake-modules;)

+1

Using commits.kde.org like this feels weird though (i.e. using it
without a commit hash).

Something like https://code.kde.org/extra-cmake-modules would look much
better imho.

Just my 2c, maybe something that could be done after the move to gitlab.

> 
> There are few additional URLs which use the old website and I would replace
> them as well using the same pattern.
> 
> I'm asking to avoid ~70 almost-identical review requests.
> 
> Ciao
> 

Ciao
Elvis


D24686: Replace usage of deprecated SlaveBase::config() by SlaveBase::configValue

2019-10-20 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D24686#550533 , @dfaure wrote:
  
  > I never know when it's OK to increase the KF5 version dependency from 
applications modules, though.
  >
  > I have a ton of pending patches (to port away from KWindowSystem deprecated 
API) which require a KF5 version dep upgrade too...
  
  
  It's always ok to bump the KF5 version as long as you do it before the 
dependency freeze day set by the release team.

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

To: meven, dfaure
Cc: elvisangelaccio, kde-frameworks-devel, kfm-devel, iasensio, fprice, 
LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, 
michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, mikesomov


D24773: kio_trash: Add size, modification, access and create date for trash:/

2019-10-20 Thread Elvis Angelaccio
elvisangelaccio added a reviewer: dfaure.

REPOSITORY
  R241 KIO

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

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


D24773: kio_trash: Add size, modification, access and create date for trash:/

2019-10-20 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  +1

INLINE COMMENTS

> trashimpl.cpp:1091
> +KIO::UDSEntry entry;
> +entry.clear();
> +

Is this really needed? We just created `entry`.

> trashimpl.cpp:1094
> +// refresh list of trashes and get the list of files in them
> +const TrashedFileInfoList fileInfoList = list();
> +

coding style: local variables never start with an uppercase.

REPOSITORY
  R241 KIO

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

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


D7446: [Places panel] Revamp the Recently Saved section

2019-10-02 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D7446

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, andreaska, gregormi, markg, alexeymin, broulik, elvisangelaccio, 
dfaure, davidedmundson, ltoscano, #konqueror, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, michaelh, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D21897: Address some issues reported by Krazy analysis

2019-09-21 Thread Elvis Angelaccio
elvisangelaccio added inline comments.

INLINE COMMENTS

> pajamapants3000 wrote in ktextedit.cpp:373
> Thank you broulik! Could we use a `QScopedPointer` here? I see them 
> throughout the KDE source, but not often for dialogs. The krazy analysis 
> looked for `QPointer`, referencing blog https://blogs.kde.org/node/3919, 
> which appears to have predated the existence of `QScopedPointer`.

While we are at it, the best way to fix this would be to not use `exec()` at 
all, but use `open()` instead: 
https://wiki.qt.io/New_Signal_Slot_Syntax#Asynchronous_made_easier

REPOSITORY
  R310 KTextWidgets

BRANCH
  krazy-50b2564 (branched from master)

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

To: pajamapants3000, elvisangelaccio
Cc: broulik, vkrause, elvisangelaccio, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, ngraham, bruns


D23961: Add apidox to most of KFilePlacesModel

2019-09-15 Thread Elvis Angelaccio
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:bcf51ae68193: Add apidox to most of KFilePlacesModel 
(authored by elvisangelaccio).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23961?vs=66124=66149

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

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.h

To: elvisangelaccio, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D23955: KBookmark: improve addBookmark apidox

2019-09-15 Thread Elvis Angelaccio
This revision was automatically updated to reflect the committed changes.
Closed by commit R294:797763d4b4d2: KBookmark: improve addBookmark apidox 
(authored by elvisangelaccio).

REPOSITORY
  R294 KBookmarks

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23955?vs=66108=66148

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

AFFECTED FILES
  src/kbookmark.h

To: elvisangelaccio, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D23961: Add apidox to most of KFilePlacesModel

2019-09-15 Thread Elvis Angelaccio
elvisangelaccio updated this revision to Diff 66124.
elvisangelaccio added a comment.


  Mention that URLs will be stored in the QUrl::FullyEncoded string format.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23961?vs=66121=66124

BRANCH
  master

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

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.h

To: elvisangelaccio, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D23961: Add apidox to most of KFilePlacesModel

2019-09-15 Thread Elvis Angelaccio
elvisangelaccio updated this revision to Diff 66121.
elvisangelaccio added a comment.


  - Fixed typo

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23961?vs=66118=66121

BRANCH
  master

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

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.h

To: elvisangelaccio, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D23961: Add apidox to most of KFilePlacesModel

2019-09-15 Thread Elvis Angelaccio
elvisangelaccio created this revision.
elvisangelaccio added a reviewer: dfaure.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
elvisangelaccio requested review of this revision.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.h

To: elvisangelaccio, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D23956: KFilePlacesModel: fix @since tags

2019-09-15 Thread Elvis Angelaccio
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:90c5872c08de: KFilePlacesModel: fix @since tags (authored 
by elvisangelaccio).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23956?vs=66109=66111

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

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.h

To: elvisangelaccio, dfaure, aacid
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D23956: KFilePlacesModel: fix @since tags

2019-09-15 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  Yeah, that's my next patch ;)

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: elvisangelaccio, dfaure, aacid
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D23956: KFilePlacesModel: fix @since tags

2019-09-15 Thread Elvis Angelaccio
elvisangelaccio created this revision.
elvisangelaccio added reviewers: dfaure, aacid.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
elvisangelaccio requested review of this revision.

REVISION SUMMARY
  A bunch of @since tags were missing or wrong.
  While at it, use the same ///< style for enum apidox comments.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.h

To: elvisangelaccio, dfaure, aacid
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D23955: KBookmark: improve addBookmark apidox

2019-09-15 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D23955#531647 , @dfaure wrote:
  
  > OK (why does this matter to the user of the class, though?)
  
  
  See D23706 , dolphin is going to need 
regex-based matches on the placesmodel URLs.

REPOSITORY
  R294 KBookmarks

BRANCH
  master

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

To: elvisangelaccio, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D23955: KBookmark: improve addBookmark apidox

2019-09-15 Thread Elvis Angelaccio
elvisangelaccio created this revision.
elvisangelaccio added a reviewer: dfaure.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
elvisangelaccio requested review of this revision.

REVISION SUMMARY
  Clarify that the URL will be stored in its toString(QUrl::FullyEncoded)
  version.

REPOSITORY
  R294 KBookmarks

BRANCH
  master

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

AFFECTED FILES
  src/kbookmark.h

To: elvisangelaccio, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D23851: Avoid sending KDirNotify::emitFilesAdded when the emptytrashjob finishes

2019-09-13 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  +1
  
  In D23851#529463 , @ngraham wrote:
  
  > If the on;y thing that `EmptyTrashJob::slotFinished()` does now is call 
`SimpleJob::slotFinished()`, do we even still need to override the base class's 
implementation of `slotFinished()`?
  
  
  Yeah we don't, but we can't remove it because it would be a BIC.

REPOSITORY
  R241 KIO

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

To: meven, broulik, ngraham
Cc: elvisangelaccio, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns


D21795: [KAuth] Add support for action details in Polkit1 backend.

2019-09-13 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  polkit-qt-1-0.113.0 has been released, so I believe now we can move over with 
this patch.

REPOSITORY
  R283 KAuth

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

To: chinmoyr, apol, bruns, davidedmundson, #frameworks, dfaure, cfeck, sitter
Cc: elvisangelaccio, bcooksley, ngraham, sitter, mreeves, kde-frameworks-devel, 
LeGast00n, GB_2, michaelh, bruns


D7446: [Places panel] Revamp the Recently Saved section

2019-09-13 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  LGTM besides the inline nitpicks.

INLINE COMMENTS

> kfileplacesmodeltest.cpp:686
>  KBookmark device = root.first(); // The device we'll move is the 6th 
> bookmark
> -for (int i = 0; i < 5; i++) {
> +int stop = m_hasRecentlyUsedKio ? 7 : 5;
> +for (int i = 0; i < stop; i++) {

`const`, and maybe a more descriptive name (`count` or similar?)

> kfileplacesmodeltest.cpp:967
>  // places
> -QTest::newRow("Places - Home") << m_places->index(0, 0)
> +int idx = 0;
> +QTest::newRow("Places - Home") << m_places->index(idx++, 0)

I'd avoid cryptic names. If we can't use `index`, maybe just `i`?

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, andreaska, gregormi, markg, alexeymin, broulik, elvisangelaccio, 
dfaure, davidedmundson, ltoscano, #konqueror, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, michaelh, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D23898: [KUrlNavigatorButton] Fix QString usage to not use [] out of bounds.

2019-09-12 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
elvisangelaccio added a comment.


  Everything looks good and the new test passes for me.

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: dfaure, elvisangelaccio
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D23898: [KUrlNavigatorButton] Fix QString usage to not use [] out of bounds.

2019-09-12 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
elvisangelaccio added a comment.
This revision is now accepted and ready to land.


  I don't have 5.14 around atm, so I trust you've tested this :)

INLINE COMMENTS

> kurlnavigatortest.cpp:265
> +KFilePlacesModel model;
> +const QString pwd = QDir::currentPath();
> +const QUrl url = QUrl::fromLocalFile(pwd);

Did you mean `cwd` ?

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: dfaure, elvisangelaccio
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D21897: Address some issues reported by Krazy analysis

2019-08-25 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
elvisangelaccio added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> vkrause wrote in krichtextwidget.cpp:638
> This is unusual, but it's the right way around. The point of this is to check 
> if the dialog has been deleted during the sub-eventloop behind exec(). No 
> idea if that can happen here though, but that's generally the idea behind 
> this pattern.

Ah of course. I didn't notice it was `exec()` that was being called.

REPOSITORY
  R310 KTextWidgets

BRANCH
  krazy-50b2564 (branched from master)

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

To: pajamapants3000, elvisangelaccio
Cc: vkrause, elvisangelaccio, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns


D21897: Address some issues reported by Krazy analysis

2019-08-25 Thread Elvis Angelaccio
elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> krichtextwidget.cpp:638
>  
> -if (linkDialog->exec()) {
> +if (linkDialog->exec() && linkDialog) {
>  q->updateLink(linkDialog->linkUrl(), linkDialog->linkText());

This should be `linkDialog && linkDialog->exec()`

> ktextedit.cpp:369
>  }
> -if (dialog.exec()) {
> -setSpellCheckingLanguage(dialog.language());
> +if (dialog->exec() && dialog) {
> +setSpellCheckingLanguage(dialog->language());

Same here, we need to check the pointer before using it.

REPOSITORY
  R310 KTextWidgets

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

To: pajamapants3000, elvisangelaccio
Cc: elvisangelaccio, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns


D23201: [KDirOperator] Middle-elide labels that are too long to fit

2019-08-24 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  middle-elide-kdiroperator-labels (branched from master)

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

To: ngraham, #vdg, elvisangelaccio
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D22265: KPluginMetaData: use Q_DECLARE_METATYPE

2019-07-17 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D22265#491249 , @kossebau wrote:
  
  > This runs the chance to break some 3rd-party software which also calls the 
Q_DECLARE_METATYPE(KPluginMetaData) macro.
  >  Any chance this could be moved to the place that needs this for now, and 
perhaps add a KF6 TODO instead?
  >
  > See e.g. R36:3aacbbaab50ac6e0557c5d69c430459eb3d71ad7 
 
which was needed to unbreak ark builds.
  
  
  Ping?

REPOSITORY
  R244 KCoreAddons

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

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


D21782: Add a warning dialog with details, continue, and cancel button

2019-07-14 Thread Elvis Angelaccio
elvisangelaccio added inline comments.

INLINE COMMENTS

> kmessagebox.h:329
> + *
> + * @since 5.60
> + */

Needs a bump to 5.61

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  master

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

To: chinmoyr, #vdg, #frameworks, dfaure, ngraham, apol
Cc: elvisangelaccio, mreeves, ngraham, GB_2, kde-frameworks-devel, LeGast00n, 
sbergeron, michaelh, bruns


D22144: - Add kio recentlyused:/ to access KactivitytStats data

2019-07-01 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  +1

REPOSITORY
  R320 KIO Extras

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

To: meven, ivan, #frameworks, ngraham
Cc: elvisangelaccio, kde-frameworks-devel, kfm-devel, fprice, LeGast00n, 
fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, navarromorales, 
firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D22144: - Add kio recentlyused:/ to access KactivitytStats data

2019-06-30 Thread Elvis Angelaccio
elvisangelaccio added inline comments.

INLINE COMMENTS

> recentlyused.protocol:1-14
> +[Protocol]
> +X-DocPath=kioslave5/recentlyused/index.html
> +exec=kf5/kio/recentlyused
> +protocol=recentlyused
> +Icon=document-open-recent
> +input=none
> +output=filesystem

We should use the JSON format for new ioslaves.

You can use the slaves in KIO as example.

REPOSITORY
  R320 KIO Extras

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

To: meven, ivan, #frameworks, ngraham
Cc: elvisangelaccio, kde-frameworks-devel, kfm-devel, fprice, LeGast00n, 
fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, navarromorales, 
firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D15739: [Places panel] Don't show Root by default

2019-06-16 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D15739

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, 
bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham


D21695: Add FindTaglib.cmake

2019-06-10 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  Where does this FindTaglib.cmake come from?
  
  We should probably use the one from kio-extras, which got a bunch of fixes in 
the last months.

REPOSITORY
  R240 Extra CMake Modules

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

To: heikobecker, kde-buildsystem, kde-frameworks-devel, dfaure
Cc: elvisangelaccio, cgiboudeaux, dfaure, LeGast00n, bencreasy, michaelh, 
ngraham, bruns


D21695: Add FindTaglib.cmake

2019-06-10 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D21695#476826 , @heikobecker 
wrote:
  
  > I'm not entirely sure about taglib-config on Windows
  
  
  See 
https://cgit.kde.org/kio-extras.git/commit/cmake/FindTaglib.cmake?id=548f525f4308810888c85f42a570139029c40618

REPOSITORY
  R240 Extra CMake Modules

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

To: heikobecker, kde-buildsystem, kde-frameworks-devel, dfaure
Cc: elvisangelaccio, cgiboudeaux, dfaure, LeGast00n, bencreasy, michaelh, 
ngraham, bruns


D21367: kioexec: change the scary debug messages for delayed deletion

2019-05-27 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.

REPOSITORY
  R241 KIO

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

To: Lekensteyn, elvisangelaccio, dfaure
Cc: ngraham, kde-frameworks-devel, michaelh, bruns


D21367: kioexec: change the scary debug messages for delayed deletion

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

INLINE COMMENTS

> main.cpp:262
>  // it will have time to start up and read the file before it 
> gets deleted. #130709.
> -qDebug() << "sleeping...";
> -QThread::currentThread()->sleep(180); // 3 mn
> +const int sleep_secs = 180;
> +qDebug() << "sleeping for" << sleep_secs << "seconds before 
> deleting file...";

Please use camelCase

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

To: Lekensteyn, elvisangelaccio, dfaure
Cc: ngraham, kde-frameworks-devel, michaelh, bruns


D21367: kioexec: change the scary debug messages for delayed deletion

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

INLINE COMMENTS

> main.cpp:261-263
>  // it will have time to start up and read the file before it 
> gets deleted. #130709.
> -qDebug() << "sleeping...";
> +qDebug() << "sleeping for 3 minutes before deleting file...";
>  QThread::currentThread()->sleep(180); // 3 mn

While at it, maybe it could be worth to put 180 in some variable `foo` and then 
use `foo/60` instead of hardcoding the number of minutes in the debug message.

REPOSITORY
  R241 KIO

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

To: Lekensteyn, elvisangelaccio, dfaure
Cc: ngraham, kde-frameworks-devel, michaelh, bruns


D21249: Test current filter before setting a new one

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

INLINE COMMENTS

> kfilewidget.cpp:132
>  void updateLocationEditExtension(const QString &);
> +bool matchFilter(const QString , const QString , const 
> bool bUpdate);
>  void updateFilter();

Nitpick: I'd make `filter` the first parameter. And we usually don't use `const 
boll` in signatures, but just `bool`.

`bUpdate` doesn't tell the reader what this variable is used for. How about 
calling it `updateCurrentFilter` or similar?

REPOSITORY
  R241 KIO

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

To: jglogowski, #frameworks, ngraham
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, 
bruns


D21249: Test current filter before setting a new one

2019-05-19 Thread Elvis Angelaccio
elvisangelaccio added a reviewer: dfaure.

REPOSITORY
  R241 KIO

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

To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, 
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


D20964: [FileWidget] Replace "Filter:" with "File type:" when saving with a limited list of mimetypes

2019-05-05 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
elvisangelaccio added a comment.
This revision is now accepted and ready to land.


  Please wait the tagging before pushing.

REPOSITORY
  R241 KIO

BRANCH
  file-type-when-saving-and-mimetype-is-defined (branched from master)

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

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


D20827: Reword some text, a couple clean ups, add a separator

2019-05-01 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D20827#45 , @meven wrote:
  
  > In D20827#457391 , 
@elvisangelaccio wrote:
  >
  > > @meven FYI this change broke the Frameworks string freeze: 
https://community.kde.org/Schedules/Frameworks
  >
  >
  > Should I revert this then ?
  
  
  Someone could have already started to translate these new strings. Just keep 
it in mind for the next time ;)

REPOSITORY
  R241 KIO

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

To: meven, ngraham, #frameworks, #vdg, GB_2
Cc: elvisangelaccio, kde-frameworks-devel, cblack, arvidhansson, ian, jguidon, 
hannahk, Pixel_Lime, jraleigh, squeakypancakes, alexde, IohannesPetros, GB_2, 
trickyricky26, mglb, michaelh, crozbo, ndavis, firef, ngraham, bruns, skadinna, 
aaronhoneycutt, mbohlender


D20938: Add Mounts Backend

2019-05-01 Thread Elvis Angelaccio
elvisangelaccio added reviewers: broulik, bruns.

REPOSITORY
  R245 Solid

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

To: hallas, #frameworks, ngraham, elvisangelaccio, broulik, bruns
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D20934: Rename file dialog "Filter" label text to "Type"

2019-05-01 Thread Elvis Angelaccio
elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  "Filter" is the correct word, you can enter any string in the label (not 
necessarily a file type) and the view will be filtered accordingly.

REPOSITORY
  R241 KIO

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

To: GB_2, #frameworks, #plasma, #vdg, elvisangelaccio
Cc: elvisangelaccio, #vdg, #plasma, kde-frameworks-devel, #frameworks, 
michaelh, ngraham, bruns


D20827: Reword some text, a couple clean ups, add a separator

2019-04-28 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  @meven FYI this change broke the Frameworks string freeze: 
https://community.kde.org/Schedules/Frameworks

REPOSITORY
  R241 KIO

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

To: meven, ngraham, #frameworks, #vdg, GB_2
Cc: elvisangelaccio, kde-frameworks-devel, cblack, arvidhansson, ian, jguidon, 
hannahk, Pixel_Lime, jraleigh, squeakypancakes, alexde, IohannesPetros, GB_2, 
trickyricky26, mglb, michaelh, crozbo, ndavis, firef, ngraham, bruns, skadinna, 
aaronhoneycutt, mbohlender


D20525: [InformationPanel] Use the new inline configuration mode

2019-04-22 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R318 Dolphin

BRANCH
  cleanup

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

To: bruns, #dolphin, #baloo, #frameworks, ngraham, astippich, #vdg, 
elvisangelaccio
Cc: elvisangelaccio, meven, kfm-devel, alexde, feverfew, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, mikesomov


D20668: [InformationPanel] Remove obsolete intermediate wrapper widget/layoutThe Baloo::MetaDataWidget is now able to stretch in a meaningful wayby itself, remove the wrapper.

2019-04-20 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.

REPOSITORY
  R318 Dolphin

BRANCH
  cleanup

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

To: bruns, #dolphin, #baloo, #frameworks, ngraham, astippich, elvisangelaccio
Cc: kfm-devel, alexde, feverfew, meven, spoorun, navarromorales, firef, 
andrebarros, emmanuelp, mikesomov


D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

2019-04-20 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.

REPOSITORY
  R824 Baloo Widgets

BRANCH
  metadata_config_inline

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

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich, elvisangelaccio
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams


D20525: [InformationPanel] Use the new inline configuration mode

2019-04-20 Thread Elvis Angelaccio
elvisangelaccio added inline comments.

INLINE COMMENTS

> informationpanelcontent.cpp:113-114
> +m_configureLabel = new QLabel(i18nc("@label::textbox",
> +  "Select which data should "
> +  "be shown:"), this);
> +m_configureLabel->setWordWrap(true);

Why split the i18n sentence into two lines? It looks weird imho.

> informationpanelcontent.cpp:118
> +
> +m_configureButtons = new 
> QDialogButtonBox(QDialogButtonBox::Save|QDialogButtonBox::Cancel);
> +m_configureButtons->setVisible(false);

Coding style: missing space before/after `|`

> informationpanelcontent.cpp:122-124
> +m_configureButtons->setVisible(false);
> +m_configureLabel->setVisible(false);
> +emit configurationFinished();

These 3 lines could go in a dedicated function which we could call from both 
lambdas, to avoid code duplication. Or it could even be a private slot 
connected to `QDialogButtonBox::clicked`.

REPOSITORY
  R318 Dolphin

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

To: bruns, #dolphin, #baloo, #frameworks, ngraham, astippich, #vdg, 
elvisangelaccio
Cc: elvisangelaccio, meven, kfm-devel, alexde, feverfew, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, mikesomov


D20691: Fix no member named 'setTime_t' in 'QDateTime' error while building

2019-04-20 Thread Elvis Angelaccio
elvisangelaccio resigned from this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R245 Solid

BRANCH
  error

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

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


D20691: Fix no member named 'setTime_t' in 'QDateTime' error while building

2019-04-20 Thread Elvis Angelaccio
elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  Please revert the mode change to `Messages.sh`

REPOSITORY
  R245 Solid

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

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


D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

2019-04-14 Thread Elvis Angelaccio
elvisangelaccio added inline comments.

INLINE COMMENTS

> filemetadatawidget.h:93
> + * Switch between regular (view) and configuration mode.
> + * @since 5.58
> + */

Should be `@since 19.08.0` (baloo-widgets follows the KDE Applications 
versioning)

REPOSITORY
  R824 Baloo Widgets

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

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams


D20525: [InformationPanel] Use the new inline configuration mode

2019-04-14 Thread Elvis Angelaccio
elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  Please bump the minimum required version for `KF5BalooWidgets` to 19.07.70.

INLINE COMMENTS

> informationpanelcontent.cpp:121
> +connect(m_configureButtons, ::accepted, this, [this]() {
> +
> this->m_metaDataWidget->setConfigurationMode(Baloo::ConfigurationMode::Accept);
> +m_configureButtons->setVisible(false);

Why `this->` only on this line? (I'd just remove it as it's unnecessary)

> informationpanelcontent.cpp:127
> +connect(m_configureButtons, ::rejected, this, [this]() {
> +
> this->m_metaDataWidget->setConfigurationMode(Baloo::ConfigurationMode::Cancel);
> +m_configureButtons->setVisible(false);

Same here

> informationpanelcontent.cpp:237
>  
> +void InformationPanelContent::configureShownProperties() {
> +m_configureLabel->setVisible(true);

Coding style: brace should go to next line (the function below is also wrong...)

REPOSITORY
  R318 Dolphin

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

To: bruns, #dolphin, #baloo, #frameworks, ngraham, astippich, #vdg, 
elvisangelaccio
Cc: elvisangelaccio, meven, kfm-devel, alexde, feverfew, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, mikesomov


D20525: [InformationPanel] Use the new inline configuration mode

2019-04-14 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  About the UI, I noticed that when the panel is in "configure" mode, it's 
still possible to right-click it and click the "Configure" entry, which will do 
nothing.
  We could fix this by disabling the entry once clicked. Another idea: make the 
entry checkable instead of using the buttonbox.
  
  If we choose to go with the buttonbox, I'd rename the "Ok" button to "Save" 
or similar.

REPOSITORY
  R318 Dolphin

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

To: bruns, #dolphin, #baloo, #frameworks, ngraham, astippich, #vdg, 
elvisangelaccio
Cc: elvisangelaccio, meven, kfm-devel, alexde, feverfew, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, mikesomov


D20513: Remove last traces of KFileMetaDataWidget from information panel

2019-04-14 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
elvisangelaccio added a comment.
This revision is now accepted and ready to land.


  Thanks

REPOSITORY
  R318 Dolphin

BRANCH
  arcpatch-D20513

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

To: bruns, #frameworks, apol, ngraham, #dolphin, elvisangelaccio
Cc: elvisangelaccio, kfm-devel, alexde, feverfew, meven, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, mikesomov


D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

2019-04-14 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  Hmm, it seems you need to rebase this patch:
  
$ arc patch D20524
 INFO  Base commit is not in local repository; trying to fetch.
Created and checked out branch arcpatch-D20524.


This diff is against commit b26a3680b04cb9877721d693eb79ccb321fe40aa, 
but
the commit is nowhere in the working copy. Try to apply it against the
current working copy state? (49e778edf7bd70842879cc338fb85c1a178ecf7f)
[Y/n] Y
  
  ( I tried to apply it on current master)

REPOSITORY
  R824 Baloo Widgets

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

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams


D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

2019-04-14 Thread Elvis Angelaccio
elvisangelaccio added inline comments.

INLINE COMMENTS

> filemetadatawidget.cpp:53
>  {
> +QCheckBox* check;
>  QLabel* label;

I'd call it `checkBox` for better clarity.

> filemetadatawidget.cpp:113-114
>  row.value->deleteLater();
> +if (row.check)
> +row.check->deleteLater();
>  }

Missing braces

> filemetadatawidget.h:47-49
> +ReStart = 0,
> +Accept,
> +Cancel

Can you add some apidox for this enum? In particular, it's not so clear what 
`ReStart` is used for.

REPOSITORY
  R824 Baloo Widgets

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

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams


D20513: Remove last traces of KFileMetaDataWidget from information panel

2019-04-14 Thread Elvis Angelaccio
elvisangelaccio added inline comments.

INLINE COMMENTS

> filemetadataconfigurationdialog.h:35
>   * @brief Dialog which allows to configure which meta data should be shown
>   *in the KFileMetaDataWidget.
>   */

Can you please also update this comment? It should mention 
`Baloo::FileMetaDataWidget`.

REPOSITORY
  R318 Dolphin

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

To: bruns, #frameworks, apol, ngraham, #dolphin
Cc: elvisangelaccio, kfm-devel, alexde, feverfew, meven, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, mikesomov


D20523: Remove a dead declaration

2019-04-14 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.

REPOSITORY
  R824 Baloo Widgets

BRANCH
  cleanup

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

To: bruns, #baloo, #frameworks, ngraham, astippich, elvisangelaccio
Cc: gennad, domson, ashaposhnikov, astippich, spoorun, abrahams


  1   2   3   4   5   6   7   8   9   10   >