D13549: Switch KIO::convertSize() to KFormat::formatByteSize()

[Pino Toscano]

pino updated this revision to Diff 3.
pino marked an inline comment as done.
pino added a comment.


  Fix precision passed to formatByteSize().

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13549?vs=36179=3

BRANCH
  kformat (branched from master)

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

AFFECTED FILES
  src/core/global.cpp

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

D13549: Switch KIO::convertSize() to KFormat::formatByteSize()

[Pino Toscano]

pino marked an inline comment as done.
pino added inline comments.

INLINE COMMENTS

> aacid wrote in global.cpp:58
> Should this -1 be 1? Looking at the kformat code it'll use that unless unit 
> is 0 so the old code would be using 1 and the new one -1

Indeed, 1 is the right value, and `KFormat` does the right thing when the unit 
is byte.

REPOSITORY
  R241 KIO

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

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

D13730: Fix memory management in WaylandOutputManagement

[David Edmundson]

davidedmundson created this revision.
davidedmundson added a reviewer: KWin.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  There was no destructor in the protocol, which meant the auto generated
  _destroy function only deletes the wl_proxy object, but doesn't actually
  send anything to the server.
  
  Result was OutputConfiguration objects on
  the server just linger forever and it's a broken state

TEST PLAN
  Added unit test that objects have the lifespan they should do

REPOSITORY
  R127 KWayland

BRANCH
  dave

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

AFFECTED FILES
  autotests/client/test_wayland_outputmanagement.cpp
  src/client/protocols/output-management.xml
  src/server/outputconfiguration_interface.cpp

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

D13729: Isolate every test within WaylandOutputManagement

[David Edmundson]

davidedmundson created this revision.
davidedmundson added a reviewer: KWin.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  The current code used the same outputInterface between all tests, with
  data and outdated connections slowly accumulating.
  
  This meant most the code didn't work as it was intended, for
  example testExampleConfig had the config applied from the connect in the
  previous test. It just happened to pass.
  
  This resets everything between each test.

TEST PLAN
  Can now add a test without going insane
  Existing tests still pass

REPOSITORY
  R127 KWayland

BRANCH
  davidedmundson/scalef

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

AFFECTED FILES
  autotests/client/test_wayland_outputmanagement.cpp

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

D13549: Switch KIO::convertSize() to KFormat::formatByteSize()

[Albert Astals Cid]

aacid added inline comments.

INLINE COMMENTS

> global.cpp:58
>  
> -const int precision = (unit == 0) ? 0 : 1; // unit == 0 -> Bytes, no 
> rounding
> -return dialectUnits.at(unit).arg(QLocale().toString(size, 'f', 
> precision));
> +return KFormat().formatByteSize(fileSize, -1, dialect);
>  }

Should this -1 be 1? Looking at the kformat code it'll use that unless unit is 
0 so the old code would be using 1 and the new one -1

REPOSITORY
  R241 KIO

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

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

KDE CI: Frameworks knewstuff kf5-qt5 FreeBSDQt5.10 - Build # 13 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20knewstuff%20kf5-qt5%20FreeBSDQt5.10/13/
 Project:
Frameworks knewstuff kf5-qt5 FreeBSDQt5.10
 Date of build:
Mon, 25 Jun 2018 21:20:37 +
 Build duration:
1 min 33 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 3 test(s)Failed: TestSuite.kmoretoolstest

D13706: [KMoreTools] Enable installing tools via appstream url

[Nicolas Fella]

This revision was automatically updated to reflect the committed changes.
Closed by commit R304:84432a55e969: [KMoreTools] Enable installing tools via 
appstream url (authored by nicolasfella).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D13706?vs=36652=36655#toc

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13706?vs=36652=36655

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

AFFECTED FILES
  src/kmoretools/kmoretools.cpp
  src/kmoretools/kmoretools.h
  src/kmoretools/kmoretools_p.h
  src/kmoretools/kmoretoolsconfigdialog_p.cpp
  src/kmoretools/kmoretoolspresets.cpp

To: nicolasfella, #frameworks, gregormi, ngraham
Cc: ngraham, dhaumann, kde-frameworks-devel, michaelh, bruns

D13706: [KMoreTools] Enable installing tools via appstream url

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Here's the last of the apps for which I could find upstream AppStream 
information:
  
  After those are updated, I'm good with this!

INLINE COMMENTS

> kmoretoolspresets.cpp:61
> +ADD_ENTRY("com.uploadedlobster.peek",   0, 
> "https://github.com/phw/peek;, "com.uploadedlobster.peek.desktop"); // easy 
> to use screen recorder, creates gif
> +ADD_ENTRY("catfish",1, 
> "http://www.twotoasts.de/index.php/catfish/;, "catfish");
> +ADD_ENTRY("ding",   0, 
> "https://www-user.tu-chemnitz.de/~fri/ding/;, ""); // Offline dict, Online: 
> http://dict.tu-chemnitz.de/dings.cgi

`catfish`

https://git.launchpad.net/catfish-search/tree/data/metainfo/catfish.appdata.xml.in#n4

> kmoretoolspresets.cpp:68
> +ADD_ENTRY("giggle", 1, 
> "https://wiki.gnome.org/Apps/giggle/;, "giggle.desktop"); // good for 
> searching in history
> +ADD_ENTRY("git-cola-folder-handler",1, 
> "https://git-cola.github.io;, "");
> +ADD_ENTRY("git-cola-view-history.kmt-edition",  1, 
> "https://git-cola.github.io;, "");

`git-cola.desktop`

https://github.com/git-cola/git-cola/blob/master/share/appdata/git-cola.appdata.xml#L3

> kmoretoolspresets.cpp:72
> +ADD_ENTRY("qgit.kmt-edition",   1, 
> "http://libre.tibirna.org/projects/qgit;, "");
> +ADD_ENTRY("gitg",   1, 
> "https://wiki.gnome.org/action/show/Apps/Gitg?action=show=Gitg;, "");
> +ADD_ENTRY("gnome-search-tool",  0, 
> "https://help.gnome.org/users/gnome-search-tool/;, 
> "gnome-search-tool.desktop"); // has good filtering options

`gitg.desktop`

https://gitlab.gnome.org/GNOME/gitg/blob/master/data/gitg.appdata.xml#L3

REPOSITORY
  R304 KNewStuff

BRANCH
  appstream

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

To: nicolasfella, #frameworks, gregormi, ngraham
Cc: ngraham, dhaumann, kde-frameworks-devel, michaelh, bruns

D13124: [RFC] Add Share action to Dolphin context menu

[Elvis Angelaccio]

elvisangelaccio added a comment.


  +1, thanks!

REPOSITORY
  R495 Purpose Library

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

To: nicolasfella, apol
Cc: markg, broulik, kde-frameworks-devel, elvisangelaccio, ngraham, apol, 
kfm-devel, #dolphin, michaelh, spoorun, navarromorales, isidorov, firef, 
andrebarros, bruns, emmanuelp

D13706: [KMoreTools] Enable installing tools via appstream url

[Nicolas Fella]

nicolasfella marked 9 inline comments as done.

REPOSITORY
  R304 KNewStuff

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

To: nicolasfella, #frameworks, gregormi, ngraham
Cc: ngraham, dhaumann, kde-frameworks-devel, michaelh, bruns

D13706: [KMoreTools] Enable installing tools via appstream url

[Nicolas Fella]

nicolasfella updated this revision to Diff 36652.
nicolasfella added a comment.


  o  - Even more appstream ids. Thanks Nate

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13706?vs=36651=36652

BRANCH
  appstream

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

AFFECTED FILES
  src/kmoretools/kmoretools.cpp
  src/kmoretools/kmoretools.h
  src/kmoretools/kmoretools_p.h
  src/kmoretools/kmoretoolsconfigdialog_p.cpp
  src/kmoretools/kmoretoolspresets.cpp

To: nicolasfella, #frameworks, gregormi, ngraham
Cc: ngraham, dhaumann, kde-frameworks-devel, michaelh, bruns

D13706: [KMoreTools] Enable installing tools via appstream url

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> kmoretoolspresets.cpp:60
> +ADD_ENTRY("angrysearch",0, 
> "https://github.com/DoTheEvo/ANGRYsearch;, "");
> +ADD_ENTRY("com.uploadedlobster.peek",   0, 
> "https://github.com/phw/peek;, ""); // easy to use screen recorder, creates 
> gif
> +ADD_ENTRY("catfish",1, 
> "http://www.twotoasts.de/index.php/catfish/;, "catfish");

`com.uploadedlobster.peek.desktop`

https://github.com/phw/peek/blob/master/data/com.uploadedlobster.peek.appdata.xml.in

> kmoretoolspresets.cpp:93
> +ADD_ENTRY("simplescreenrecorder",   0, 
> "http://www.maartenbaert.be/simplescreenrecorder/;, 
> "simplescreenrecorder.desktop");
> +ADD_ENTRY("shutter",0, 
> "http://shutter-project.org;, ""); // good for edit screenshot after capture
> +ADD_ENTRY("vokoscreen", 0, 
> "https://github.com/vkohaupt/vokoscreen;, ""); // feature-rich screen recorder

`org.shutterproject.shutter`

https://bugs.launchpad.net/shutter/+bug/1739971

REPOSITORY
  R304 KNewStuff

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

To: nicolasfella, #frameworks, gregormi, ngraham
Cc: ngraham, dhaumann, kde-frameworks-devel, michaelh, bruns

D13706: [KMoreTools] Enable installing tools via appstream url

[Nicolas Fella]

nicolasfella updated this revision to Diff 36651.
nicolasfella added a comment.


  - Add more appstream ids

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13706?vs=36650=36651

BRANCH
  appstream

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

AFFECTED FILES
  src/kmoretools/kmoretools.cpp
  src/kmoretools/kmoretools.h
  src/kmoretools/kmoretools_p.h
  src/kmoretools/kmoretoolsconfigdialog_p.cpp
  src/kmoretools/kmoretoolspresets.cpp

To: nicolasfella, #frameworks, gregormi, ngraham
Cc: ngraham, dhaumann, kde-frameworks-devel, michaelh, bruns

D13706: [KMoreTools] Enable installing tools via appstream url

[Nicolas Fella]

nicolasfella updated this revision to Diff 36650.
nicolasfella added a comment.


  - doc++

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13706?vs=36620=36650

BRANCH
  appstream

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

AFFECTED FILES
  src/kmoretools/kmoretools.cpp
  src/kmoretools/kmoretools.h
  src/kmoretools/kmoretools_p.h
  src/kmoretools/kmoretoolsconfigdialog_p.cpp
  src/kmoretools/kmoretoolspresets.cpp

To: nicolasfella, #frameworks, gregormi, ngraham
Cc: ngraham, dhaumann, kde-frameworks-devel, michaelh, bruns

D13549: Switch KIO::convertSize() to KFormat::formatByteSize()

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> aacid wrote in global.cpp:44
> Am i reading the code wrong or are we doing the readEntry twice? (i know the 
> old code did the same)
> 
> Do you understand why?

> Am i reading the code wrong or are we doing the readEntry twice? (i know the 
> old code did the same)

Apparently so.

> Do you understand why?

No, that is why I chose to not touch at all, keeping the behaviour unchanged 
(it is not related to this patch, anyway).

REPOSITORY
  R241 KIO

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

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

D13706: [KMoreTools] Enable installing tools via appstream url

[gregormi]

gregormi added a comment.


  In D13706#282765 , @ngraham wrote:
  
  > I'd like to see the missing AppStream URLs filled in for all the software 
in this list. There are currently some claring omissions, especially for KDE 
software (e.g. ksysguard) for which there is definitely an appstream ID 
available). For any software that doesn't have any AppStream information (and 
is therefore not visible or installable via Discover), I would actually 
advocate removing it from the list, and making the presence of AppStream data a 
pre-condition of inclusion--the reason being that otherwise an Install button 
can't be presented to the user. Entries without an Install button are just 
frustrating, tantamount to taunting them ("here's some cool software you could 
use; oh, sorry, can't actually install it lol")
  
  
  Hi Nate, thanks for reviewing.
  
  - I tried `appstreamcli ksysguard` and `appstreamcli ding`. Both do not yield 
a result. => I vote not to remove a program from this list just because it has 
no obvious appstream id (yet).
  - Programs which serve the same purpose should have a comment why they are in 
the list, like it was done with giggle ("// good for searching in history"). In 
such cases, for me it would be ok if there is no appstream id yet.
  - For me, it would be ok, if Nicolas does not have to search for all the 
missing appstream ids - except if you have fun to do it, Nicolas :-). The patch 
is a definite improvement to the current state and missing ids could also be 
added later incrementally.

REPOSITORY
  R304 KNewStuff

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

To: nicolasfella, #frameworks, gregormi, ngraham
Cc: ngraham, dhaumann, kde-frameworks-devel, michaelh, bruns

D13257: [server] Send frame event instead of flush on relative pointer motion

[Roman Gilg]

romangg edited the summary of this revision.

REPOSITORY
  R127 KWayland

BRANCH
  fixRelativeMotionFrame

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

To: romangg, #plasma, #kwin, #frameworks, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, michaelh, ngraham, bruns

D13719: OdfExtractor: deal with non-common prefix names

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 36647.
kossebau added a comment.


  Update to Jos' feedback (thanks for that)
  
  Using "firstChildElementNS" as name for the method, so it's closer to
  QDomNode API and more telling what it does
  
  If no-one has further comments, would commit on Wednesday.

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13719?vs=36637=36647

BRANCH
  noncommonprefixodf

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

AFFECTED FILES
  src/extractors/odfextractor.cpp

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

D13549: Switch KIO::convertSize() to KFormat::formatByteSize()

[Albert Astals Cid]

aacid added inline comments.

INLINE COMMENTS

> global.cpp:44
> +KFormat::BinaryUnitDialect 
> dialect(KFormat::BinaryUnitDialect(mainGroup.readEntry("BinaryUnitDialect", 
> int(KFormat::DefaultBinaryDialect;
> +dialect = 
> static_cast(mainGroup.readEntry("BinaryUnitDialect",
>  int(dialect)));
>  

Am i reading the code wrong or are we doing the readEntry twice? (i know the 
old code did the same)

Do you understand why?

REPOSITORY
  R241 KIO

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

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

D13722: Don't export kf5-config to the CMake config file

[Volker Krause]

vkrause created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
vkrause requested review of this revision.

REVISION SUMMARY
  It's not needed during building, but the corresponding find_package call
  will check for its existence anyway, which is inconvenient when cross-
  compiling.

REPOSITORY
  R239 KDELibs4Support

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt

To: vkrause
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13719: OdfExtractor: deal with non-common prefix names

[Jos van den Oever]

vandenoever accepted this revision.
vandenoever added a comment.
This revision is now accepted and ready to land.


  Good fixes. Just some small naming suggestions.

INLINE COMMENTS

> odfextractor.cpp:37
> +
> +QDomElement namedItemNS(const QDomNode , const QString , const 
> QString )
> +{

Perhaps name this `elementByTagNameNS`.

> odfextractor.cpp:39
> +{
> +for (auto n = node.firstChildElement(); !n.isNull(); n = 
> n.nextSiblingElement()) {
> +if (n.localName() == localName && n.namespaceURI() == nsURI) {

Variable name `e` for element is more logical.

REPOSITORY
  R286 KFileMetaData

BRANCH
  noncommonprefixodf

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

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

D13718: Support choosing .ico files in custom icon file chooser

[Nathaniel Graham]

ngraham added a comment.


  It does actually work; I tried it. The bug reporter also reported that it 
worked for him too if he manually entered the path to a .ico file.

REPOSITORY
  R302 KIconThemes

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

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

D13718: Support choosing .ico files in custom icon file chooser

[Christoph Feck]

cfeck added a comment.


  But does it actually work? Our icon loader removes the extension, and tries 
to find the icon with a set of known extensions (grep -i xpm in 
kiconloader.cpp).

REPOSITORY
  R302 KIconThemes

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

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

D13721: [ECMGenerateHeaders] Add option for other header file extension than .h

[Friedrich W. H. Kossebau]

kossebau created this revision.
Restricted Application added projects: Frameworks, Build System.
Restricted Application added subscribers: kde-buildsystem, kde-frameworks-devel.
kossebau requested review of this revision.

REVISION SUMMARY
  Quite some C++-based projects do not use .h as header file extension, but
  .hpp, .hxx or other variants. Making the header file extension configurable
  enables to make use of ECMGenerateHeaders in such projects.

TEST PLAN
  The added unit test works, existing unit tests work as before.
  Also using in project with .hpp files works.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  supportotherheaderextensions

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

AFFECTED FILES
  modules/ECMGenerateHeaders.cmake
  tests/ECMGenerateHeadersTest/headtest1.hpp
  tests/ECMGenerateHeadersTest/headtest2.hpp
  tests/ECMGenerateHeadersTest/run_test.cmake.config

To: kossebau
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D13706: [KMoreTools] Enable installing tools via appstream url

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  Tried this out. It's a really great feature to support installation via the 
AppStream URL--the logical next step after an app has been recommended to you. 
I tried a few and installation worked perfectly via Discover. There are a few 
user-facing papercuts such as when the appstream URL isn't found, but that's 
not your fault.
  
  I'd like to see the missing AppStream URLs filled in for all the software in 
this list. There are currently some claring omissions, especially for KDE 
software (e.g. ksysguard) for which there is definitely an appstream ID 
available). For any software that doesn't have any AppStream information (and 
is therefore not visible or installable via Discover), I would actually 
advocate removing it from the list, and making the presence of AppStream data a 
pre-condition of inclusion--the reason being that otherwise an Install button 
can't be presented to the user. Entries without an Install button are just 
frustrating, tantamount to taunting them ("here's some cool software you could 
use; oh, sorry, can't actually install it lol")
  
  To find a program's appstream ID, you can do `appstreamcli search ` to find that information on your system. There's a lot of missing data 
on an old distro like Neon; best to do this on a rolling release distro or a 
fixed-release distro with a recent release (e.g. Fedora 28 or Kubuntu 18.04) 
since the AppStream information is more likely to be up-to-date there. 
Alternatively, check out the sources, find the `.appdata.xml file` 
and find the ID there. If none exists, just default to using the desktop file 
name, which is what most distros currently do.

REPOSITORY
  R304 KNewStuff

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

To: nicolasfella, #frameworks, gregormi, ngraham
Cc: ngraham, dhaumann, kde-frameworks-devel, michaelh, bruns

D13719: OdfExtractor: deal with non-common prefix names

[Friedrich W. H. Kossebau]

kossebau created this revision.
kossebau added a reviewer: vandenoever.
Restricted Application added projects: Frameworks, Baloo.
Restricted Application added subscribers: Baloo, kde-frameworks-devel.
kossebau requested review of this revision.

TEST PLAN
  Autotest odfextractortest still works.

REPOSITORY
  R286 KFileMetaData

BRANCH
  noncommonprefixodf

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

AFFECTED FILES
  src/extractors/odfextractor.cpp

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

D13718: Support choosing .ico files in custom icon file chooser

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R302 KIconThemes

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

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

D13718: Support choosing .ico files in custom icon file chooser

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Frameworks, cfeck.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  .ico files work fine, so we should allow them to be chosen in the custom icon 
file chooser dialog/
  
  BUG: 233201
  FIXED-IN: 5.48

TEST PLAN
  - Find a .ico file lying around, or turn a .png file into a .ico file using 
https://convertico.com/
  - Typ to apply it to a folder in Dolphin
  - You can select the .ico file in the dialog, and when applied, it appears 
correctly as a custom icon for the folder

REPOSITORY
  R302 KIconThemes

BRANCH
  ico-support-in-icon-file-chooser-dialog (branched from master)

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

AFFECTED FILES
  src/kicondialog.cpp

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

D13706: [KMoreTools] Enable installing tools via appstream url

[Dominik Haumann]

dhaumann added a comment.


  The API documentation can / should still be improved. I tried to give an 
example of possible improvements.

INLINE COMMENTS

> kmoretools.h:490
> + * @since 5.48
> + * @return The service' appstream id
> + */

What about this:

  /**
   * Returns the associated appstream id that was previously set with 
setAppstreamId().
   * If no appstream id was set, an empty string is returned.
   *
   * @return The service's appstream id.
   *
   * @since 5.48
   */

> kmoretools.h:495
> +/**
> + * @since 5.48
> + * Sets the appstream id of the service. This is used to create a

Please move @since down. The tags are usually the last part in the API 
documentation.

> kmoretools.h:500
> + */
> +void setAppstreamId(const QString&);
> +

This is also missing the parameter, which can be used for documentation:

  /**
   * Sets the appstream id of the service. This is used to create a
   * appstream url for installing the service via a software store
   * (e.g. Discover). For instance, the appstream id for filelight is
   * "org.kde.filelight.desktop".
   *
   * @param id the appstream id
   *
   * @since 5.48
   */
  void setAppstreamId(const QString& id);

REPOSITORY
  R304 KNewStuff

BRANCH
  appstream

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

To: nicolasfella, #frameworks, gregormi
Cc: ngraham, dhaumann, kde-frameworks-devel, michaelh, bruns

D13706: [KMoreTools] Enable installing tools via appstream url

[Dominik Haumann]

dhaumann resigned from this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.


  @ngraham Since you know more about installation (e.g. via Discover), I would 
like you to give another +1, if possible.

REPOSITORY
  R304 KNewStuff

BRANCH
  appstream

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

To: nicolasfella, #frameworks, gregormi
Cc: ngraham, dhaumann, kde-frameworks-devel, michaelh, bruns

D13257: [server] Send frame event instead of flush on relative pointer motion

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added a comment.
This revision is now accepted and ready to land.


  You're matching the buttonReleased/buttonPressed/axis/etc events. So +2 for 
the immediate fix.
  
  But longterm the point of sendFrame is to help batching so the current code 
of calling sendFrame in every send event is wrong and undermines the wayland 
design.
  IMHO it needs to be either explicit from kwin, or alternatively some clever 
code here that only actually calls sendFrame once when we hit the event loop.

REPOSITORY
  R127 KWayland

BRANCH
  fixRelativeMotionFrame

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

To: romangg, #plasma, #kwin, #frameworks, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, michaelh, ngraham, bruns

D13714: Fix broken url to API specification

[Ralf Habacker]

This revision was automatically updated to reflect the committed changes.
Closed by commit R235:864107a0184a: Fix broken url to API specification 
(authored by habacker).

REPOSITORY
  R235 Attica

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13714?vs=36629=36630

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

AFFECTED FILES
  README.md

To: habacker, mlaurent, leinir
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13714: Fix broken url to API specification

[Dan Leinir Turthra Jensen]

leinir accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R235 Attica

BRANCH
  master

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

To: habacker, mlaurent, leinir
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13714: Fix broken url to API specification

[Ralf Habacker]

habacker added a reviewer: mlaurent.

REPOSITORY
  R235 Attica

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

To: habacker, mlaurent
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13714: Fix broken url to API specification

[Ralf Habacker]

habacker updated this revision to Diff 36629.
habacker added a comment.


  - fixed author

REPOSITORY
  R235 Attica

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13714?vs=36628=36629

BRANCH
  master

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

AFFECTED FILES
  README.md

To: habacker
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13714: Fix broken url to API specification

[Ralf Habacker]

habacker created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
habacker requested review of this revision.

TEST PLAN
  checked link in browser

REPOSITORY
  R235 Attica

BRANCH
  master

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

AFFECTED FILES
  README.md

To: habacker
Cc: kde-frameworks-devel, michaelh, ngraham, bruns