D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

[Volker Krause]

vkrause added a comment.


  That should work, yes.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, 
ngraham, bruns, demsking, cullmann, sars

D14237: Make Konqi look good in HiDPI

[Nathaniel Graham]

ngraham updated this revision to Diff 40104.
ngraham added a comment.


  Un-simplify; the previous version was better

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14237?vs=40101&id=40104

BRANCH
  hidpi-konqi (branched from master)

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

AFFECTED FILES
  src/aboutkde.png
  src/aboutkde.svg
  src/kaboutkdedialog_p.cpp
  src/kxmlgui.qrc

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

D14237: Make Konqi look good in HiDPI

[Nathaniel Graham]

ngraham updated this revision to Diff 40101.
ngraham added a comment.


  Simplify even more (maybe this is too much simplification though?)

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14237?vs=40099&id=40101

BRANCH
  hidpi-konqi (branched from master)

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

AFFECTED FILES
  src/aboutkde.png
  src/aboutkde.svg
  src/kaboutkdedialog_p.cpp
  src/kxmlgui.qrc

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

D14237: Make Konqi look good in HiDPI

[Nathaniel Graham]

ngraham updated this revision to Diff 40099.
ngraham added a comment.


  Simplify, simplify

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14237?vs=40083&id=40099

BRANCH
  hidpi-konqi (branched from master)

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

AFFECTED FILES
  src/aboutkde.png
  src/aboutkde.svg
  src/kaboutkdedialog_p.cpp
  src/kxmlgui.qrc

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

D14957: Create a first example of the use of KMessageBox

[Olivier Churlaud]

This revision was automatically updated to reflect the committed changes.
Closed by commit R236:6748d24d5fb2: Create a first example of the use of 
KMessageBox (authored by ochurlaud).

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14957?vs=40094&id=40095

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

AFFECTED FILES
  CMakeLists.txt
  examples/CMakeLists.txt
  examples/kmessagebox/CMakeLists.txt
  examples/kmessagebox/main.cpp

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

D14957: Create a first example of the use of KMessageBox

[Olivier Churlaud]

ochurlaud marked an inline comment as done.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  master

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

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

D14957: Create a first example of the use of KMessageBox

[Olivier Churlaud]

ochurlaud updated this revision to Diff 40094.
ochurlaud added a comment.


  Fix as dfaure said

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14957?vs=40092&id=40094

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  examples/CMakeLists.txt
  examples/kmessagebox/CMakeLists.txt
  examples/kmessagebox/main.cpp

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

D14957: Create a first example of the use of KMessageBox

[David Faure]

dfaure accepted this revision.
dfaure added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> main.cpp:33
> +// Define a button
> +KGuiItem yesButton( QStringLiteral("Hello"), // the button label
> +QStringLiteral("view-filter"), // iconName, can be 
> empty to fallback to default

remove space after '('

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  master

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

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

D14955: Move example from techbase to own repo

[Olivier Churlaud]

This revision was automatically updated to reflect the committed changes.
Closed by commit R311:87a84ac866c5: Move example from techbase to own repo 
(authored by ochurlaud).

REPOSITORY
  R311 KWallet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14955?vs=40090&id=40093

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

AFFECTED FILES
  CMakeLists.txt
  examples/CMakeLists.txt
  examples/asynchronous_app/CMakeLists.txt
  examples/asynchronous_app/dialog.cpp
  examples/asynchronous_app/dialog.h
  examples/asynchronous_app/main.cpp

To: ochurlaud, #frameworks, valir, dfaure
Cc: dfaure, aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

D14957: Create a first example of the use of KMessageBox

[Olivier Churlaud]

ochurlaud updated this revision to Diff 40092.
ochurlaud added a comment.


  Added licences and add some changes to fit what was done on D14955 


REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14957?vs=40082&id=40092

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  examples/CMakeLists.txt
  examples/kmessagebox/CMakeLists.txt
  examples/kmessagebox/main.cpp

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

D14955: Move example from techbase to own repo

[David Faure]

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

REPOSITORY
  R311 KWallet

BRANCH
  master

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

To: ochurlaud, #frameworks, valir, dfaure
Cc: dfaure, aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

D14955: Move example from techbase to own repo

[David Faure]

dfaure added a comment.


  +1, OK, this works for me. Simpler indeed.

REPOSITORY
  R311 KWallet

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

To: ochurlaud, #frameworks, valir, dfaure
Cc: dfaure, aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

D14955: Move example from techbase to own repo

[Olivier Churlaud]

ochurlaud marked 4 inline comments as done.

REPOSITORY
  R311 KWallet

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

To: ochurlaud, #frameworks, valir, dfaure
Cc: dfaure, aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

D14955: Move example from techbase to own repo

[Olivier Churlaud]

ochurlaud updated this revision to Diff 40090.
ochurlaud added a comment.


  Added Licence, made it compile within the whole project and fix issues raised 
by dfaure.
  
  I think we shouldn't care about making the example self-standing. It's aimed 
at tutoring people to use the lib, not ECM nor CMake

REPOSITORY
  R311 KWallet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14955?vs=40069&id=40090

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  examples/CMakeLists.txt
  examples/asynchronous_app/CMakeLists.txt
  examples/asynchronous_app/dialog.cpp
  examples/asynchronous_app/dialog.h
  examples/asynchronous_app/main.cpp

To: ochurlaud, #frameworks, valir, dfaure
Cc: dfaure, aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

D14927: KConfig: handle directory symlinks correctly.

[Adriaan de Groot]

adridg added a comment.


  Without patch, all tests
  
  
  Totals: 44 passed, 2 failed, 0 skipped, 0 blacklisted, 225ms
  
  The two failed tests are:
  
  - testThreads
  - testDelete
  
  With patch, all tests
  =
  
  Totals: 45 passed, 1 failed, 0 skipped, 0 blacklisted, 229ms
  
  The one failed test is:
  
  - testDefaults
  
  Without patch, some tests
  =
  
  Some tests means run `HOME=/tmp/drop bin/kconfigtest testDelete  testThreads  
testDefaults`
  
* Start testing of KConfigTest *
Config: Using QtTest library 5.10.1, Qt 5.10.1 (x86_64-little_endian-lp64 
shared (dynamic) release build; by Clang 5.0.0 (tags/RELEASE_500/final 312559))
PASS   : KConfigTest::initTestCase()
FAIL!  : KConfigTest::testDelete() '!delgr.exists()' returned FALSE. ()
   Loc: [/home/adridg/src/kde/tier-1/kconfig/autotests/kconfigtest.cpp(802)]
PASS   : KConfigTest::testThreads()
FAIL!  : KConfigTest::testDefaults() Compared values are not the same
   Actual   (group.readEntry("entry1", QString())): "hello"
   Expected (Default) : "Default"
   Loc: [/home/adridg/src/kde/tier-1/kconfig/autotests/kconfigtest.cpp(399)]
PASS   : KConfigTest::cleanupTestCase()
  
  
  
  Without patch, some tests
  =
  
* Start testing of KConfigTest *
Config: Using QtTest library 5.10.1, Qt 5.10.1 (x86_64-little_endian-lp64 
shared (dynamic) release build; by Clang 5.0.0 (tags/RELEASE_500/final 312559))
PASS   : KConfigTest::initTestCase()
PASS   : KConfigTest::testDelete()
FAIL!  : KConfigTest::testThreads() Compared values are not the same
   Actual   (group.readEntry("entry1", QString())): "hello"
   Expected (Default) : "Default"
   Loc: [/zbigone/src/kde/tier-1/kconfig/autotests/kconfigtest.cpp(399)]
PASS   : KConfigTest::testDefaults()
PASS   : KConfigTest::cleanupTestCase()
Totals: 4 passed, 1 failed, 0 skipped, 0 blacklisted, 41ms
* Finished testing of KConfigTest *

REPOSITORY
  R237 KConfig

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

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

D14955: Move example from techbase to own repo

[David Faure]

dfaure added a comment.


  (or maybe the wiki was explicit about what the license was?)

REPOSITORY
  R311 KWallet

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

To: ochurlaud, #frameworks, valir, dfaure
Cc: dfaure, aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

D14955: Move example from techbase to own repo

[David Faure]

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


  And yes this needs a license, obviously. If the original author(s) can't be 
reached anymore, that means rewriting the example :(

INLINE COMMENTS

> CMakeLists.txt:3
> +
> +find_package(ECM NO_MODULE)
> +find_package(Qt5Widgets)

CONFIG looks less negative than the old "NO_MODULE"

> dialog.cpp:33
> +layout()->addWidget(explanation);
> +qobject_cast(layout())->addStretch();
> +layout()->addWidget(m_statusLabel);

Urgh, better have a QVBoxLayout* local variable.

> dialog.cpp:41
> +
> +connect(m_launchButton, SIGNAL(clicked()), SLOT(doSave()));
> +

new-style connect would be better

> dialog.h:16
> +public:
> +Dialog(QWidget *parent = 0);
> +

nullptr

REPOSITORY
  R311 KWallet

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

To: ochurlaud, #frameworks, valir, dfaure
Cc: dfaure, aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

D14955: Move example from techbase to own repo

[David Faure]

dfaure added a comment.


  Yes this needs to be compiled so it doesn't rot.
  The solution is obviously not add_subdirectory since that wouldn't catch 
missing find_packages etc.
  
  One way is to have cmake run cmake, like ki18n/autotests/CMakeLists.txt does 
to run cmake on the ki18n_install subdir.

REPOSITORY
  R311 KWallet

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

To: ochurlaud, #frameworks, valir
Cc: dfaure, aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

D13227: update mailto kioslave docbook

[Burkhard Lück]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:84436b0d4874: update mailto kioslave docbook (authored by 
lueck).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D13227?vs=35228&id=40086#toc

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13227?vs=35228&id=40086

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

AFFECTED FILES
  docs/kioslave5/mailto/index.docbook

To: lueck, #frameworks, #documentation, yurchor
Cc: kde-frameworks-devel, kde-doc-english, michaelh, ngraham, bruns, skadinna

Re: New framework: KF5Syndication

[David Faure]

On samedi 18 août 2018 15:38:48 CEST Volker Krause wrote:
> On Wednesday, 22 April 2015 21:44:05 CEST Daniel Vrátil wrote:
> > Hi all,
> > 
> > I'd like to ask for review of another Framework from kdepimlibs:
> > KF5Syndication
> > 
> > KF5Syndication is an RSS/Atom parsing library. It also provides API to
> > fetch
> > feeds directly from network.
> > 
> > It's a Tier 3 Framework (depends on KCodecs and KIO). AFAIK it's
> > currently being
> > used only by Akregator.
> > 
> > I would like to submit KF5Syndication for the standard 2 week review
> > period, and if everything is OK, then move it to Frameworks.
> 
> Now that 18.08 is done let's finally move this forward.
> 
> The KIO dependency has been refactored away, so this is now a tier 2
> functional framework.
> 
> Unless there are objections we'd like to move KF5Syndication from PIM to KF5
> before the 18.12 dependency freeze (ie. in to the KF5 September or October
> releases).

Works for me. Please request the move and the addition to Frameworks CI so it 
shows up on my radar :)

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

D14723: Add option to disable KWallet entirely in the new wallet dialog

[David Faure]

dfaure added a comment.


  The patch looks OK to me, but yeah, it should be emphasized that disabling 
KWallet *reduces* security greatly.

REPOSITORY
  R311 KWallet

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

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

D14955: Move example from techbase to own repo

[Albert Astals Cid]

aacid added a comment.


  In D14955#312287 , @ochurlaud 
wrote:
  
  > In D14955#312279 , @aacid wrote:
  >
  > > -1
  > >
  > > The example is not getting compiled it will rot
  >
  >
  > I'm not sure to understand: it *can* be compiled, whereas on the current 
status (in the wiki) it will never be ! Maybe the next step would be to add a 
trigger from the make test command?
  
  
  It can be compiled, but it is not.
  
  So it will rot, we've had lots of cases of this, if the code is not compiled 
every single time on the CI, it will eventually not compile.

REPOSITORY
  R311 KWallet

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

To: ochurlaud, #frameworks, valir
Cc: aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D14955: Move example from techbase to own repo

[Olivier Churlaud]

ochurlaud added a comment.


  In D14955#312279 , @aacid wrote:
  
  > -1
  >
  > The example is not getting compiled it will rot
  
  
  I'm not sure to understand: it *can* be compiled, whereas on the current 
status (in the wiki) it will never be ! Maybe it the next step would be to add 
a trigger from the make test command?

REPOSITORY
  R311 KWallet

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

To: ochurlaud, #frameworks, valir
Cc: aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D14955: Move example from techbase to own repo

[Albert Astals Cid]

aacid added a comment.


  -1
  
  The example is not getting compiled it will rot

REPOSITORY
  R311 KWallet

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

To: ochurlaud, #frameworks, valir
Cc: aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

[Tutorials] Proposition of tutorial organization

[Olivier Churlaud]

Dear framework maintainers,

Hidden somewhere on techbase.kde.org, one can find this: 
https://techbase.kde.org/Development/Tutorials

Not everything is up-to-date nor at the right level.  It's often too complex, 
not showing fully that Framework libs can be used alone (currently it seems 
that if you take one, you need the whole stack).

What I would propose is to move theses tutorials to each repo : for instance 
the tutorials concerning KWallet would be in the kwallet repo, under the 
examples folder. I made a review request here:
https://phabricator.kde.org/D14955

Several examples/tutorials can be under each lib. When this exists, the code 
can be used to enhance the API documentation thanks to magic  command "\snippet 
 " [1]
It would be simple to obtain such a tutorial [2] but more easy to find, and 
always up to date.

Please add me in copy if you answer as I'm not part of the mailing list;

Cheers
Olivier

[1] 
https://community.kde.org/Guidelines_and_HOWTOs/API_Documentation#Nice_way:_.5Csnippet
[2] https://techbase.kde.org/Development/Tutorials/First_program

D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

[Christoph Cullmann]

cullmann added a comment.


  Just for the implementation: Is the static default than just a e.g. function 
static QExplicitlySharedDataPointer(new FormatPrivate()) like:
  
  QExplicitlySharedDataPointer &staticDefault()
  {
  
static QExplicitlySharedDataPointer default(new 
FormatPrivate());
return default;
  
  }
  
  and the constructor will do
  
  Format::Format() : d(staticDefault())
  {
  }
  
  or is that some other trick?

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, 
ngraham, bruns, demsking, cullmann, sars

D14237: Make Konqi look good in HiDPI

[Nathaniel Graham]

ngraham updated this revision to Diff 40083.
ngraham added a comment.


  Rebase on current master

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14237?vs=40081&id=40083

BRANCH
  hidpi-konqi (branched from master)

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

AFFECTED FILES
  src/aboutkde.png
  src/aboutkde.svg
  src/kaboutkdedialog_p.cpp
  src/kxmlgui.qrc

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

D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

[Volker Krause]

vkrause added a comment.


  I have no strong preference for either of those approaches, but I do agree 
with this optimization of course :)

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, 
ngraham, bruns, demsking, cullmann, sars

D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

[Christoph Cullmann]

cullmann added a comment.


  If that is more elegant, I would be happy with this a solution, too.
  The Format() allocations just showed up as a few MBs during heaptrack 
profiling of KWrite (just loading one large file and rehighlighting it).

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, 
ngraham, bruns, demsking, cullmann, sars

D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

[Volker Krause]

vkrause added a comment.


  A shared static null object should work with QExplicitlySharedDataPointer as 
with any other d pointer type: just copy the null object into d in the default 
ctor. Trades the conditions per call for an atomic increment/decrement per 
instance.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, 
ngraham, bruns, demsking, cullmann, sars

D14957: Create a first example of the use of KMessageBox

[Olivier Churlaud]

ochurlaud added a reviewer: Frameworks.

REPOSITORY
  R236 KWidgetsAddons

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

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

D14957: Create a first example of the use of KMessageBox

[Olivier Churlaud]

ochurlaud created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ochurlaud requested review of this revision.

REVISION SUMMARY
  Based on https://techbase.kde.org/Development/Tutorials/First_program

TEST PLAN
  It compiles

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  master

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

AFFECTED FILES
  examples/kmessagebox/CMakeLists.txt
  examples/kmessagebox/main.cpp

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

D14237: Make Konqi look good in HiDPI

[Nathaniel Graham]

ngraham added a reviewer: bruns.

REPOSITORY
  R263 KXmlGui

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

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

D14237: Make Konqi look good in HiDPI

[Nathaniel Graham]

ngraham updated this revision to Diff 40081.
ngraham added a comment.


  ...Actually add the SVG image :)

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14237?vs=40080&id=40081

BRANCH
  hidpi-konqi (branched from master)

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

AFFECTED FILES
  src/aboutkde.png
  src/aboutkde.svg
  src/kaboutkdedialog_p.cpp
  src/kxmlgui.qrc

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

D14237: Make Konqi look good in HiDPI

[Nathaniel Graham]

ngraham edited the summary of this revision.
ngraham edited the test plan for this revision.

REPOSITORY
  R263 KXmlGui

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

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

D14237: Make Konqi look good in HiDPI

[Nathaniel Graham]

ngraham updated this revision to Diff 40080.
ngraham added a comment.


  Use @bruns' awesome SVG

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14237?vs=38367&id=40080

BRANCH
  hidpi-konqi (branched from master)

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

AFFECTED FILES
  src/aboutkde.png
  src/kaboutkdedialog_p.cpp
  src/kxmlgui.qrc

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

D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

[Christoph Cullmann]

cullmann added a comment.


  I am a QExplicitlySharedDataPointer noob ;=) Perhaps Volker can tell about 
that ;=)

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, 
ngraham, bruns, demsking, cullmann, sars

D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

[Dominik Haumann]

dhaumann added a comment.


  Btw, would it also work to point to an internal static d-pointer to avoid all 
the d-pointer checking? The current patch is likely fine, but it feels a bit 
messy... But I'd agree to this if other solutions are not feasible.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, 
ngraham, bruns, demsking, cullmann, sars

D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

[Christoph Cullmann]

cullmann added a comment.


  But I can call all accessors on some default constructed Format() if that 
increases the confidence, if this patch is at all wanted.
  It saves heap allocations but adds some branches.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, 
ngraham, bruns, demsking, cullmann, sars

D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

[Christoph Cullmann]

cullmann added a comment.


  There is no test missing, these code paths are anyways already tested.
  ;=) Otherwise I would not have found all segfaults.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, 
ngraham, bruns, demsking, cullmann, sars

D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

[Dominik Haumann]

dhaumann added a comment.


  Unit test missing ...

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, 
ngraham, bruns, demsking, cullmann, sars

D14723: Add option to disable KWallet entirely in the new wallet dialog

[Fabian Vogt]

fvogt added inline comments.

INLINE COMMENTS

> aacid wrote in knewwalletdialog.h:67
> do we really need to rename this?

It's not just a rename - it's doing the opposite now as there are three instead 
of two actions.

If you follow which slot this ends up, it's responsible for switching between 
"Next" and "Finished" buttons in the step. Next is only shown if GPG is 
selected, so this makes IMO more sense.

REPOSITORY
  R311 KWallet

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

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

D14723: Add option to disable KWallet entirely in the new wallet dialog

[Fabian Vogt]

fvogt updated this revision to Diff 40079.
fvogt added a comment.


  Quit kwalletd if option was selected.
  
  The .sync is necessary because the change is made in a KSharedConfig but read
  by the simple KConfig in reconfigure. I guess it should use KSharedConfig
  everywhere and just call "reparseConfiguration" in reconfigure manually. 
That's
  something for another patch however.

REPOSITORY
  R311 KWallet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14723?vs=39396&id=40079

BRANCH
  disableopt

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

AFFECTED FILES
  src/runtime/kwalletd/knewwalletdialog.cpp
  src/runtime/kwalletd/knewwalletdialog.h
  src/runtime/kwalletd/knewwalletdialogintro.ui
  src/runtime/kwalletd/kwalletd.cpp

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

D14893: [recentdocuments:/] Filter out files that can't be browsed with a file manager

[Nathaniel Graham]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R320:8606d4d97f3e: [recentdocuments:/] Filter out files that 
can't be browsed with a file manager (authored by ngraham).

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14893?vs=40054&id=40074

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

AFFECTED FILES
  recentdocuments/recentdocuments.cpp

To: ngraham, broulik, #dolphin, #frameworks
Cc: elvisangelaccio, markg

D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

[Christoph Cullmann]

cullmann created this revision.
cullmann added a reviewer: vkrause.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
cullmann requested review of this revision.

REVISION SUMMARY
  Format() is used as invalid return value in some places.
  At the moment that does at least one heap allocation.
  Now only for formats loaded from the XML a heap allocation is done

TEST PLAN
  make && make test

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

AFFECTED FILES
  src/lib/format.cpp
  src/lib/format_p.h

To: cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D14955: Move example from techbase to own repo

[Elvis Angelaccio]

elvisangelaccio added a comment.


  +1
  
  Do we need a license for the example source files?

REPOSITORY
  R311 KWallet

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

To: ochurlaud, #frameworks, valir
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D14955: Move example from techbase to own repo

[Olivier Churlaud]

ochurlaud added reviewers: Frameworks, valir.

REPOSITORY
  R311 KWallet

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

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

D14955: Move example from techbase to own repo

[Olivier Churlaud]

ochurlaud created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ochurlaud requested review of this revision.

REVISION SUMMARY
  Add example based on https://techbase.kde.org/Development/Tutorials/KWallet 
so that it can be kept up-to-date. The techbase tutoriel can be archived and 
not maintained anymore.

TEST PLAN
  It compiles

REPOSITORY
  R311 KWallet

BRANCH
  master

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

AFFECTED FILES
  examples/asynchronous_app/CMakeLists.txt
  examples/asynchronous_app/dialog.cpp
  examples/asynchronous_app/dialog.h
  examples/asynchronous_app/main.cpp

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

D14952: Fix includedDefinitions, handle definition change in context switch

[Christoph Cullmann]

cullmann added a comment.


  If we want a nice order, we should sort them at the end like the repo does.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, genethomas, kevinapavew, 
ngraham, bruns, demsking, cullmann, vkrause, sars, dhaumann

D14952: Fix includedDefinitions, handle definition change in context switch

[Dominik Haumann]

dhaumann added a comment.


  Looks ok to me. Only thing is: with takeLast() you now changed the order of 
the IncludedDefinitions compared to the previous version. This is of course ok, 
but in the color config dialog the order is now different than before. Whatever 
this means... I liked that Modelines and Alert were rather late in the list - 
on the other hand that was not guaranteed before either... ;)

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, genethomas, kevinapavew, 
ngraham, bruns, demsking, cullmann, vkrause, sars, dhaumann

D14952: Fix includedDefinitions, handle definition change in context switch

[Christoph Cullmann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:9cfb8fa9a684: Fix includedDefinitions, handle definition 
change in context switch… (authored by cullmann).

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14952?vs=40060&id=40067

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  src/lib/definition.cpp

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, genethomas, kevinapavew, 
ngraham, bruns, demsking, cullmann, vkrause, sars, dhaumann

D14952: Fix includedDefinitions, handle definition change in context switch

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, genethomas, kevinapavew, 
ngraham, bruns, demsking, cullmann, vkrause, sars, dhaumann

D7423: [WIP/assistance needed] Populate UDS_CREATION_TIME on Linux if statx system call is available

[Rolf Eike Beer]

dakon added inline comments.

INLINE COMMENTS

> CMakeLists.txt:14
> +  check_function_exists(statx HAVE_STATX)
> +  set(HAVE_STATX ${HAVE_STATX})
>  endif()

This line looks needless.

> file.cpp:86
> +#include 
> +// 332 on my system
> +#define STAT(path, buf) 
> syscall(__NR_statx,(AT_FDCWD),(path),(AT_SYMLINK_NOFOLLOW),(0),(buf))

This comment isn't needed either.

> file.cpp:950
>  #endif
> +#ifdef stx_btime
> +/* As above, but it's called "stx_btime" in Linux kernel 4.11+

Where would that define come from? At least on glibc 2.28 I see it nowhere 
defined.

REPOSITORY
  R241 KIO

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

To: ngraham, dfaure, broulik, elvisangelaccio, #frameworks, #dolphin, fvogt
Cc: dakon, fvogt, kde-frameworks-devel, bruns, meven, ltoscano, #frameworks, 
michaelh, ngraham

D14952: Fix includedDefinitions, handle definition change in context switch

[Volker Krause]

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

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, genethomas, kevinapavew, 
ngraham, bruns, demsking, cullmann, vkrause, sars, dhaumann

D14927: KConfig: handle directory symlinks correctly.

[Adriaan de Groot]

adridg added a comment.


  This shows up in the unit tests. Whether it has any effect in real life is 
unknown. FreeBSD often -- sometimes, maybe, depending on FS setup and layout -- 
has /home -> /usr/home or /home -> usr/home, and of course there could be weird 
user setups as well where .cache is symlinked to a different location with more 
disk space. In any case, this is triggering unit-test failures in the CI, so 
cleaning it up to consistently compare the same kind of filename is a good 
thing. I'll give this a test on my home system and give a shout when I have.

REPOSITORY
  R237 KConfig

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

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

D14952: Fix includedDefinitions, handle definition change in context switch

[Christoph Cullmann]

cullmann added a comment.


  kate *.* in the autotests/input folder works now without any crash, too.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, genethomas, kevinapavew, 
ngraham, bruns, demsking, cullmann, vkrause, sars, dhaumann

D14952: Fix includedDefinitions, handle definition change in context switch

[Christoph Cullmann]

cullmann created this revision.
cullmann added reviewers: vkrause, dhaumann.
cullmann added projects: Framework: Syntax Highlighting, Frameworks.
Herald added a project: Kate.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
cullmann requested review of this revision.

REVISION SUMMARY
  Fix includedDefinitions, handle definition change in context switch.
  They were missing, see PHP/HTML.
  I cleaned up the code a bit to just use QVector, no need for a queue, the 
order is anyways arbitrary (but stable).

TEST PLAN
  make && make test
  
  https://bugs.kde.org/show_bug.cgi?id=397659 works now

REPOSITORY
  R216 Syntax Highlighting

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  src/lib/definition.cpp

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, genethomas, kevinapavew, 
ngraham, bruns, demsking, cullmann, vkrause, sars, dhaumann

D14893: [recentdocuments:/] Filter out files that can't be browsed with a file manager

[Elvis Angelaccio]

elvisangelaccio added a comment.


  +1, lgtm

REPOSITORY
  R320 KIO Extras

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

To: ngraham, broulik, #dolphin, #frameworks
Cc: elvisangelaccio, markg

D14893: [recentdocuments:/] Filter out files that can't be browsed with a file manager

[Nathaniel Graham]

ngraham added a comment.


  In D14893#311411 , @markg wrote:
  
  > It "looks" oke to me, but i don't know this code one bit.
  
  
  Give it a try, it's pretty easy to test with the test plan.

REPOSITORY
  R320 KIO Extras

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

To: ngraham, broulik, #dolphin, #frameworks
Cc: markg

D14893: [recentdocuments:/] Filter out files that can't be browsed with a file manager

[Nathaniel Graham]

ngraham updated this revision to Diff 40054.
ngraham added a comment.


  Move comment

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14893?vs=39892&id=40054

BRANCH
  arcpatch-D14893

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

AFFECTED FILES
  recentdocuments/recentdocuments.cpp

To: ngraham, broulik, #dolphin, #frameworks
Cc: markg

D14893: [recentdocuments:/] Filter out files that can't be browsed with a file manager

[Nathaniel Graham]

ngraham marked an inline comment as done.

REPOSITORY
  R320 KIO Extras

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

To: ngraham, broulik, #dolphin, #frameworks
Cc: markg

D10937: Retouching of Screen Layout Selection OSD Icons

[Nathaniel Graham]

ngraham added a subscriber: gladhorn.
ngraham added a comment.


  In D10937#311297 , @bruns wrote:
  
  > I think this works better if you actually rotate the screen:
  >  F6203935: rotate2.png 
  
  
  In KScreen, we recently did something similar and just used fully-rotated 
versions:
  
  F6206188: Rotation.png 
  
  This works well because when the screen is physically rotated already, it 
becomes really easy to select the right choice because only one of them has an 
icon that looks like it's oriented correctly. We might want to consider doing 
the same here. So the icons that depict rotation wouldn't need to have rotation 
arrows or windows that were turned at funny angles; they'd simply be depict the 
final rotation state. In which case, the "base" of the screen should probably 
show up on the side that the display has been rotated towards, too.
  
  As for moving the icons into the Plasma theme, I approve, but I think that 
should be done //first// and in a separate commit/patch. (The location is 
https://cgit.kde.org/plasma-framework.git/tree/src/desktoptheme/breeze/icons).So
 you would submit one patch to move the icons into the plasma theme, then a 
second to remove them here (which is safe because `breeze-icons` and 
`plasma-frameworks` have the same release schedules), and then in a third 
patch, you'd replace the existing icons with your awesome new ones.
  
  Does that sound like a reasonable plan? Please let me know how I can help you 
with that. If we're going to make the move, we need to do it fairly soon so 
that both changes (moving to plasma theme and also improving the icons) land in 
KDE Frameworks 5.50, which is going to be the version that Plasma 5.14 depends 
on.

REPOSITORY
  R266 Breeze Icons

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

To: pstefan, #breeze, ngraham
Cc: gladhorn, broulik, bruns, #vdg, kde-frameworks-devel, emateli, abetts, 
mart, ngraham, michaelh, crozbo, firef, skadinna, aaronhoneycutt, mbohlender

D14724: autotests: don't fail if an unrelated window shows up.

[David Faure]

dfaure added reviewers: broulik, davidedmundson.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: dfaure, hein, drosca, broulik, davidedmundson
Cc: kde-frameworks-devel, #plasma, michaelh, ngraham, bruns

D14946: [DialogShadows] Use 0 offset for disabled borders on Wayland

[Vlad Zagorodniy]

zzag created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
zzag requested review of this revision.

REVISION SUMMARY
  On Wayland, DialogShadows doesn't attach empty tiles for disabled
  borders. Yet, it sets offset of 1 for them. This results in visual
  artifacts like this
  
  F6206088: before.png 
  
  //KWin thinks that there is 1px tall shadow in the bottom.//
  
  In general we don't need to pass empty shadow tiles for disabled
  borders. Thus, that's fine to set 0 offset for them.

TEST PLAN
  - Started kwin_wayland:
  
dbus-launch kwin_wayland --windowed --xwayland
  
  - Launched latte-dock and kate:
  
unset DISPLAY QT_QPA_PLATFORM=wayland latte-dock & QT_QPA_PLATFORM=wayland 
kate
  
  - Opened calendar popup:
  
  F6206090: after.png 

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  wayland-shadows

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

AFFECTED FILES
  src/plasmaquick/dialogshadows.cpp

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

D14779: Fix 'kformattest fails with installed kcoreaddons language package'

[Ralf Habacker]

habacker added a comment.


  Thanks for this pointer - with this setting the german translation is not 
fetched anymore.
  
  But there is still the question open how to deal with plurals with and 
without installed kcoreaddons language package.
  
  I guess running tests without kcoreaddons-lang installed will be performed at 
KDE CI and running tests with kcoreaddons language package mainly happens on 
developers desktop systems, where KF5 is installed and a developer is working 
on kcoreaddons.
  
  From what I read on Qt doc ui string with plurals always need to use a 
translation string with "%n millisecond(s)"  (cleaned with  D14940 
), but in the test code there are two cases
  
QCOMPARE(format.formatDecimalDuration(1 * MSecsInDay + 10 * MSecsInHour, 
3), QStringLiteral("1.417 days"));
  
  This one requires to have a translation installed, which translates "day(s)" 
to "days"
  
  and this one
  
QCOMPARE(format.formatDecimalDuration(10), QStringLiteral("10 
millisecond(s)"));
  
  This fails with a translation installed because an installed english 
translation translates this to "10 milliseconds"
  
  How to proceed ?

REPOSITORY
  R244 KCoreAddons

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

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

D14940: kformattest: Use plural suffix (s) consequently

[Ralf Habacker]

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


  - type fix in KFormatTest::formatDecimalDuration
  - missing plural fix in KFormatPrivate::formatDecimalDuration

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14940?vs=40027&id=40038

BRANCH
  master

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

AFFECTED FILES
  autotests/kformattest.cpp
  src/lib/util/kformatprivate.cpp

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

D14779: Fix 'kformattest fails with installed kcoreaddons language package'

[David Faure]

dfaure added a comment.


  Use qputenv instead of setenv.
  
  Example:
  kio/autotests/kdirmodeltest.cpp:qputenv("LC_ALL", "en_US.UTF-8");

REPOSITORY
  R244 KCoreAddons

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

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

D14779: Fix 'kformattest fails with installed kcoreaddons language package'

[Ralf Habacker]

habacker added a comment.


  In D14779#311511 , @aacid wrote:
  
  > Can you please replace your setLocale call in initTestCase for
  >
  >   void initLocale()
  >   {
  > setenv("LC_ALL", "en_US.utf-8", 1);
  
  
  Just to inform you: this will not work with msvc 
https://stackoverflow.com/questions/17258029/c-setenv-undefined-identifier-in-visual-studio
 and adds an additional 'kdewin' dependency to windows builds
  
  >   }
  >   Q_CONSTRUCTOR_FUNCTION(initLocale)
  >
  > 
  > This should make it work (or at least it does here)
  
  Just recognized  general issues in kcoreaddons translation, which need to be 
fixed before - please accept D14940 .
  
  Then I will rebase this patch and check your suggestion.

REPOSITORY
  R244 KCoreAddons

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

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