D12756: [KDateTable] Use a more visible red color

[Nathaniel Graham]

ngraham added a comment.


  In D12756#259691 , @mwolff wrote:
  
  > Let's try to fix the bug for real, instead of implementing half-baked 
workarounds that only work for the default configurations.
  >
  > Alternative ideas: add setters for the two possible cell text colors, then 
somehow set the KColorScheme color from the outside.
  >
  > We really should upstream more of KColorScheme to Qt/QPalette...
  
  
  That would require changes to all clients, which doesn't seem desirable. 
"Fixing the bug for real" probably looks like making KWidgetsAddons not a tier 
1 frameworks anymore so it can use `KColorScheme`.
  
  FWIW, we ran into the same issue with the recent visual update to 
`KMessageWidget`: D12508 . The only 
practical near-term options here are to hardcode colors, or start depending on 
`KColorScheme`.

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, #frameworks, cfeck
Cc: mwolff, apol, michaelh, ngraham, bruns

KDE CI: Frameworks kcoreaddons kf5-qt5 SUSEQt5.9 - Build # 27 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kcoreaddons%20kf5-qt5%20SUSEQt5.9/27/
 Project:
Frameworks kcoreaddons kf5-qt5 SUSEQt5.9
 Date of build:
Wed, 09 May 2018 01:29:56 +
 Build duration:
3 min 32 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 23 test(s), Skipped: 0 test(s), Total: 24 test(s)Failed: TestSuite.kdirwatch_qfswatch_unittest

KDE CI: Frameworks kcoreaddons kf5-qt5 SUSEQt5.10 - Build # 82 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kcoreaddons%20kf5-qt5%20SUSEQt5.10/82/
 Project:
Frameworks kcoreaddons kf5-qt5 SUSEQt5.10
 Date of build:
Wed, 09 May 2018 01:29:56 +
 Build duration:
3 min 33 sec and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 22 test(s), Skipped: 0 test(s), Total: 24 test(s)Failed: TestSuite.kdirwatch_qfswatch_unittestFailed: TestSuite.kdirwatch_stat_unittest

KDE CI: Frameworks kcoreaddons kf5-qt5 FreeBSDQt5.10 - Build # 5 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kcoreaddons%20kf5-qt5%20FreeBSDQt5.10/5/
 Project:
Frameworks kcoreaddons kf5-qt5 FreeBSDQt5.10
 Date of build:
Wed, 09 May 2018 01:29:55 +
 Build duration:
3 min 25 sec and counting
   JUnit Tests
  Name: (root) Failed: 3 test(s), Passed: 20 test(s), Skipped: 0 test(s), Total: 23 test(s)Failed: TestSuite.kdirwatch_inotify_unittestFailed: TestSuite.kdirwatch_qfswatch_unittestFailed: TestSuite.kdirwatch_stat_unittest

D12768: Allow wrapping selection off top/bottom of autocomplete results

[Subramaniyam Raizada]

sraizada added a project: KTextEditor.
sraizada added a subscriber: KTextEditor.

REPOSITORY
  R39 KTextEditor

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

To: sraizada, #ktexteditor
Cc: #ktexteditor, #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, 
head7, cullmann, kfunk, sars, dhaumann

D12768: Allow wrapping selection off top/bottom of autocomplete results

[Subramaniyam Raizada]

sraizada created this revision.
sraizada added a reviewer: KTextEditor.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added a subscriber: Frameworks.
sraizada requested review of this revision.

REVISION SUMMARY
  When at the first autocomplete result, pressing up will wrap around to the 
last result, and vice versa.
  
  This does make test 56 (completion_test) fail with an "Exception: Child 
aborted" - I'm looking into it.

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/completion/katecompletiontree.cpp

To: sraizada, #ktexteditor
Cc: #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, 
sars, dhaumann

D12765: Use a more intuitive, user-friendly name for Samba share access

[Elvis Angelaccio]

elvisangelaccio added a comment.


  +1

REPOSITORY
  R320 KIO Extras

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

To: ngraham, #frameworks, #dolphin, #vdg
Cc: ltoscano, elvisangelaccio

D12765: Use a more intuitive, user-friendly name for Samba share access

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R320 KIO Extras

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

To: ngraham, #frameworks, #dolphin, #vdg
Cc: ltoscano, elvisangelaccio

D12765: Use a more intuitive, user-friendly name for Samba share access

[Nathaniel Graham]

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


  Add the protocol acronym to help tech-savvy users know what they're looking at

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12765?vs=33840=33844

BRANCH
  friendlier-samba-share-name (branched from master)

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

AFFECTED FILES
  smb/smb-network.desktop

To: ngraham, #frameworks, #dolphin, #vdg
Cc: ltoscano, elvisangelaccio

D12765: Use a more intuitive, user-friendly name for Samba share access

[Nathaniel Graham]

ngraham added a comment.


  Sounds good to me!

REPOSITORY
  R320 KIO Extras

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

To: ngraham, #frameworks, #dolphin, #vdg
Cc: ltoscano, elvisangelaccio

D12765: Use a more intuitive, user-friendly name for Samba share access

[Luigi Toscano]

ltoscano added a comment.


  or "Shared folders (SMB)", as the protocol is probably more relevant.

REPOSITORY
  R320 KIO Extras

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

To: ngraham, #frameworks, #dolphin, #vdg
Cc: ltoscano, elvisangelaccio

D12765: Use a more intuitive, user-friendly name for Samba share access

[Nathaniel Graham]

ngraham added a comment.


  "Shared folders (Samba)"?
  
  Then again, I doubt that people who know what Samba is will have difficulty 
figuring out that "Shared folders" under the "Network" Place is where Samba 
shares live.

REPOSITORY
  R320 KIO Extras

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

To: ngraham, #frameworks, #dolphin, #vdg
Cc: elvisangelaccio

D12765: Use a more intuitive, user-friendly name for Samba share access

[Elvis Angelaccio]

elvisangelaccio added a comment.


  If we remove Samba, how do people who know Samba find where Samba is?
  
  Maybe we can use "Samba Shared Folders" ?

REPOSITORY
  R320 KIO Extras

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

To: ngraham, #frameworks, #dolphin, #vdg
Cc: elvisangelaccio

D12765: Use a more intuitive, user-friendly name for Samba share access

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R320 KIO Extras

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

To: ngraham, #frameworks, #dolphin, #vdg

D12765: Use a more intuitive, user-friendly name for Samba share access

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Frameworks, Dolphin, VDG.
ngraham requested review of this revision.

REVISION SUMMARY
  "Samba shares" is a jargonistic term that won't mean much to most people. 
This patch replaces it with a more intuitive and user-friendly term.
  
  BUG: 141257
  FIXED-IN: 18.08.0

REPOSITORY
  R320 KIO Extras

BRANCH
  friendlier-samba-share-name (branched from master)

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

AFFECTED FILES
  smb/smb-network.desktop

To: ngraham, #frameworks, #dolphin, #vdg

D12756: [KDateTable] Use a more visible red color

[Milian Wolff]

mwolff added a comment.


  Let's try to fix the bug for real, instead of implementing half-baked 
workarounds that only work for the default configurations.
  
  Alternative ideas: add setters for the two possible cell text colors, then 
somehow set the KColorScheme color from the outside.
  
  We really should upstream more of KColorScheme to Qt/QPalette...

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, #frameworks, cfeck
Cc: mwolff, apol, michaelh, ngraham, bruns

D12756: [KDateTable] Use a more visible red color

[Nathaniel Graham]

ngraham added a comment.


  That would change the color to something different (e.g. with the Breeze 
themes, it's blue). If we're okay with that I can do it, but I was trying to 
produce as minimal a patch as possible.

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, #frameworks, cfeck
Cc: mwolff, apol, michaelh, ngraham, bruns

D12756: [KDateTable] Use a more visible red color

[Milian Wolff]

mwolff added a comment.


  Maybe instead use the HighlightText QPalette color? Hardcoding red may work 
for the two styles you present, but I could just set the view background to red 
and the text becomes unusable.

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, #frameworks, cfeck
Cc: mwolff, apol, michaelh, ngraham, bruns

D12744: Add null pointer check when creating SocketAddress

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 33831.
chinmoyr added a comment.


  Used qPrintable()

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12744?vs=33820=33831

BRANCH
  master

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

AFFECTED FILES
  src/ioslaves/file/fdreceiver.cpp
  src/ioslaves/file/kauth/fdsender.cpp

To: chinmoyr, dfaure, ossi
Cc: #frameworks, michaelh, ngraham, bruns

D12756: [KDateTable] Use a more visible red color

[Nathaniel Graham]

ngraham added a comment.


  We can't because `KWidgetsAddons` is a tier 1 framework and can't rely on any 
other frameworks, and the positive/negative colors come from `KColorScheme` 
rather than anywhere in Qt.

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, #frameworks, cfeck
Cc: apol, michaelh, ngraham, bruns

D12756: [KDateTable] Use a more visible red color

[Aleix Pol Gonzalez]

apol added a comment.


  Shouldn't we be using a color from the palette/color scheme?

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, #frameworks, cfeck
Cc: apol, michaelh, ngraham, bruns

D9794: Provide access to data returned by helper even on error replies

[Alexander Volkov]

volkov added a comment.


  ping

REPOSITORY
  R283 KAuth

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

To: volkov, #frameworks
Cc: #frameworks, michaelh, ngraham, bruns

D12756: [KDateTable] Use a more visible red color

[Nathaniel Graham]

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

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, #frameworks, cfeck
Cc: michaelh, ngraham, bruns

D12756: [KDateTable] Use a more visible red color

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Frameworks, cfeck.
Restricted Application added a project: Frameworks.
ngraham requested review of this revision.

REVISION SUMMARY
  When I started work on this, I first wrote a conditional lightness check that 
switched between `Qt::red` and `Qt::darkRed` depending on the theme's window 
background color. That worked, but then noticed that `Qt::darkRed` isn't all 
that visible for Breeze light in the first place. `Qt::red` looks better and is 
more visible for both of them, so let's just use that instead!
  
  BUG: 389075
  FIXED-IN 5.47

TEST PLAN
  Breeze Light:
  
  Breeze Dark:

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  kdatetable-more-visible-red-color (branched from master)

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

AFFECTED FILES
  src/kdatetable.cpp

To: ngraham, #frameworks, cfeck
Cc: michaelh, ngraham, bruns

D12696: Use the new uds implementation

[Jaime Torres Amate]

jtamate added inline comments.

INLINE COMMENTS

> dfaure wrote in udsentry.cpp:454
> Hmm why can't this be the friend function directly?
> 
> I don't like the added global functions in the public header...

> Hmm why can't this be the friend function directly?

Because the compiler (clang++ in this case) doesn't know which one to apply.
Unless both definitions are equivalent (in source and binary) and the later can 
be removed. That is something I don't known.

  src/core/slavebase.cpp:728:14: error: use of overloaded operator '<<' is 
ambiguous (with operand types 'QDataStream' and 'const KIO::UDSEntry')
  KIO_DATA << entry;
   ^  ~
  src/core/udsentry.h:315:40: note: candidate function
  friend QDataStream << (QDataStream , const KIO::UDSEntry );
  ^
  src/core/udsentry.h:361:29: note: candidate function
  KIOCORE_EXPORT QDataStream << (QDataStream , const KIO::UDSEntry 
);
  ^

> I don't like the added global functions in the public header...

Me neither, but otherwise clang++will not be able to find the function.

  src/core/udsentry.h:314:19: error: no function named 'save' with type 'void 
(QDataStream &, const KIO::UDSEntry &)' was found in the specified scope
  friend void ::save(QDataStream &, const KIO::UDSEntry &);
^

> dfaure wrote in udsentry.cpp:45
> 1ms is relative to a benchmark which isn't clear when reading this code in 
> other contexts. As someone said in another review, do we still need this 
> operator== anyway, given that you pass lambdas to find_if()?

ok, ok, bye bye ==

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh, ngraham

D12744: Add null pointer check when creating SocketAddress

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> dfaure wrote in fdreceiver.cpp:36
> print out `m_path.toLocal8Bit()` here?

Possibly nitpicking, but toStdString converts to utf8 while toLocal8Bit() is 
what's used for SocketAddress and what's recommended for terminal output as 
well. If toLocal8Bit() doesn't compile, then you need 
m_path.toLocal8Bit().constData(). Or the short version for a cout/cerr 
statement:

  << qPrintable(m_path) << std::endl;

REPOSITORY
  R241 KIO

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

To: chinmoyr, dfaure, ossi
Cc: #frameworks, michaelh, ngraham, bruns

D12734: Scale up folder icon before creating preview overlays

[Peter Mühlenpfordt]

muhlenpfordt updated this revision to Diff 33822.
muhlenpfordt added a comment.


  Rebase to `Applications/18.04`

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12734?vs=33749=33822

BRANCH
  scale-up-folder-icon (branched from Applications/18.04)

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

AFFECTED FILES
  thumbnail/thumbnail.cpp

To: muhlenpfordt, #frameworks, rkflx
Cc: ngraham, rkflx

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham closed this revision.

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, ngraham, bruns

D12745: Unify API for file descriptor sharing

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 33821.
chinmoyr added a comment.


  Rebased on updated D12744 .

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12745?vs=33819=33821

BRANCH
  master

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

AFFECTED FILES
  src/ioslaves/file/fdreceiver.cpp
  src/ioslaves/file/fdreceiver.h
  src/ioslaves/file/file_unix.cpp
  src/ioslaves/file/kauth/fdsender.cpp
  src/ioslaves/file/kauth/fdsender.h
  src/ioslaves/file/sharefd_p.h

To: chinmoyr, dfaure, ossi
Cc: #frameworks, michaelh, ngraham, bruns

D12744: Add null pointer check when creating SocketAddress

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 33820.
chinmoyr added a comment.


  Print the invalid socket path in case of an error.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12744?vs=33769=33820

BRANCH
  master

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

AFFECTED FILES
  src/ioslaves/file/fdreceiver.cpp
  src/ioslaves/file/kauth/fdsender.cpp

To: chinmoyr, dfaure, ossi
Cc: #frameworks, michaelh, ngraham, bruns

D12745: Unify API for file descriptor sharing

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 33819.
chinmoyr added a comment.


  Print the invalid socket path in case of error.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12745?vs=33775=33819

BRANCH
  master

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

AFFECTED FILES
  src/ioslaves/file/fdreceiver.cpp
  src/ioslaves/file/kauth/fdsender.cpp

To: chinmoyr, dfaure, ossi
Cc: #frameworks, michaelh, ngraham, bruns

D12744: Add null pointer check when creating SocketAddress

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> fdreceiver.cpp:36
> +if (!addr.address()) {
> +std::cerr << "Invalid socket address" << std::endl;
> +return;

print out `m_path.toLocal8Bit()` here?

> fdsender.cpp:31
> +if (!addr.address()) {
> +std::cerr << "Invalid socket address" << std::endl;
> +return;

print out `path` here?

REPOSITORY
  R241 KIO

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

To: chinmoyr, dfaure, ossi
Cc: #frameworks, michaelh, ngraham, bruns

D12696: Use the new uds implementation

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> udsentry.cpp:454
>  
> -KIOCORE_EXPORT QDebug operator<<(QDebug stream, const KIO::UDSEntry )
> +KIOCORE_EXPORT QDataStream <<(QDataStream , const UDSEntry )
>  {

Hmm why can't this be the friend function directly?

I don't like the added global functions in the public header...

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh, ngraham

D12696: Use the new uds implementation

[Jaime Torres Amate]

jtamate updated this revision to Diff 33815.
jtamate added a comment.


  Make some friends functions.
  A bad type in copy didn't allowed me the use of the module functions 
save/load/debugUDSEntry,

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12696?vs=33809=33815

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

AFFECTED FILES
  src/core/kfileitem.cpp
  src/core/listjob.cpp
  src/core/udsentry.cpp
  src/core/udsentry.h

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh, ngraham

D12734: Scale up folder icon before creating preview overlays

[Henrik Fehlauer]

rkflx added a comment.


  In D12734#259445 , @muhlenpfordt 
wrote:
  
  > I wanted to be careful here since I'm a bit afraid of changing library 
functions. If you think it's ok for `Applications/18.04` I'll rebase it.
  
  
  How else would you fix problems in a library ;)
  
  Nevertheless, let's check the impact of this again: 
https://lxr.kde.org/ident?_i=thumbForDirectory
  
  Does not look too critical to me, and we probably tested most of the code 
paths.

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

To: muhlenpfordt, #frameworks, rkflx
Cc: ngraham, rkflx

D12734: Scale up folder icon before creating preview overlays

[Peter Mühlenpfordt]

muhlenpfordt added a comment.


  Forgot to mention this...
  There's a nice test application in `kio` for viewing a folder's preview in 
any desired size (created by `KIO::PreviewJob`): `previewtest`. Just enter the 
full path to the folder here.

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

To: muhlenpfordt, #frameworks, rkflx
Cc: ngraham, rkflx

D12696: Use the new uds implementation

[Jaime Torres Amate]

jtamate updated this revision to Diff 33809.
jtamate added a comment.


  Don't use java style.
  As UDSEntryPrivate is private, declare public methods save, load and 
debugUDSEntry, otherwise I couldn't access the private d pointer from the << 
and >> operators.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12696?vs=33758=33809

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

AFFECTED FILES
  src/core/kfileitem.cpp
  src/core/listjob.cpp
  src/core/udsentry.cpp
  src/core/udsentry.h

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh, ngraham