D23579: WIP: port ftp slave to new error reporting system

2019-09-20 Thread Pino Toscano 
pino added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in ftp.h:49
> That's BIC we cannot remove, reorder nor insert parent(s)

> That's BIC we cannot remove, reorder nor insert parent(s)

This is a kioslave, built as plugin. There is no such thing as BC for this kind 
of stuff.

REPOSITORY
  R241 KIO

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

To: sitter, dfaure
Cc: pino, anthonyfieroni, dfaure, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, ngraham, bruns

D23579: WIP: port ftp slave to new error reporting system

2019-09-20 Thread Anthony Fieroni 
anthonyfieroni added inline comments.

INLINE COMMENTS

> ftp.h:49
>  
> //===
> -class Ftp : public QObject, public KIO::SlaveBase
>  {

That's BIC we cannot remove, reorder nor insert parent(s)

> ftp.h:167
> +
> +class FtpInternal : public QObject
> +{

Should be hidden?

REPOSITORY
  R241 KIO

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

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

D23667: Add == and != operators to KIO::UDSEntry

2019-09-20 Thread David Faure 
dfaure added a comment.


  Note that there is `KFileItem::cmp` for a more thorough comparison of two 
fileitems. But it only looks at some UDS fields -- those that matter at the 
KDirLister level.
  Or maybe it shows that UDSEntry::operator== was missing so KFileItem::cmp 
uses a poor man's version of that ;)
  This confirms the need for such an operator== indeed (though I wouldn't 
change the implementation of KFileItem::cmp until we see a need to do that).
  
  Thanks for the explanation.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D23667

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

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

D23579: WIP: port ftp slave to new error reporting system

2019-09-20 Thread David Faure 
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Nice work!

INLINE COMMENTS

> ftptest.cpp:33
> +public:
> +QUrl url(const QString )
> +{

this method should be const

> ftptest.cpp:50
> +// works around the fact that kioslave would load the slave from the 
> system
> +// as first choice instead of the one from teh build dir.
> +qputenv("QT_PLUGIN_PATH", 
> QCoreApplication::applicationDirPath().toUtf8());

typo: teh -> the

> ftptest.cpp:53
> +
> +// Run ftpd to talk t.
> +QVERIFY(m_remoteDir.isValid());

"talk t" ? missing 'o'?

> ftptest.cpp:61
> +QVERIFY(m_daemonProc.waitForStarted());
> +QVERIFY(m_daemonProc.state() == QProcess::Running);
> +// Wait for the daemon to print its port. That tells us both where 
> it's listening

QCOMPARE so we see the state if it fails

> ftptest.cpp:97
> +{
> +Q_ASSERT(m_daemonProc.state() == QProcess::Running);
> +}

QCOMPARE

> ftptest.cpp:114
> +job->setUiDelegate(nullptr);
> +QVERIFY(job->exec());
> +QCOMPARE(job->data(), data);

QVERIFY2(job->exec(), qUtf8Printable(job->errorString()));

> ftptest.cpp:167
> +job->setUiDelegate(nullptr);
> +QEXPECT_FAIL("", "Trying to write to an inacessible path fails.", 
> Continue);
> +QVERIFY2(job->exec(), qUtf8Printable(job->errorString()));

QEXPECT_FAIL is for known bugs.

Here, what you want is

  QVERIFY(!job->exec());

> ftptest.cpp:180
> +inaccessibleUrl.setPassword("password");
> +const QString remoteInaccesiblePath = m_remoteDir.path() + 
> inaccessiblePath;
> +QVERIFY(QFile::copy(QFINDTESTDATA("ftp/testCopy1"),

missing a 's' in Inaccessible

I'm confused whether this test is about inaccessible path or something around 
resuming.

> ftptest.cpp:186
> +job->setUiDelegate(nullptr);
> +QEXPECT_FAIL("", "Trying to write with bad quot doesn't work.", 
> Continue);
> +QVERIFY2(job->exec(), qUtf8Printable(job->errorString()));

what's bad quot?

Is this test about a bug, or the job failing is what we want?

> ftptest.cpp:206
> +job->setUiDelegate(nullptr);
> +QVERIFY(job->exec());
> +QCOMPARE(job->error(), 0);

I should ask for a clazy rule about QVERIFY(job->exec()) :-)
[same as above, QVERIFY2...]

> ftptest.cpp:217
> +
> +// FIXME: Actually BROKEN
> +

Now *that* is what QEXPECT_FAIL is about :-)

You can re-enable the code, and use QEXPECT_FAIL above the failing QVERIFY or 
QCOMPARE.

> ftp.cpp:671
> +if (!result.success) {
> +return result;
> +}

I don't think we want to return on failure here.
If the server doesn't support this command, we might as well try proceeding.
That's why the old code actually ignored errors here.

> ftp.cpp:696
>  qCDebug(KIO_FTP) << "Searching for pwd";
> -if (!ftpSendCmd(QByteArrayLiteral("PWD")) || (m_iRespType != 2)) {
> +result = ftpSendCmd(QByteArrayLiteral("PWD"));
> +if (!result.success || (m_iRespType != 2)) {

The reuse of the `result` var irks me a bit.
const vars rock :-)

Would it be a bad idea to make Result implicitly convertible to bool, so you 
can keep doing this inside the if? ` if (!ftpSendCmd(QByteArrayLiteral("PWD")) 
|| (m_iRespType != 2)) {` would still work fine then.

This would simplify all cases where we test for failures, but ignore the actual 
error message on failure (because we'll provide a more high-level one, like 
here). In other cases, we indeed need a result var.

> ftp.cpp:801
> +const auto result = ftpOpenConnection(LoginMode::Defered);
> +#pragma message "we ignore success of the new command its unclear if that is 
> intentional"
> +if (result.success) {

Oh... it's a recursive call... wow, tricky.

Yeah, that means this method returns false even if the nested method managed to 
login. Very confusing, and probably wrong...

> ftp.cpp:828
>  }
> -return false;
> +return Result::fail(); // else we don't know what went 
> wrong
>  }

I think this is all quite redundant.

openConnection() sets m_bLoggedOn to true if it succeeds, and to false if it 
fails.
So the ret val from openConnection and m_bLoggedOn are the same thing.
It makes little sense to test both independently and then even have a third 
case of failure in this line.
It's all the same, I suggest to simplify this.

I think this can all be simplified to

  if (!openResult.success) {
  if (m_control) { // ...
  closeConnection();
  }
  return openResult; // or Result::fail(ERR_CANNOT_LOGIN, m_host), if the 
error code from openResult is no good
  }

> ftp.cpp:1120
>  errormessage = m_host;
> -return false;
> +return Result::fail();
>  }

why not use errorcode and errormessage here?
or in fact just removing that line,

D24113: xcf: Properly read image resolution

2019-09-20 Thread Christoph Feck 
cfeck added a comment.


  That's bug 362484, please close. Merci!

REPOSITORY
  R287 KImageFormats

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

To: aacid, cfeck, apol, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D12992: New elisa icon

2019-09-20 Thread Noah Davis 
ndavis added a comment.


  icons/apps/22/elisa.svg does not apply cleanly. Not sure why.

REPOSITORY
  R266 Breeze Icons

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

To: lshoravi, #vdg, ngraham, #elisa
Cc: trickyricky26, ndavis, KonqiDragon, abetts, paullesur, januz, mgallien, 
alex-l, andreask, kde-frameworks-devel, mfraser, mnesbitt, LeGast00n, 
carneirogustavo, jguidon, ctakano, Tizon, oussemabouaneni, ashwind, 
fbampaloukas, GB_2, sourabhboss, aureliencouderc, tgraves, hantzv, lcmscheid, 
nhuisman, ursjoss, mykolak, jussiv, michaelh, astippich, James, ngraham, bruns, 
kmf, lemuel

KDE CI: Frameworks » kimageformats » kf5-qt5 FreeBSDQt5.13 - Build # 11 - Still Unstable!

2019-09-20 Thread CI System 
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kimageformats/job/kf5-qt5%20FreeBSDQt5.13/11/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Fri, 20 Sep 2019 21:18:48 +
 Build duration:
1 min 24 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 13 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: projectroot.autotests.kimageformats_read_hdr

KDE CI: Frameworks » kimageformats » kf5-qt5 FreeBSDQt5.13 - Build # 9 - Still Unstable!

2019-09-20 Thread CI System 
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kimageformats/job/kf5-qt5%20FreeBSDQt5.13/9/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Fri, 20 Sep 2019 20:41:19 +
 Build duration:
46 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 13 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: projectroot.autotests.kimageformats_read_hdr

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

2019-09-20 Thread CI System 
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kimageformats/job/kf5-qt5%20FreeBSDQt5.13/10/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Fri, 20 Sep 2019 20:42:07 +
 Build duration:
41 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 13 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: projectroot.autotests.kimageformats_read_hdr

D24113: xcf: Properly read image resolution

2019-09-20 Thread Albert Astals Cid 
aacid closed this revision.

REPOSITORY
  R287 KImageFormats

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

To: aacid, cfeck, apol, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24114: xcf: Fix regression when reading files with "unsupported" properties

2019-09-20 Thread Albert Astals Cid 
aacid closed this revision.

REPOSITORY
  R287 KImageFormats

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

To: aacid, cfeck, apol, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23667: Add == and != operators to KIO::UDSEntry

2019-09-20 Thread Méven Car 
meven added a comment.


  In D23667#535222 , @dfaure wrote:
  
  > Looks ok, but I'm just curious about the use case. "I felt it was missing" 
doesn't sound as strong an argument as "I need this"...
  >
  > Slaves are supposed to mostly create those, not compare them, and apps are 
supposed to use KFileItem rather than UDSEntry directly.
  
  
  The use case I felt I might need it, is when running stats for a file which I 
already stated earlier and have a KFileItem already.
  And checking if the file changed between the two stats.
  Without this it would be hard to do.
  
  Using the entry to compare the KFileItem seemed to me the simplest course of 
action (KFileItem equals compares only file path) but lacked the equal 
operators.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D23667

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

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

D23692: kdesu: set kernel flags to prevent ptrace instead of relying on setgid

2019-09-20 Thread Fabian Vogt 
fvogt added a reviewer: Frameworks.

REPOSITORY
  R299 KDESu

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

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

D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-20 Thread Nathaniel Graham 
This revision was automatically updated to reflect the committed changes.
Closed by commit R263:93455f78d78d: Move Full Screen Mode item from 
Settings menu to View menu (authored by ngraham).

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24091?vs=66541=66554

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

AFFECTED FILES
  src/ui_standards.rc

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

D24113: xcf: Properly read image resolution

2019-09-20 Thread Volker Krause 
vkrause accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R287 KImageFormats

BRANCH
  master

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

To: aacid, cfeck, apol, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24114: xcf: Fix regression when reading files with "unsupported" properties

2019-09-20 Thread Volker Krause 
vkrause accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R287 KImageFormats

BRANCH
  master

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

To: aacid, cfeck, apol, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24076: add a simple smoke test for slaves by using kioclient5

2019-09-20 Thread David Faure 
dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R320 KIO Extras

BRANCH
  smoketests

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

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

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.13 - Build # 96 - Still Unstable!

2019-09-20 Thread CI System 
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/96/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Fri, 20 Sep 2019 16:01:40 +
 Build duration:
8 min 13 sec and counting
   JUnit Tests
  Name: projectroot Failed: 3 test(s), Passed: 49 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_kmountpointtestFailed: projectroot.autotests.kiowidgets_kdirlistertestFailed: projectroot.autotests.kiowidgets_kdirmodeltestName: projectroot.autotests Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)Name: projectroot.src.ioslaves.trash Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.src.kpasswdserver Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)

D24113: xcf: Properly read image resolution

2019-09-20 Thread Albert Astals Cid 
aacid created this revision.
aacid added reviewers: cfeck, apol.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
aacid requested review of this revision.

REVISION SUMMARY
  QDataStream reads 64 bits when reading into a float unless you tell it to use 
SinglePrecision,
  since floats in xcf are 32 bit, do that

REPOSITORY
  R287 KImageFormats

BRANCH
  master

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

AFFECTED FILES
  src/imageformats/xcf.cpp

To: aacid, cfeck, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24003: kwaylandScanner produce version enum per class

2019-09-20 Thread Roman Gilg 
romangg added a comment.


  By the way I guess the reason why only the 
`PrimarySelectionOfferV1InterfaceVersion` is there without the patch is that 
the other interfaces are created through the manager on request of the client 
while the offer is created server-side and then sent through an event to client.

REPOSITORY
  R127 KWayland

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

To: gladhorn, #kwin
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D21897: Address some issues reported by Krazy analysis

2019-09-20 Thread Tommy Lincoln 
pajamapants3000 added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in krichtextwidget.cpp:638
> Ah of course. I didn't notice it was `exec()` that was being called.

Coming back after two months, even I thought this looked wrong at first. I will 
endeavor to make my intent clearer, thank you :)

> broulik wrote in ktextedit.cpp:373
> You can do `dialog->setAttribute(Qt::WA_DeleteOnClose);` when creating the 
> dialog instead of an explicit delete call

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

REPOSITORY
  R310 KTextWidgets

BRANCH
  krazy-50b2564 (branched from master)

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

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

D24114: xcf: Fix regression when reading files with "unsupported" properties

2019-09-20 Thread Albert Astals Cid 
aacid created this revision.
aacid added reviewers: cfeck, apol.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
aacid requested review of this revision.

REVISION SUMMARY
  The fact that we don't know the property is most of the times not fatal,
  so what we have to do is just "skip" the property and hope for the best
  
  BUGS: 411327

REPOSITORY
  R287 KImageFormats

BRANCH
  master

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

AFFECTED FILES
  src/imageformats/xcf.cpp

To: aacid, cfeck, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-20 Thread Nathaniel Graham 
ngraham edited the test plan for this revision.

REPOSITORY
  R263 KXmlGui

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

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

D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-20 Thread Nathaniel Graham 
ngraham updated this revision to Diff 66541.
ngraham added a comment.


  Add safe extra separator

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24091?vs=66484=66541

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

AFFECTED FILES
  src/ui_standards.rc

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

D24003: kwaylandScanner produce version enum per class

2019-09-20 Thread Roman Gilg 
romangg added a comment.


  Header output with the patch:
  
#ifndef KWAYLAND_SERVER_WP_PRIMARY_SELECTION_UNSTABLE_V1_H
#define KWAYLAND_SERVER_WP_PRIMARY_SELECTION_UNSTABLE_V1_H

#include "global.h"
#include "resource.h"

#include 

namespace KWayland
{
namespace Server
{

class Display;

/**
 * Enum describing the interface versions the 
PrimarySelectionDeviceManagerV1Interface can support.
 *
 * @since 5.XX
 **/
enum class PrimarySelectionDeviceManagerV1InterfaceVersion {
/**
 * zwp_primary_selection_device_manager_v1
 **/
 UnstableV1
};

/**
 * Enum describing the interface versions the 
PrimarySelectionDeviceV1Interface can support.
 *
 * @since 5.XX
 **/
enum class PrimarySelectionDeviceV1InterfaceVersion {
/**
 * zwp_primary_selection_device_v1
 **/
 UnstableV1
};

/**
 * Enum describing the interface versions the 
PrimarySelectionOfferV1Interface can support.
 *
 * @since 5.XX
 **/
enum class PrimarySelectionOfferV1InterfaceVersion {
/**
 * zwp_primary_selection_offer_v1
 **/
 UnstableV1
};

/**
 * Enum describing the interface versions the 
PrimarySelectionSourceV1Interface can support.
 *
 * @since 5.XX
 **/
enum class PrimarySelectionSourceV1InterfaceVersion {
/**
 * zwp_primary_selection_source_v1
 **/
 UnstableV1
};

class KWAYLANDSERVER_EXPORT PrimarySelectionDeviceManagerV1Interface : 
public Global
{
Q_OBJECT
public:
virtual ~PrimarySelectionDeviceManagerV1Interface();

/**
 * @returns The interface version used by this 
PrimarySelectionDeviceManagerV1Interface
 **/
PrimarySelectionDeviceManagerV1InterfaceVersion interfaceVersion() 
const;

protected:
class Private;
explicit PrimarySelectionDeviceManagerV1Interface(Private *d, QObject 
*parent = nullptr);

private:
Private *d_func() const;
};

class KWAYLANDSERVER_EXPORT PrimarySelectionDeviceV1Interface : public 
Resource
{
Q_OBJECT
public:

virtual ~PrimarySelectionDeviceV1Interface();

/**
 * @returns The interface version used by this 
PrimarySelectionDeviceV1Interface
 **/
PrimarySelectionDeviceManagerV1InterfaceVersion interfaceVersion() 
const;

protected:
class Private;
explicit PrimarySelectionDeviceV1Interface(Private *p, QObject *parent 
= nullptr);

private:
Private *d_func() const;
};

class KWAYLANDSERVER_EXPORT PrimarySelectionOfferV1Interface : public Global
{
Q_OBJECT
public:
virtual ~PrimarySelectionOfferV1Interface();

/**
 * @returns The interface version used by this 
PrimarySelectionOfferV1Interface
 **/
PrimarySelectionOfferV1InterfaceVersion interfaceVersion() const;

protected:
class Private;
explicit PrimarySelectionOfferV1Interface(Private *d, QObject *parent = 
nullptr);

private:
Private *d_func() const;
};

class KWAYLANDSERVER_EXPORT PrimarySelectionSourceV1Interface : public 
Resource
{
Q_OBJECT
public:

virtual ~PrimarySelectionSourceV1Interface();

/**
 * @returns The interface version used by this 
PrimarySelectionSourceV1Interface
 **/
PrimarySelectionDeviceManagerV1InterfaceVersion interfaceVersion() 
const;

protected:
class Private;
explicit PrimarySelectionSourceV1Interface(Private *p, QObject *parent 
= nullptr);

private:
Private *d_func() const;
};


}
}

#endif
  
  Without it just the first enum is generated.
  
  That being said only two of the four defined enums are actually used in the 
above skeleton and normally only one enum is defined which then determines all 
interfaces created by the manager. See for example:
  https://cgit.kde.org/kwayland.git/tree/src/server/textinput_interface.h
  
  Mixing different versions does not make sense so it's enough to have one enum 
for all interfaces.

REPOSITORY
  R127 KWayland

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

To: gladhorn, #kwin
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D24059: Replace KSslError::errorString with QSslError::errorString

2019-09-20 Thread Volker Krause 
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:b56de79a281f: Replace KSslError::errorString with 
QSslError::errorString (authored by vkrause).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24059?vs=66400=66546

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

AFFECTED FILES
  src/core/ktcpsocket.cpp

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

D12992: New elisa icon

2019-09-20 Thread Nathaniel Graham 
ngraham added a subscriber: trickyricky26.
ngraham added a comment.


  @abetts has been inactive recently; probably needs @lshoravi or @ndavis or 
@trickyricky26 to take over.

REPOSITORY
  R266 Breeze Icons

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

To: lshoravi, #vdg, ngraham, #elisa
Cc: trickyricky26, ndavis, KonqiDragon, abetts, paullesur, januz, mgallien, 
alex-l, andreask, kde-frameworks-devel, mfraser, mnesbitt, LeGast00n, 
carneirogustavo, jguidon, ctakano, Tizon, oussemabouaneni, ashwind, 
fbampaloukas, GB_2, sourabhboss, aureliencouderc, tgraves, hantzv, lcmscheid, 
nhuisman, ursjoss, mykolak, jussiv, michaelh, astippich, James, ngraham, bruns, 
kmf, lemuel

D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-20 Thread Nathaniel Graham 
ngraham added a comment.


  Oh perfect, then this can land as-is. Thanks!

REPOSITORY
  R263 KXmlGui

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

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

D24003: kwaylandScanner produce version enum per class

2019-09-20 Thread Frederik Gladhorn 
gladhorn marked an inline comment as done.
gladhorn added inline comments.

INLINE COMMENTS

> zzag wrote in generator.cpp:431
> Is it correct thogugh? Can you provide some test input so it's easier to see 
> the problem?

run kwaylandScanner on 
/usr/share/wayland-protocols/unstable/primary-selection/primary-selection-unstable-v1.xml
 and see that it produces something that doesn't compile. With this change it 
compiles.

REPOSITORY
  R127 KWayland

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

To: gladhorn, #kwin
Cc: zzag, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24004: Teach kwaylandscanner about PrimarySelection

2019-09-20 Thread Frederik Gladhorn 
gladhorn added subscribers: zzag, romangg, davidedmundson.
gladhorn added a comment.


  @romangg and @davidedmundson  any comments? I think @zzag  prefers the class 
names without the V1. I'd like to progress on this step by step and the 
generator at least gives a good starting point.

REPOSITORY
  R127 KWayland

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

To: gladhorn, #kwin
Cc: davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, ngraham, bruns

D23667: Add == and != operators to KIO::UDSEntry

2019-09-20 Thread David Faure 
dfaure accepted this revision.
dfaure added a comment.


  Looks ok, but I'm just curious about the use case. "I felt it was missing" 
doesn't sound as strong an argument as "I need this"...
  
  Slaves are supposed to mostly create those, not compare them, and apps are 
supposed to use KFileItem rather than UDSEntry directly.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D23667

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

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

D24091: Move "Full Screen Mode" item from Settings menu to View menu

2019-09-20 Thread David Faure 
dfaure added a comment.


  You can never end up with double separators, QMenu takes care of that.

REPOSITORY
  R263 KXmlGui

BRANCH
  move_fullscreen_to_view_menu (branched from master)

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

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

D21897: Address some issues reported by Krazy analysis

2019-09-20 Thread Kai Uwe Broulik 
broulik added inline comments.

INLINE COMMENTS

> ktextedit.cpp:373
> +
> +delete dialog;
>  }

You can do `dialog->setAttribute(Qt::WA_DeleteOnClose);` when creating the 
dialog instead of an explicit delete call

REPOSITORY
  R310 KTextWidgets

BRANCH
  krazy-50b2564 (branched from master)

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

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

D21897: Address some issues reported by Krazy analysis

2019-09-20 Thread Tommy Lincoln 
pajamapants3000 added a comment.


  Thank you for the feedback and approval! I don't have the access needed to 
land a change, what should I do next? I would certainly appreciate someone else 
landing it for me. Let me know, thanks!

REPOSITORY
  R310 KTextWidgets

BRANCH
  krazy-50b2564 (branched from master)

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

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

D24072: properly install whole plasmacomponent3

2019-09-20 Thread Marco Martin 
This revision was automatically updated to reflect the committed changes.
Closed by commit R242:455b41bc625e: properly install whole plasmacomponent3 
(authored by mart).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24072?vs=66441=66537

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

AFFECTED FILES
  src/declarativeimports/CMakeLists.txt

To: mart, #plasma, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D21721: Bring KNewStuffQuick to feature parity with KNewStuff(Widgets)

2019-09-20 Thread Arjen Hiemstra 
ahiemstra added inline comments.

INLINE COMMENTS

> leinir wrote in Button.qml:112
> Humhum, yes... so it needs to be possible to change enabled from outside, but 
> it also needs to be locked to disabled if allowedByKiosk is false... i'm 
> thinking this probably won't be the prettiest thing, but... catch enabled 
> changing, check whether allowedByKiosk is false and force it to false in the 
> case it is, and otherwise ignore it... unless there's a magic trick i'm not 
> aware of, which would be neat ;)

With a Binding object I can make sure that a button is kept disabled even when 
the main enabled is bound to a different value. However, I can not prevent 
javascript "button.enabled = true" from enabling the button, so forcing things 
in an enabledChanged handler seems the better solution.

REPOSITORY
  R304 KNewStuff

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

To: leinir, #knewstuff, #vdg, #frameworks, ahiemstra
Cc: davidedmundson, broulik, ahiemstra, anthonyfieroni, pino, ngraham, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D21721: Bring KNewStuffQuick to feature parity with KNewStuff(Widgets)

2019-09-20 Thread Dan Leinir Turthra Jensen 
leinir requested review of this revision.
leinir added a comment.


  Ping for some of the frameworks peeps... would be great to get this in sooner 
rather than later :)

INLINE COMMENTS

> ahiemstra wrote in author.cpp:37
> Suggestion: It might make sense to do this caching at the provider level so 
> users don't need to reimplement this.

A lot of this will probably want some work when we can break up the BC for 
KF6... but given the timeframe, yes, that of course means giving the actual 
bits we want to do some thought now :)

> ahiemstra wrote in categoriesmodel.cpp:49
> Suggestion: I tend to make roles static since it doesn't change between 
> instances anyway.
> Like so:
> 
>   static QHash roles{
>   { NameRole, "name" },
>   ...
>   };

Good call indeed, also perhaps a touch easier to read and such :) (also, this 
is what the QtQuick ItemsModel already does - i'll leave it to the new models, 
but probably wants proliferating through the codebase)

> ahiemstra wrote in categoriesmodel.h:61
> Suggestion: I use `const std::unique_ptr d;` these days, it removes 
> the need to explicitly delete the d pointer.

That sounds like something that'll want doing for KF6 as well, yes - i like the 
consistency, so just leaving it like this for now, but certainly something for 
the todo list :)

> ahiemstra wrote in commentsmodel.h:38
> Like Author, it might be a good idea to use QQmlParserStatus here.

Less critical codepath, but yeah, why not :)

> ahiemstra wrote in Button.qml:112
> Hmm, this is quite tricky, as a user of the Button I can now no longer safely 
> bind enabled as that would override the allowedByKiosk binding. You should 
> probably make this an explicit binding so it doesn't break as easily.

Humhum, yes... so it needs to be possible to change enabled from outside, but 
it also needs to be locked to disabled if allowedByKiosk is false... i'm 
thinking this probably won't be the prettiest thing, but... catch enabled 
changing, check whether allowedByKiosk is false and force it to false in the 
case it is, and otherwise ignore it... unless there's a magic trick i'm not 
aware of, which would be neat ;)

REPOSITORY
  R304 KNewStuff

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

To: leinir, #knewstuff, #vdg, #frameworks, ahiemstra
Cc: davidedmundson, broulik, ahiemstra, anthonyfieroni, pino, ngraham, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D21721: Bring KNewStuffQuick to feature parity with KNewStuff(Widgets)

2019-09-20 Thread Dan Leinir Turthra Jensen 
leinir updated this revision to Diff 66529.
leinir added a comment.


  - Fix some whitespace issues

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21721?vs=66368=66529

BRANCH
  knsquick-feature-parity-with-kns (branched from master)

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

AFFECTED FILES
  CMakeLists.txt
  src/attica/atticaprovider.cpp
  src/attica/atticaprovider_p.h
  src/core/CMakeLists.txt
  src/core/author.cpp
  src/core/author.h
  src/core/commentsmodel.cpp
  src/core/commentsmodel.h
  src/core/engine.cpp
  src/core/engine.h
  src/core/entryinternal.cpp
  src/core/installation.cpp
  src/core/itemsmodel.cpp
  src/core/itemsmodel.h
  src/core/provider.h
  src/core/question.cpp
  src/core/question.h
  src/qtquick/CMakeLists.txt
  src/qtquick/author.cpp
  src/qtquick/author.h
  src/qtquick/categoriesmodel.cpp
  src/qtquick/categoriesmodel.h
  src/qtquick/commentsmodel.cpp
  src/qtquick/commentsmodel.h
  src/qtquick/qml/Button.qml
  src/qtquick/qml/Dialog.qml
  src/qtquick/qml/DialogContent.qml
  src/qtquick/qml/DownloadItemsSheet.qml
  src/qtquick/qml/EntryDetails.qml
  src/qtquick/qml/NewStuffItem.qml
  src/qtquick/qml/NewStuffList.qml
  src/qtquick/qml/Page.qml
  src/qtquick/qml/QuestionAsker.qml
  src/qtquick/qml/private/ConditionalLoader.qml
  src/qtquick/qml/private/EntryCommentDelegate.qml
  src/qtquick/qml/private/EntryCommentsPage.qml
  src/qtquick/qml/private/EntryScreenshots.qml
  src/qtquick/qml/private/GridTileDelegate.qml
  src/qtquick/qml/private/Rating.qml
  src/qtquick/qml/private/Shadow.qml
  src/qtquick/qml/private/entrygriddelegates/BigPreviewDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/ThumbDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml
  src/qtquick/qmldir
  src/qtquick/qmlplugin.cpp
  src/qtquick/quickengine.cpp
  src/qtquick/quickengine.h
  src/qtquick/quickitemsmodel.cpp
  src/qtquick/quickitemsmodel.h
  src/qtquick/quickquestionlistener.cpp
  src/qtquick/quickquestionlistener.h
  tests/CMakeLists.txt
  tests/khotnewstuff-dialog-ui/main.qml
  tests/khotnewstuff-dialog.cpp

To: leinir, #knewstuff, #vdg, #frameworks, ahiemstra
Cc: davidedmundson, broulik, ahiemstra, anthonyfieroni, pino, ngraham, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns