D26320: endl is Qt:: namespaced in qt5.15. Port to \n and flush when QTextStream is not on a QFile

[Friedrich W. H. Kossebau]

kossebau added a comment.


  @mlaurent  Bonne année :) Hm, are we sure that endl is namespaced with Qt:: 
in 5.15 already? Isn't that rather Qt6 only?
  
  For Qt5 `endl` is in the namespace `QTextStreamFunctions` rather, which 
though is also inlined by `using namespace QTextStreamFunctions`, so that 
should still compile, no?
  IMHO this should be rather only ported/touched during Qt6 then. Well, unless 
doing the flush only at the end is an improvement of course.
  
  See 
https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/serialization/qtextstream.h:
  
#if defined(Q_QDOC) || QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
namespace Qt {
#else
// This namespace only exists for 'using namespace' declarations.
namespace QTextStreamFunctions {
#endif
// [...]
Q_CORE_EXPORT QTextStream &endl(QTextStream &s);
// [...]

} // namespace QTextStreamFunctions

#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0) && !defined(Q_QDOC)
namespace Qt {
using namespace QTextStreamFunctions;
}

QT_WARNING_PUSH
QT_WARNING_DISABLE_CLANG("-Wheader-hygiene")
// We use 'using namespace' as that doesn't cause
// conflicting definitions compiler errors.
using namespace QTextStreamFunctions;
QT_WARNING_POP
#endif // QT_VERSION < QT_VERSION_CHECK(6, 0, 0) && !defined(Q_QDOC)

REPOSITORY
  R269 BluezQt

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

To: mlaurent, dfaure
Cc: kossebau, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D26321: Expose show-line-count in the ConfigInterface

[Dominik Haumann]

dhaumann accepted this revision.
dhaumann added a comment.


  Ok with this... Thanks. A note " Since 5.6x would be nice".

REPOSITORY
  R39 KTextEditor

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

To: mludwig, #ktexteditor, cullmann, dhaumann
Cc: dhaumann, cullmann, kwrite-devel, kde-frameworks-devel, cent, LeGast00n, 
szutmael, GB_2, domson, michaelh, ngraham, bruns, demsking, head7, kfunk, sars

T11950: Reduce the pain of working on monochrome Breeze icons

[Noah Davis]

ndavis added a comment.


  For the Inkscape optimized svg gradient problem: 
https://gitlab.com/inkscape/inbox/issues/1121

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

To: ngraham, ndavis
Cc: mglb, #frameworks, mart, trickyricky26, ndavis, #vdg, ngraham, manueljlin, 
LeGast00n, cblack, konkinartem, ian, jguidon, hannahk, Ghost6, jraleigh, 
MrPepe, fbampaloukas, squeakypancakes, alexde, IohannesPetros, GB_2, michaelh, 
crozbo, firef, bruns, skadinna, aaronhoneycutt, mbohlender

D26326: Add application/x-audacity-project icon

[Noah Davis]

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


  Just needs some optimization and it'll be good to go

REPOSITORY
  R266 Breeze Icons

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

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

D26306: Define more documentation search paths (also custom)

[Andreas Sturmlechner]

asturmlechner added a comment.


  Thanks for that, works for me!

REPOSITORY
  R238 KDocTools

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

To: ltoscano
Cc: cgiboudeaux, lbeltrame, rdieter, arojas, rikmills, maximilianocuria, 
asturmlechner, kde-frameworks-devel, kde-doc-english, LeGast00n, gennad, 
fbampaloukas, GB_2, michaelh, ngraham, bruns, skadinna

D26319: Port endl to \n (QFile flushs data when deleted)

[Aleix Pol Gonzalez]

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

REPOSITORY
  R289 KNotifications

BRANCH
  port_endl_qt5.15 (branched from master)

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

To: mlaurent, dfaure, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26320: endl is Qt:: namespaced in qt5.15. Port to \n and flush when QTextStream is not on a QFile

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> CppGenerator.cpp:61
>  writeCopyrightHeader(stream);
> -stream << "#pragma once" << endl << endl;
> -stream << "#include " << endl << endl;
> -stream << "class QDBusObjectPath;" << endl << endl;
> -stream << "namespace BluezQt" << endl << "{" << endl << endl;
> -stream << "class " << className << ";" << endl << endl;
> -stream << "class " << className << "Adaptor : public 
> QDBusAbstractAdaptor" << endl << "{" << endl;
> -stream << "Q_OBJECT " << endl;
> -stream << "Q_CLASSINFO(\"D-Bus Interface\", \"" << 
> interface.name() << "\")" << endl;
> +stream << "#pragma once" << "\n" << "\n";
> +stream << "#include " << "\n" << "\n";

I'd either merge into the line or at least use '\n'.

REPOSITORY
  R269 BluezQt

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

To: mlaurent, dfaure
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26318: Port endl to "\n" flush at the end + use const'ref in loop

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> scalabletest.cpp:312
>  QTextStream stream(&msg);
> -stream << "Duplicated scalable icons:" << endl;
> -for (auto icon : duplicatedScalableIcons.keys()) {
> -stream << QString("  %1:").arg(icon) << endl;
> -for (auto info : duplicatedScalableIcons[icon]) {
> -stream << QString("%1").arg(info.absoluteFilePath()) 
> << endl;
> +stream << "Duplicated scalable icons:" << "\n";
> +for (const auto &icon : duplicatedScalableIcons.keys()) {

'\n' or merge into the string.

> scalabletest.cpp:319
>  }
> +stream.flush();
>  QFAIL(qPrintable(msg));

Maybe add a {}-scope?

REPOSITORY
  R266 Breeze Icons

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

To: mlaurent, dfaure
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26303: Remove endl in qDebug as it's already add "\n" + port to Qt::endl in qt5.15

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> plugintest.cpp:60
>  
> -cout << "-- KPluginTrader Test --" << endl;
> +cout << "-- KPluginTrader Test --"
> +#if (QT_VERSION < QT_VERSION_CHECK(5, 15, 0))

std::endl should just work with cout.

REPOSITORY
  R309 KService

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

To: mlaurent, dfaure
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26300: Port endl to \n as QTextStream is flushed at the end. endl is namespaced in qt5.15

[Aleix Pol Gonzalez]

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

INLINE COMMENTS

> pidgin_emoticons.cpp:221
> +out << QStringLiteral("Description=") + themeName() << "\n";
> +out << "Author=" << "\n";
> +out << "\n";

Put it in the same string as `"Author=\n"`? Or at least use '\n'.

REPOSITORY
  R301 KEmoticons

BRANCH
  port_endl_qt5.15 (branched from master)

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

To: mlaurent, dfaure, apol
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26299: endl is namespaced in qt5.15

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> prison-datamatrix.cpp:39
>QTextStream str(stdout);
> -  str << error << ": " << errormessage << endl;
> +  str << error << ": " << errormessage
> +   #if (QT_VERSION < QT_VERSION_CHECK(5, 15, 0))

Wouldn't make more sense to port to std::endl which as been around since 
forever? Or pass a '\n'.

REPOSITORY
  R280 Prison

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

To: mlaurent, dfaure
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26294: Allow to load widget based KCMs from kcms subdir

[Aleix Pol Gonzalez]

apol added a comment.


  It would be useful if you were a bit more explicit about how you tested it on 
the commit message.

INLINE COMMENTS

> kcmoduleloader.cpp:103
>  if (!cm) {
> +module = factory->create(parent, args2);
> +if (module) {

I see that cm is initialized with a nullptr parent. Why's it different here?

REPOSITORY
  R295 KCMUtils

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

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

D26202: WIP: Refactor KConfigXT

[David Faure]

dfaure added a reviewer: dfaure.

REPOSITORY
  R237 KConfig

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

To: tcanabrava, #frameworks, ervin, bport, dfaure
Cc: bport, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D26331: Removed reading description from desktop files

[Nathaniel Graham]

ngraham added reviewers: Frameworks, Dolphin.
ngraham added a comment.


  It would be helpful if you could update the description to mention why this 
change is being made and when benefit comes from not reading desktop files' 
comment fields.
  
  Also, updating the Test Plan section with details of your testing would be 
nice. :)

REPOSITORY
  R241 KIO

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

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

D26331: Removed reading description from desktop files

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: count, broulik
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26332: [KURISearchFilterEngine] Port QRegExp to QRegularExpression

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: dfaure, apol.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  Use const as need be.
  Use QList::at() where possible.

TEST PLAN
  make && ctest

REPOSITORY
  R241 KIO

BRANCH
  l-ikws (branched from master)

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

AFFECTED FILES
  src/urifilters/ikws/kuriikwsfiltereng.cpp

To: ahmadsamir, dfaure, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26331: Removed reading description from desktop files

[count negative]

count added a reviewer: broulik.

REPOSITORY
  R241 KIO

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

To: count, broulik
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26331: Removed reading description from desktop files

[count negative]

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

REVISION SUMMARY
  For https://bugs.kde.org/show_bug.cgi?id=382325 i've removed reading comments 
inside desktop files.

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/core/kfileitem.cpp

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

D26321: Expose show-line-count in the ConfigInterface

[Michel Ludwig]

mludwig added a comment.


  In D26321#585381 , @cullmann wrote:
  
  > I am ok with this , but in the long run, we want to change the behavior of 
the view's config interface to just dispatch to the internal config interface 
like for the document.
  >  You missed to add the line-count to KTextEditor::ViewPrivate::configKeys()
  
  
  Good catch :) I've added it now.

REPOSITORY
  R39 KTextEditor

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

To: mludwig, #ktexteditor, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, cent, LeGast00n, szutmael, 
GB_2, domson, michaelh, ngraham, bruns, demsking, head7, kfunk, sars, dhaumann

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

[Luigi Toscano]

ltoscano added a comment.


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

REPOSITORY
  R241 KIO

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

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

D26321: Expose show-line-count in the ConfigInterface

[Michel Ludwig]

mludwig updated this revision to Diff 72484.
mludwig added a comment.


  Add "line-count" to KTextEditor::ViewPrivate::configKeys()

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26321?vs=72445&id=72484

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

AFFECTED FILES
  autotests/src/configinterface_test.cpp
  src/include/ktexteditor/configinterface.h
  src/utils/kateconfig.h
  src/view/kateview.cpp

To: mludwig, #ktexteditor, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, cent, LeGast00n, szutmael, 
GB_2, domson, michaelh, ngraham, bruns, demsking, head7, kfunk, sars, dhaumann

D26306: Define more documentation search paths (also custom)

[Luca Beltrame]

lbeltrame added a comment.


  +1, would allow us to get rid of some downstream patches.

REPOSITORY
  R238 KDocTools

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

To: ltoscano
Cc: cgiboudeaux, lbeltrame, rdieter, arojas, rikmills, maximilianocuria, 
asturmlechner, kde-frameworks-devel, kde-doc-english, LeGast00n, gennad, 
fbampaloukas, GB_2, michaelh, ngraham, bruns, skadinna

D26317: Port endl to "\n". endl in qt5.15 is namespaced. We don't need to flush as when QFile is deleted it flush data

[Luigi Toscano]

ltoscano added a comment.


  In D26317#585315 , @aacid wrote:
  
  > Isn't it better to just use `Qt::endl` ?
  >
  > I think it's much clearer to understand `Qt::endl` than `QLatin1Char('\n')`
  >
  > But if we prefer to change to use \n it should be merged into the existing 
strings, doesn't make much sense to do 
  >  `outStream << "" << QLatin1Char('\n');`
  >  instead of
  >  `outStream << "\n';`
  
  
  I agree with Albert here; I think it's a bit more painful but it's probably 
going to be cleaner.

REPOSITORY
  R238 KDocTools

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

To: mlaurent, dfaure
Cc: ltoscano, aacid, anthonyfieroni, kde-frameworks-devel, kde-doc-english, 
LeGast00n, gennad, fbampaloukas, GB_2, michaelh, ngraham, bruns, skadinna

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.13 - Build # 259 - Aborted!

[CI System]

BUILD ABORTED
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.13/259/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Mon, 30 Dec 2019 19:19:23 +
 Build duration:
17 hr and counting

D26321: Expose show-line-count in the ConfigInterface

[Christoph Cullmann]

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


  I am ok with this , but in the long run, we want to change the behavior of 
the view's config interface to just dispatch to the internal config interface 
like for the document.
  You missed to add the line-count to KTextEditor::ViewPrivate::configKeys()

REPOSITORY
  R39 KTextEditor

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

To: mludwig, #ktexteditor, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, cent, LeGast00n, szutmael, 
GB_2, domson, michaelh, ngraham, bruns, demsking, head7, kfunk, sars, dhaumann

D24431: Restore cursor thumbnailer

[Kai Uwe Broulik]

broulik requested review of this revision.

REPOSITORY
  R320 KIO Extras

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

To: broulik, #plasma, fredrik, ngraham
Cc: adridg, ngraham, kde-frameworks-devel, kfm-devel, pberestov, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, 
mikesomov

D26317: Port endl to "\n". endl in qt5.15 is namespaced. We don't need to flush as when QFile is deleted it flush data

[Albert Astals Cid]

aacid added a comment.


  Ah, it works fine in 5.14.
  
  I was going to say we must report to Qt immediately that they broke source 
compatibility and that was unacceptable but then i realized that the old non 
namespaced version works just fine, it's just deprecated.
  
  I'm going to go back to party preparations :)

REPOSITORY
  R238 KDocTools

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

To: mlaurent, dfaure
Cc: aacid, anthonyfieroni, kde-frameworks-devel, kde-doc-english, LeGast00n, 
gennad, fbampaloukas, GB_2, michaelh, ngraham, bruns, skadinna

D24431: Restore cursor thumbnailer

[Kai Uwe Broulik]

broulik updated this revision to Diff 72476.
broulik edited the summary of this revision.
broulik added a comment.
This revision is now accepted and ready to land.


  - Find XCursor specifically

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24431?vs=67357&id=72476

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

AFFECTED FILES
  thumbnail/CMakeLists.txt

To: broulik, #plasma, fredrik, ngraham
Cc: adridg, ngraham, kde-frameworks-devel, kfm-devel, pberestov, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, 
mikesomov

D26313: Add application/vnd.apple.pkpass icon

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R266:eca72cb0689b: Add application/vnd.apple.pkpass icon 
(authored by broulik).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26313?vs=72464&id=72477

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

AFFECTED FILES
  icons-dark/mimetypes/16/application-vnd.apple.pkpass.svg
  icons-dark/mimetypes/22/application-vnd.apple.pkpass.svg
  icons-dark/mimetypes/32/application-vnd.apple.pkpass.svg
  icons-dark/mimetypes/64/application-vnd.apple.pkpass.svg
  icons/mimetypes/16/application-vnd.apple.pkpass.svg
  icons/mimetypes/22/application-vnd.apple.pkpass.svg
  icons/mimetypes/32/application-vnd.apple.pkpass.svg
  icons/mimetypes/64/application-vnd.apple.pkpass.svg

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

D26319: Port endl to \n (QFile flushs data when deleted)

[Laurent Montel]

mlaurent updated this revision to Diff 72475.
mlaurent added a comment.


  Use QLatin1Char

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26319?vs=72442&id=72475

BRANCH
  port_endl_qt5.15 (branched from master)

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

AFFECTED FILES
  src/notifybylogfile.cpp

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

D26317: Port endl to "\n". endl in qt5.15 is namespaced. We don't need to flush as when QFile is deleted it flush data

[Laurent Montel]

mlaurent added a comment.


  "/compile/kde5/framework/frameworks/kdoctools/src/docbookl10nhelper.cpp: Dans 
la fonction « int writeLangFile(const QString&, const QString&, const 
LangListType&) »:
  /compile/kde5/framework/frameworks/kdoctools/src/docbookl10nhelper.cpp:62:57: 
erreur: « endl » n'est pas un membre de « Qt »
  
62 |   .arg(dtdPath) << QLatin1Char('\n') << Qt::endl;
  
  "
  no it's not :) qt5.13 :)

REPOSITORY
  R238 KDocTools

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

To: mlaurent, dfaure
Cc: aacid, anthonyfieroni, kde-frameworks-devel, kde-doc-english, LeGast00n, 
gennad, fbampaloukas, GB_2, michaelh, ngraham, bruns, skadinna

D26317: Port endl to "\n". endl in qt5.15 is namespaced. We don't need to flush as when QFile is deleted it flush data

[Laurent Montel]

mlaurent added a comment.


  "No, it's not. It's namespaced in Qt 5.15 means it should be ifdef guarded 
which makes things to suck." yep it's for that I switched to "\n"
  
  I don't want to add #ifdef if it's not necessary.

REPOSITORY
  R238 KDocTools

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

To: mlaurent, dfaure
Cc: aacid, anthonyfieroni, kde-frameworks-devel, kde-doc-english, LeGast00n, 
gennad, fbampaloukas, GB_2, michaelh, ngraham, bruns, skadinna

D26317: Port endl to "\n". endl in qt5.15 is namespaced. We don't need to flush as when QFile is deleted it flush data

[Albert Astals Cid]

aacid added a comment.


  But it is already namespaced in older Qt too, so no ifdef needed

REPOSITORY
  R238 KDocTools

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

To: mlaurent, dfaure
Cc: aacid, anthonyfieroni, kde-frameworks-devel, kde-doc-english, LeGast00n, 
gennad, fbampaloukas, GB_2, michaelh, ngraham, bruns, skadinna

D26326: Add application/x-audacity-project icon

[Kai Uwe Broulik]

broulik edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D26326: Add application/x-audacity-project icon

[Kai Uwe Broulik]

broulik edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D26326: Add application/x-audacity-project icon

[Kai Uwe Broulik]

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

REVISION SUMMARY
  BUG: 415722

TEST PLAN
  - Takes the audacity app icon, simplifies it slightly and monochromizes it. 
Getting it pixel-aligned was quite hard and I didn't do a very good job at it I 
think :)
  
  F7852129: Screenshot_20191231_124031.png 

  F7852130: Screenshot_20191231_124038.png 

  F7852131: Screenshot_20191231_124043.png 

  F7852132: Screenshot_20191231_124049.png 

  F7852133: Screenshot_20191231_124055.png 


REPOSITORY
  R266 Breeze Icons

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

AFFECTED FILES
  icons/mimetypes/16/application-x-audacity-project.svg
  icons/mimetypes/22/application-x-audacity-project.svg
  icons/mimetypes/32/application-x-audacity-project.svg
  icons/mimetypes/64/application-x-audacity-project.svg

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

D26317: Port endl to "\n". endl in qt5.15 is namespaced. We don't need to flush as when QFile is deleted it flush data

[Anthony Fieroni]

anthonyfieroni added a comment.


  In D26317#585315 , @aacid wrote:
  
  > Isn't it better to just use `Qt::endl` ?
  
  
  No, it's not. It's namespaced in Qt 5.15 means it should be ifdef guarded 
which makes things to suck.

REPOSITORY
  R238 KDocTools

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

To: mlaurent, dfaure
Cc: aacid, anthonyfieroni, kde-frameworks-devel, kde-doc-english, LeGast00n, 
gennad, fbampaloukas, GB_2, michaelh, ngraham, bruns, skadinna

D26317: Port endl to "\n". endl in qt5.15 is namespaced. We don't need to flush as when QFile is deleted it flush data

[Albert Astals Cid]

aacid added a comment.


  Isn't it better to just use `Qt::endl` ?
  
  I think it's much clearer to understand `Qt::endl` than `QLatin1Char('\n')`
  
  But if we prefer to change to use \n it should be merged into the existing 
strings, doesn't make much sense to do 
  `outStream << "" << QLatin1Char('\n');`
  instead of
  `outStream << "\n';`

REPOSITORY
  R238 KDocTools

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

To: mlaurent, dfaure
Cc: aacid, anthonyfieroni, kde-frameworks-devel, kde-doc-english, LeGast00n, 
gennad, fbampaloukas, GB_2, michaelh, ngraham, bruns, skadinna

D26313: Add application/vnd.apple.pkpass icon

[Noah Davis]

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

REPOSITORY
  R266 Breeze Icons

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

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

D26313: Add application/vnd.apple.pkpass icon

[Kai Uwe Broulik]

broulik updated this revision to Diff 72464.
broulik added a comment.


  - Fix misaligned object
  - Optimize SVGs (also remove unused stylesheet from monochrome icons)
  - Copy to breeze-dark

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26313?vs=72432&id=72464

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

AFFECTED FILES
  icons-dark/mimetypes/16/application-vnd.apple.pkpass.svg
  icons-dark/mimetypes/22/application-vnd.apple.pkpass.svg
  icons-dark/mimetypes/32/application-vnd.apple.pkpass.svg
  icons-dark/mimetypes/64/application-vnd.apple.pkpass.svg
  icons/mimetypes/16/application-vnd.apple.pkpass.svg
  icons/mimetypes/22/application-vnd.apple.pkpass.svg
  icons/mimetypes/32/application-vnd.apple.pkpass.svg
  icons/mimetypes/64/application-vnd.apple.pkpass.svg

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

D17816: Support for xattrs on kio copy/move

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> jobtest.cpp:529
>  
> +QHash getSampleAttrs()
> +{

prepend `static`

> jobtest.cpp:626
> +
> +QString getXattrCommand()
> +{

prepend `static`

> jobtest.cpp:638
> +if (command.isEmpty()) {
> +command = QStandardPaths::findExecutable("xattr");
> +}

if (command.isEmpty())
  qWarning() << "ERROR: setfattr/setextattr/xattr not found";

to make it easier to debug than just this method returning empty.

> jobtest.cpp:650
> +if (command.isEmpty() || command != "setfattr") {
> +return;
> +}

QSKIP("This test requires setfattr")

Otherwise it will look like a "PASS" in the test output.

> jobtest.cpp:753
> +if (command.isEmpty() || command != "setfattr") {
> +return;
> +}

QSKIP

> tmarshall wrote in copyjob.cpp:1119
> Thanks to @ahmad78 for chatting with me about this in irc.
> 
> Everything works for me if I replace the `exec` call with a `start` call. 
> Currently the result of the exec call is thrown away as the job itself does 
> the error handling and there isn't much to do if the xattrs can't be copied 
> for whatever reason.
> 
> As for why this is a different job, that was a design decision by the 
> original author. It seems like a decent enough way forward to me as it leans 
> on the side of modularity. The applications of xattrs are so broad that it's 
> hard to say what they will be used for in the future or what the desired 
> functionality might be. Having a separate job will allow for more flexibility 
> regarding xattrs going forward.
> 
> In short, I don't really think it hurts to have a separate job and there is 
> certainly the potential that it comes in handy.

I don't mind that errors don't propagate up here, that's fine, we certainly 
don't want the copy to fail if attributes can't be copied.

I do mind that this uses exec() (Qt eventloop re-entrancy is a nasty source of 
bugs), and I do mind a fire-and-forget subjob (if that's what start() here does 
-- unless the subjob gets registered?).

I have provided the solution already, connect the subjob's result to a lambda 
that contains the rest of the code in this method [but see below].

I just realized another problem: this is at the wrong layer. It should be in 
FileCopyJob, not in the high-level CopyJob (which lists directories, creates 
directories, etc. and delegates the task of copying one file to FileCopyJob). 
FileCopyJob is also used directly in many places (when a single file has to be 
copied) so if you want that to preserve xattr (rethorical question, I'm 
assuming you do) this should go down to FileCopyJob.

About this being a different job: actually, one doesn't prevent the other. Look 
at permissions. We have ChmodJob for setting permissions, but when FileCopyJob 
finds that the kioslave supports copy() (see DirectCopyJob on the app side) 
then copy() takes care of copying permissions too. FileCopyJob only calls 
ChmodJob for the special case of renaming and because the FileCopyJob API 
actually allows to change permissions, this doesn't apply to xattr.

When the kioslave doesn't support copy() and it falls back to "data pump" mode 
(get() + put()), FileCopyJob passes the permissions to the put() job. *That* is 
the case where it should follow that with a KIO::copy_xattr subjob, unless 
xattr can be passed by metadata to the put job?

I believe the same idea should be done for xattr (kio_file's copy() should copy 
xattr all by itself), to minimize roundtrips between the app and the slave. 
People complain that copying files with KIO is too slow, let's not make it 
worse.

> cfeck wrote in filecopyjob.cpp:519
> This waits (i.e. spawns a separate event loop) until the job is finished. 
> Should use a subjob.

That's what should happen in copy() IMHO. But that's an optimization, I can 
accept that being done later.

And if you want to support other cases than file->file, like desktop, trash, 
smb, etc. then this should also be done when the put job finishes.

What's for sure is that doing it both in CopyJob and in FileCopyJob is overkill 
since the former uses the latter :-)

REPOSITORY
  R241 KIO

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

To: tmarshall, dfaure, chinmoyr, bruns, #frameworks, cochise
Cc: anthonyfieroni, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, GB_2, michaelh

D26313: Add application/vnd.apple.pkpass icon

[Volker Krause]

vkrause added a comment.


  Awesome, thanks!

REPOSITORY
  R266 Breeze Icons

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

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

D26177: Port QRegExp to QRegularExpression

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> dfaure wrote in kconfig_compiler.cpp:646
> I do, and they were, that was always a big selling point for Qt ;)
> 
> (I started contributing to KDE at the time of Qt 1.1, in 1998)

:D

REPOSITORY
  R237 KConfig

BRANCH
  l-qregularexpression (branched from master)

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

To: ahmadsamir, #frameworks, dfaure, ervin, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26177: Port QRegExp to QRegularExpression

[David Faure]

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

INLINE COMMENTS

> ahmadsamir wrote in kconfig_compiler.cpp:646
> Yep; (and who knows if Qt docs were as expansive/exhaustive as they're now 
> :)).
> 
> And then fileNameOnly() became unneeded when QFileInfo was used in the code.

I do, and they were, that was always a big selling point for Qt ;)

(I started contributing to KDE at the time of Qt 1.1, in 1998)

REPOSITORY
  R237 KConfig

BRANCH
  l-qregularexpression (branched from master)

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

To: ahmadsamir, #frameworks, dfaure, ervin, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26148: Add truncation support to FileJob

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> filejob.h:109
> + *
> + * @param length the desired length to truncate to
> + */

Missing @since 5.66

> filejob.h:180
> + */
> +void truncated(KIO::Job *job, KIO::filesize_t length);
> +

@since 5.66

> kprotocolmanager.h:477
> + * @param url the url to check
> + * @return true if the protocol supports truncating
> + */

@since 5.66

> slavebase.h:209
>  
> +void truncated(KIO::filesize_t _length);
> +

@since 5.66

> slavebase.h:481
> + */
> +virtual void truncate(KIO::filesize_t length);
>  /**

WARNING RED ALERT adding a virtual method is a binary incompatible change. 
This cannot be done in the 5.x timeframe.

Use a different solution, like special() or virtual_hook(). I think 
virtual_hook is better if the idea is to turn it into a virtual method in KF6.

REPOSITORY
  R241 KIO

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

To: feverfew, fvogt, dfaure, sitter
Cc: apol, ngraham, sitter, dfaure, kde-frameworks-devel, fvogt, LeGast00n, 
GB_2, michaelh, bruns

D26321: Expose show-line-count in the ConfigInterface

[Michel Ludwig]

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

REVISION SUMMARY
  The show-line-count property is added to the ConfigInterface.
  
  As Kile doesn't use KatePart's status bar, it needs a way to query whether 
the line count should be shown or not, so that Kile can show the line count in 
its own status bar.

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  autotests/src/configinterface_test.cpp
  src/include/ktexteditor/configinterface.h
  src/utils/kateconfig.h
  src/view/kateview.cpp

To: mludwig, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, cent, LeGast00n, szutmael, GB_2, 
domson, michaelh, ngraham, bruns, demsking, head7, cullmann, kfunk, sars, 
dhaumann