D24932: Add button to open the folder in filelight to view more details

2020-01-13 Thread Shubham 
shubham abandoned this revision.
shubham added a comment.


  Abandoned infavour of D26650 

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, broulik
Cc: broulik, ltoscano, pino, kde-frameworks-devel, #frameworks, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D24932: Add button to open the folder in filelight to view more details

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


  It would make more sense to submit a new patch. This one wasn't reverted, so 
it's not clear what would happen if we tried to land it.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, broulik
Cc: broulik, ltoscano, pino, kde-frameworks-devel, #frameworks, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D24932: Add button to open the folder in filelight to view more details

2020-01-13 Thread Shubham 
shubham added a comment.


  @broulik I believe this is what you wanted?

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, broulik
Cc: broulik, ltoscano, pino, kde-frameworks-devel, #frameworks, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D24932: Add button to open the folder in filelight to view more details

2020-01-13 Thread Shubham 
shubham updated this revision to Diff 73376.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24932?vs=72263&id=73376

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

AFFECTED FILES
  src/widgets/kpropertiesdialog.cpp

To: shubham, ngraham, #frameworks, broulik
Cc: broulik, ltoscano, pino, kde-frameworks-devel, #frameworks, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D24932: Add button to open the folder in filelight to view more details

2020-01-07 Thread Kai Uwe Broulik 
broulik requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, broulik
Cc: broulik, ltoscano, pino, kde-frameworks-devel, #frameworks, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D24932: Add button to open the folder in filelight to view more details

2020-01-07 Thread Kai Uwe Broulik 
broulik reopened this revision.
broulik added a comment.
This revision is now accepted and ready to land.


  Can you please look for filelight using 
`KService::serviceByDesktopName("org.kde.filelight")`, then you don't have 
hardcode the executable path, name, icon, etc.

REPOSITORY
  R241 KIO

BRANCH
  file

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

To: shubham, ngraham, #frameworks
Cc: broulik, ltoscano, pino, kde-frameworks-devel, #frameworks, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D24932: Add button to open the folder in filelight to view more details

2019-12-31 Thread Luigi Toscano 
ltoscano added a comment.


  As a reference for the future: this change broke the two weeks string freeze 
- which we haven't enforced so strongly in the past months, but please keep an 
eye on it. Expecially this time of the year with many countries on vacation.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks
Cc: ltoscano, pino, kde-frameworks-devel, #frameworks, LeGast00n, GB_2, 
michaelh, ngraham, bruns

D24932: Add button to open the folder in filelight to view more details

2019-12-28 Thread Shubham 
shubham added a comment.


  @pino @ngraham Fixed here https://phabricator.kde.org/D26265.

REPOSITORY
  R241 KIO

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

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

D24932: Add button to open the folder in filelight to view more details

2019-12-28 Thread Pino Toscano 
pino added inline comments.

INLINE COMMENTS

> kpropertiesdialog.cpp:1453
> +// Open the current folder in filelight
> +KRun::run(QStringLiteral("/usr/bin/filelight"), { properties->url() }, 
> properties->window(), QStringLiteral("Filelight"), 
> QStringLiteral("filelight"));
> +}

please do not hardcode the path to filelight... other than breaking on systems 
where filelight is installed in a prefix different than /usr, this breaks on 
Mac and Windows (and not only).

REPOSITORY
  R241 KIO

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

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

D24932: Add button to open the folder in filelight to view more details

2019-12-27 Thread Shubham 
shubham closed this revision.

REPOSITORY
  R241 KIO

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

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

D24932: Add button to open the folder in filelight to view more details

2019-12-27 Thread Nathaniel Graham 
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Ship it!

REPOSITORY
  R241 KIO

BRANCH
  file

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

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

D24932: Add button to open the folder in filelight to view more details

2019-12-27 Thread Shubham 
shubham marked an inline comment as done.

REPOSITORY
  R241 KIO

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

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

D24932: Add button to open the folder in filelight to view more details

2019-12-27 Thread Shubham 
shubham updated this revision to Diff 72263.
shubham added a comment.


  Remove the use of variable

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24932?vs=72261&id=72263

BRANCH
  file

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

AFFECTED FILES
  src/widgets/kpropertiesdialog.cpp
  src/widgets/kpropertiesdialog_p.h

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

D24932: Add button to open the folder in filelight to view more details

2019-12-27 Thread Nathaniel Graham 
ngraham added a comment.


  Much better now, thanks. There's just one more thing I only notices now, 
sorry. Then I think it'll be good to go.

INLINE COMMENTS

> kpropertiesdialog.cpp:1452
> +{
> +const QUrl url = properties->url();
> +

Instead of defining this as a variable that's used only once, you can just 
directly access the value from `properties->url()` in the `KRun::run()` function

REPOSITORY
  R241 KIO

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

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

D24932: Add Button to open the folder in filelight to view more details

2019-12-27 Thread Shubham 
shubham retitled this revision from "Add Button to open the folder in filelight 
for more details" to "Add Button to open the folder in filelight to view more 
details".

REPOSITORY
  R241 KIO

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

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

D24932: Add button to open the folder in filelight to view more details

2019-12-27 Thread Shubham 
shubham retitled this revision from "Add Button to open the folder in filelight 
to view more details" to "Add button to open the folder in filelight to view 
more details".

REPOSITORY
  R241 KIO

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

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