D17595: Upstream Dolphin's file rename dialog

2018-12-14 Thread Nathaniel Graham 
ngraham added a dependent revision: D17597: Use newly-upstreamed rename dialog 
from KIO.

REPOSITORY
  R241 KIO

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

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

D17595: Upstream Dolphin's file rename dialog

2018-12-14 Thread Nathaniel Graham 
ngraham added a dependent revision: D17596: [KDirOperator] Allow renaming files 
from the context menu.

REPOSITORY
  R241 KIO

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

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

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-14 Thread Nathaniel Graham 
ngraham created this revision.
ngraham added reviewers: Frameworks, Dolphin.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  BUG: 189482
  FIXED-IN: 5.54

TEST PLAN
  [image goes here]
  
  - Using the menu item works to rename one or more items
  - The menu item is disables when there's no selection
  - This menu item is inappropriately enabled for items that cannot be deleted, 
just like the trash/delete menu item (pre-existing bug, will fix in another 
patch)

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h

To: ngraham, #frameworks, #dolphin
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17596: [KDirOperator] Allow renaming files from the context menu

2018-12-14 Thread Nathaniel Graham 
ngraham edited the summary of this revision.
ngraham edited the test plan for this revision.
ngraham added a dependency: D17595: Upstream Dolphin's file rename dialog.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17595: Upstream Dolphin's file rename dialog

2018-12-14 Thread Nathaniel Graham 
ngraham created this revision.
ngraham added reviewers: Frameworks, Dolphin, broulik.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  KIO didn't actually have its own rename dialog, which is necessary to 
implement the reuested rename-from-the-file-dialog feature (189482). This patch 
upstreams Dolphin's so all KIO users can use it.
  
  CCBUG: 189482

TEST PLAN
  Apply [patch X] or [patch Y] and initiate a rename operation for one or more 
files. See that it works as expected.

REPOSITORY
  R241 KIO

BRANCH
  upstream-dolphins-rename-dialog (branched from master)

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

AFFECTED FILES
  src/widgets/CMakeLists.txt
  src/widgets/renamefiledialog.cpp
  src/widgets/renamefiledialog.h

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

D17594: Notify about the provider not being loaded

2018-12-14 Thread Aleix Pol Gonzalez 
apol created this revision.
apol added reviewers: Frameworks, leinir.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
apol requested review of this revision.

REVISION SUMMARY
  Adopts D17593 

REPOSITORY
  R304 KNewStuff

BRANCH
  master

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

AFFECTED FILES
  src/core/engine.cpp

To: apol, #frameworks, leinir
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17593: Notify if a default provider failed to download

2018-12-14 Thread Aleix Pol Gonzalez 
apol created this revision.
apol added reviewers: Frameworks, leinir.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
apol requested review of this revision.

REVISION SUMMARY
  Otherwise we end up waiting forever without really knowing what
  happened.

REPOSITORY
  R235 Attica

BRANCH
  master

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

AFFECTED FILES
  src/providermanager.cpp
  src/providermanager.h

To: apol, #frameworks, leinir
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17528: Refactor SlaveInterface::calcSpeed

2018-12-14 Thread Stefan Brüns 
bruns added inline comments.

INLINE COMMENTS

> slaveinterface.cpp:102
> +// Minimum list size is 1 and maximum list size is 8. Delta is calculated
> +// using first and last item from the list at least every 900 msecs.
> +

.. at most ...

> slaveinterface.cpp:203
>  d->filesize = d->offset;
> -d->sizes[0] = d->filesize - d->offset;
> -d->times[0] = 0;
> -d->nums = 1;
> +d->transfer_details.append({0, (d->filesize - d->offset)});
>  d->speed_timer.start(1000);

Strange way of writing {0, 0} ...

> dfaure wrote in slaveinterface_p.h:48
> the old "nums" is now the vector size, right?
> 
> nums was initialized to 0, so this should not initialize the vector to 
> max_count items

The result is the same in this case - after 7 seconds, the initial items are 
shifted out of the array. During these 7 seconds, the first element is always 
{0, 0}. It does not matter if last() is the 2nd or 7th element.

REPOSITORY
  R241 KIO

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

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

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

2018-12-14 Thread Elvis Angelaccio 
elvisangelaccio reopened this revision.
elvisangelaccio added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> kcoredirlister.cpp:1576-1577
> +for (auto kit = dir->lstItems.begin(), kend = 
> dir->lstItems.end(); kit != kend; ++kit) {
>  const KFileItem oldItem = *kit;
> +KFileItem newItem = oldItem;
>  

Stacktrace from bug #401552  
points here. Reverting this commit seems to fix the crash.

@jtamate Can you have a look?

REPOSITORY
  R241 KIO

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

To: jtamate, #frameworks, dfaure
Cc: elvisangelaccio, bruns, kde-frameworks-devel, mwolff, markg, michaelh, 
ngraham

D17310: Improve Kile icon with LaTeX font

2018-12-14 Thread loh tar 
loh.tar added a comment.


  For me is these root sign nondescript. When the orig old blue symbol is not 
desired, how about to freshen it up? How? I don't know.
  Perhaps Black & White just as all your other offers on these rectangular 
sheet of paper?
  Furthermore exist these splash-screen. So maybe in yellow on these fuzzy gray 
paper?
  
  F6477111: kile_splash.png 

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

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

To: trickyricky26, #vdg, ngraham, #kile
Cc: loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17310: Improve Kile icon with LaTeX font

2018-12-14 Thread TrickyRicky 
trickyricky26 added a comment.


  I have made another design based on @ndavis feedback:
  F6477088: kile-alternative.svg.png 
  
  F6477093: Original root a with pen screenshot.png 

  This uses the root a design from the Cantor icon which makes it pixel perfect 
and more consistent. Also, the pen is a bigger to be more recognizable at 100% 
scaling (this change can be made to the LaTeX font design as well).

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

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

To: trickyricky26, #vdg, ngraham, #kile
Cc: mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17310: Improve Kile icon with LaTeX font

2018-12-14 Thread TrickyRicky 
trickyricky26 edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

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

To: trickyricky26, #vdg, ngraham, #kile
Cc: mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17310: Improve Kile icon with LaTeX font

2018-12-14 Thread TrickyRicky 
trickyricky26 updated this revision to Diff 47591.
trickyricky26 edited the test plan for this revision.
trickyricky26 added a comment.


  - Usr silver pen design to improve contrast

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17310?vs=47364=47591

BRANCH
  improve-kile-icon (branched from master)

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

AFFECTED FILES
  icons-dark/apps/48/kile.svg
  icons/apps/48/kile.svg

To: trickyricky26, #vdg, ngraham, #kile
Cc: mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17466: Fix doxygen markdown rendering

2018-12-14 Thread Eli MacKenzie 
This revision was automatically updated to reflect the committed changes.
Closed by commit R264:28dca00d2f3a: Fix doxygen markdown rendering (authored by 
argonel).

REPOSITORY
  R264 KApiDox

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17466?vs=47230=47589

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

AFFECTED FILES
  docs/metainfo_syntax.md

To: argonel, apol
Cc: apol, kde-frameworks-devel, kde-doc-english, michaelh, ngraham, bruns, 
skadinna

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, #frameworks, dfaure, mlaurent
Cc: mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, 
michaelh, bruns

D17355: Align Plasma QML button content in center if it has an icon

2018-12-14 Thread Björn Feber 
GB_2 planned changes to this revision.
GB_2 added a comment.


  In D17355#376973 , @mart wrote:
  
  > in general, buttons with icons may be stacked vertically, which for me 
makes a general -1
  
  
  But it's not consistent with Qt Widgets and looks kind of weird.
  
  In D17355#376975 , @mart wrote:
  
  > In D17355#371722 , @GB_2 wrote:
  >
  > > Improved code.
  > >  How `qmlscene tests/components/button.qml` looks like:
  > >  F6457867: Plasma QML Button Content Align qmlscene (1).png 

  > >  For some reason "elide" doesn't work...
  >
  >
  > is elide working on the latest revision?
  
  
  No, I still didn't figure that out.
  
  In D17355#376976 , @mart wrote:
  
  > pleaase add the changes to plasmacomponents3 as well
  
  
  I will do that when this is done.
  
  This needs revision, but it's not really my priority.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: mart, davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, 
squeakypancakes, alexde, IohannesPetros, GB_2, trickyricky26, ragreen, Pitel, 
michaelh, crozbo, ndavis, ZrenBot, firef, bruns, skadinna, lesliezhai, 
ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, sebas, mbohlender

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

2018-12-14 Thread Nathaniel Graham 
ngraham added a comment.


  @dfaure, how's this looking now?

REPOSITORY
  R241 KIO

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

To: emateli, #frameworks, dfaure, mlaurent
Cc: mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, 
michaelh, bruns

T3689: Add abi compliance checker to CI

2018-12-14 Thread Sandro Knauß 
knauss added a revision: D17579: Handle cases where tailing "/" in 
CMAKE_PREFIX_PATH fails the detection of additional include directories..

TASK DETAIL
  https://phabricator.kde.org/T3689

To: knauss
Cc: dfaure, kde-frameworks-devel, bcooksley, sysadmin, scarlettclark, aacid, 
knauss, alexeymin, kaning, blazquez

D17355: Align Plasma QML button content in center if it has an icon

2018-12-14 Thread Marco Martin 
mart added a comment.


  pleaase add the changes to plasmacomponents3 as well

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: mart, davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, 
squeakypancakes, alexde, IohannesPetros, GB_2, trickyricky26, ragreen, Pitel, 
michaelh, crozbo, ndavis, ZrenBot, firef, bruns, skadinna, lesliezhai, 
ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, sebas, mbohlender

D17355: Align Plasma QML button content in center if it has an icon

2018-12-14 Thread Marco Martin 
mart added a comment.


  In D17355#371722 , @GB_2 wrote:
  
  > Improved code.
  >  How `qmlscene tests/components/button.qml` looks like:
  >  F6457867: Plasma QML Button Content Align qmlscene (1).png 

  >  For some reason "elide" doesn't work...
  
  
  is elide working on the latest revision?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: mart, davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, 
squeakypancakes, alexde, IohannesPetros, GB_2, trickyricky26, ragreen, Pitel, 
michaelh, crozbo, ndavis, ZrenBot, firef, bruns, skadinna, lesliezhai, 
ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, sebas, mbohlender

D17355: Align Plasma QML button content in center if it has an icon

2018-12-14 Thread Marco Martin 
mart added a comment.


  in general, buttons with icons may be stacked vertically, which for me makes 
a general -1

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: mart, davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, 
squeakypancakes, alexde, IohannesPetros, GB_2, trickyricky26, ragreen, Pitel, 
michaelh, crozbo, ndavis, ZrenBot, firef, bruns, skadinna, lesliezhai, 
ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, sebas, mbohlender

D17528: Refactor SlaveInterface::calcSpeed

2018-12-14 Thread David Faure 
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> slaveinterface.cpp:112
> +const TransferInfo first = d->transfer_details.first();
> +const TransferInfo last = d->transfer_details.last();
> +KIO::filesize_t lspeed = 1000 * (last.size - first.size) / 
> (last.time - first.time);

why not just call "last" the (currently unnamed) TransferInfo created 2 lines 
above, rather than extracting it out of the vector just after appending?

> slaveinterface_p.h:39
> +
> +struct TransferInfo {
> +qint64 time;

global namespace pollution, better keep this within KIO::SlaveInterfacePrivate.

> slaveinterface_p.h:48
>  SlaveInterfacePrivate()
> -: connection(nullptr), filesize(0), offset(0), last_time(0), 
> start_time(0),
> -  nums(0), slave_calcs_speed(false)
> +: connection(nullptr), transfer_details(max_count), filesize(0), 
> offset(0),
> +  slave_calcs_speed(false)

the old "nums" is now the vector size, right?

nums was initialized to 0, so this should not initialize the vector to 
max_count items

REPOSITORY
  R241 KIO

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

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

D17528: Refactor SlaveInterface::calcSpeed

2018-12-14 Thread Chinmoy Ranjan Pradhan 
chinmoyr updated this revision to Diff 47567.
chinmoyr added a comment.


  Used a structure to group size and time.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17528?vs=47518=47567

BRANCH
  master

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

AFFECTED FILES
  src/core/slaveinterface.cpp
  src/core/slaveinterface_p.h

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

D17500: Restore mobipocket extractor

2018-12-14 Thread Luigi Toscano 
ltoscano added a comment.


  In D17500#376854 , @astippich 
wrote:
  
  > In D17500#376221 , @aacid wrote:
  >
  > > In D17500#375753 , @astippich 
wrote:
  > >
  > > > Oh, thanks for the hint, didn't know that. That makes it a lot more 
complicated than a straight port :(
  > > >  Looking at the code of kdegraphics-mobipocket, shouldn't the thumbnail 
extractor actually be part of kio-extras? Seems quite KIO-specific, and a quick 
look at lxr didn't reveal any usages of the thumbnailer.
  > >
  > >
  > > Why would it be part of kio-extras? the beauty of plugins is that they 
can live wherever, no?
  >
  >
  > My thinking here was that a lot of other thumbnailers are located in 
kio-extras. Moving the thumbnailer there too would lift the KIO dependency and 
make qmobipocket easier to deploy and to be used by others.
  
  
  Just a note about this: kio-extras was not meant to be a collector of all 
possible kios. It's becaming as such, but if this continues we may end up 
revisiting it and splitting it.

REPOSITORY
  R286 KFileMetaData

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

To: astippich, bruns
Cc: ltoscano, mgallien, aacid, kde-frameworks-devel, #baloo, ashaposhnikov, 
michaelh, astippich, spoorun, ngraham, bruns, abrahams

D17500: Restore mobipocket extractor

2018-12-14 Thread Alexander Stippich 
astippich added a comment.


  In D17500#376221 , @aacid wrote:
  
  > In D17500#375753 , @astippich 
wrote:
  >
  > > Oh, thanks for the hint, didn't know that. That makes it a lot more 
complicated than a straight port :(
  > >  Looking at the code of kdegraphics-mobipocket, shouldn't the thumbnail 
extractor actually be part of kio-extras? Seems quite KIO-specific, and a quick 
look at lxr didn't reveal any usages of the thumbnailer.
  >
  >
  > Why would it be part of kio-extras? the beauty of plugins is that they can 
live wherever, no?
  
  
  My thinking here was that a lot of other thumbnailers are located in 
kio-extras. Moving the thumbnailer there too would lift the KIO dependency and 
make qmobipocket easier to deploy and to be used by others.
  
  > Do I understand that the answer to my "Is it possible to move the extractor 
to kdegraphics-mobipocket instead of having it in kfilemetadata? " question is 
no?
  > 
  > If so, that probably needs fixing, the fact that you can't have external 
plugins means that the code is probably not as good as it should
  
  No, the answer was I don't know :) But as @mgallien and @bruns stated, it's 
possible. But I lack the time and motivation to work on this.
  
  But honestly, I'm not super interested in this, I don't have any mobipocket 
files. I just saw that this code lay there disabled for years, and thought it 
would be easy to enable, which it isn't.
  Would you be fine if I disable the mobiextractor like before, and merge the 
changes anyways? This way, it does at least compile and runs if someone enables 
it in cmake.
  Otherwise I will abandon this revision.

REPOSITORY
  R286 KFileMetaData

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

To: astippich, bruns
Cc: mgallien, aacid, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D17310: Improve Kile icon with LaTeX font

2018-12-14 Thread TrickyRicky 
trickyricky26 edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

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

To: trickyricky26, #vdg, ngraham, #kile
Cc: mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D10446: Add KLanguageName

2018-12-14 Thread Harald Sitter 
sitter added a comment.


  Stacking the functions seems to work fine
  
QString KLanguageName::nameForCode(const QString )
{
const QStringList parts = QLocale().name().split(QChar('_'));
return nameForCodeInLocale(code, parts.at(0));
}
  
  I do have various refinements and fixes for the logic plus a unit test I can 
update the diff if you are ok with that.
  
  As for kcoreaddons, I don't think we could go there with kconfig could we? 
And I am not sure QSettings is up to the task?
  
  I also have some behavior concerns. The auto-fallback to QLocale is super 
handy, but isn't very flexible: As an application author I might have another 
way to map a language to a pretty string, if KLanguageName automatically falls 
back to QLocale, I won't be able to easily determine if I should use another 
(possibly better) fallback.
  At the same time falling back to `QLocale::languageToString` is not 
necessarily in the interest of the user either. Who's to say the user will know 
what a language means in English.
  
  So, I haven't give this a great deal of thought, but it seems that always 
returning QString(), if we cannot resolve a string internally, is possibly more 
flexible. Worst case the consumer has to do:
  
str = KLanguageName::nameForCode("fr")
if (str.isEmpty()) { str = QLocale("fr").nativeLanguageName() }
  
  Which isn't that much code TBH.

REPOSITORY
  R265 KConfigWidgets

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

To: aacid
Cc: kde-frameworks-devel, sitter, markg, apol, michaelh, ngraham, bruns

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-14 Thread Shubham 
shubham marked an inline comment as done.

REPOSITORY
  R241 KIO

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

To: shubham, #frameworks, dfaure
Cc: davidedmundson, ngraham, broulik, kde-frameworks-devel, michaelh, bruns

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-14 Thread Shubham 
shubham updated this revision to Diff 47549.
shubham added a comment.


  Unit test

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17545?vs=47503=47549

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/core/copyjob.cpp

To: shubham, #frameworks, dfaure
Cc: davidedmundson, ngraham, broulik, kde-frameworks-devel, michaelh, bruns