D17495: [KMessageBox] Fix minimum dialog size when details are requested

[Dominik Haumann]

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

REPOSITORY
  R236 KWidgetsAddons

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

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

KDE CI: Frameworks » networkmanager-qt » kf5-qt5 SUSEQt5.9 - Build # 69 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/networkmanager-qt/job/kf5-qt5%20SUSEQt5.9/69/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Tue, 11 Dec 2018 06:51:50 +
 Build duration:
3 min 6 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 test(s)Name: projectroot.autotests Failed: 0 test(s), Passed: 35 test(s), Skipped: 0 test(s), Total: 35 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(6/6)53%
(159/300)53%
(159/300)58%
(7697/13315)37%
(1878/5056)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(3/3)100%
(3/3)100%
(236/236)47%
(107/230)autotests.settings100%
(35/35)100%
(35/35)99%
(1680/1688)74%
(202/272)src27%
(27/99)27%
(27/99)27%
(975/3553)22%
(424/1912)src.dbus21%
(16/78)21%
(16/78)10%
(88/909)100%
(0/0)src.fakenetwork75%
(6/8)75%
(6/8)74%
(494/665)46%
(52/114)src.settings94%
(72/77)94%
(72/77)67%
(4224/6264)43%
(1093/2528)

KDE CI: Frameworks » networkmanager-qt » kf5-qt5 SUSEQt5.11 - Build # 7 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/networkmanager-qt/job/kf5-qt5%20SUSEQt5.11/7/
 Project:
kf5-qt5 SUSEQt5.11
 Date of build:
Tue, 11 Dec 2018 06:51:50 +
 Build duration:
3 min 14 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 test(s)Name: projectroot.autotests Failed: 0 test(s), Passed: 35 test(s), Skipped: 0 test(s), Total: 35 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(6/6)53%
(159/300)53%
(159/300)58%
(7711/13322)37%
(1879/5056)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(3/3)100%
(3/3)100%
(236/236)47%
(107/230)autotests.settings100%
(35/35)100%
(35/35)99%
(1680/1688)74%
(202/272)src27%
(27/99)27%
(27/99)28%
(983/3557)22%
(425/1912)src.dbus21%
(16/78)21%
(16/78)10%
(94/912)100%
(0/0)src.fakenetwork75%
(6/8)75%
(6/8)74%
(494/665)46%
(52/114)src.settings94%
(72/77)94%
(72/77)67%
(4224/6264)43%
(1093/2528)

D17497: A minor typo in ipv4 test

[Jan Grulich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R282:7036e0ade135: A minor typo in ipv4 test (authored by 
jgrulich).

REPOSITORY
  R282 NetworkManagerQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17497?vs=47321=47322

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

AFFECTED FILES
  autotests/settings/ipv4settingtest.cpp

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

D17497: A minor typo in ipv4 test

[Jan Grulich]

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

REPOSITORY
  R282 NetworkManagerQt

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

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

D17459: SearchBar: Add Cancel button to stop long running tasks

[Christoph Cullmann]

cullmann added a comment.


  If you get deleted in-between that, the slot is never called again by the 
timer.
  That means you are save.
  It will just naturally stop work if the widget/... is destroyed.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: brauch, cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, 
hase, michaelh, ngraham, bruns, demsking, sars, dhaumann

D17459: SearchBar: Add Cancel button to stop long running tasks

[loh tar]

loh.tar added a comment.


  > everything could have happened, e.g. the search bar got deleted because the 
view got delete
  
  OK, but I still do not see why that will be different with that singleShot 
approach.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: brauch, cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, 
hase, michaelh, ngraham, bruns, demsking, sars, dhaumann

D17436: BrightScript: Remove unused keyword list 'end'

[Daniel Levin]

dlevin added a comment.


  Thanks for the fix, yes it is valid. Previously "end" was used as a 
standalone keyword, but keyword requires space after the word. Now it used in 
StringDetect to detect cases like "end sub" and "endsub" in the same rule.

REPOSITORY
  R216 Syntax Highlighting

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

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

D17459: SearchBar: Add Cancel button to stop long running tasks

[Christoph Cullmann]

cullmann added a comment.


  After you return from processEvents, everything could have happened, e.g. the 
search bar got deleted because the view got delete => all things you do 
afterwards might do "something".

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: brauch, cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, 
hase, michaelh, ngraham, bruns, demsking, sars, dhaumann

D17474: A minor bug in ipv4 and ipv6 test

[Pranav Gade]

pranavgade added a comment.


  In D17474#374522 , @pranavgade 
wrote:
  
  > This change causes the ipv4 address test to fail due to a difference in 
size of list.
  >  [F6468448 ]
  
  
  D17497: A minor typo in ipv4 test  will 
fix this.

REPOSITORY
  R282 NetworkManagerQt

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17497: A minor typo in ipv4 test

[Pranav Gade]

pranavgade created this revision.
pranavgade added reviewers: jgrulich, cfeck.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
pranavgade requested review of this revision.

REVISION SUMMARY
  A minor typo in ipv4setting test was causing the test to fail after D17474: A 
minor bug in ipv4 and ipv6 test 
  Fixed that.

REPOSITORY
  R282 NetworkManagerQt

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

AFFECTED FILES
  autotests/settings/ipv4settingtest.cpp

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

D17496: Change network/web color icons to consistent style

[Noah Davis]

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

REPOSITORY
  R266 Breeze Icons

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

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

D17496: Change network/web color icons to consistent style

[Noah Davis]

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

REVISION SUMMARY
  The old style didn't match the sharp, grid aligned style of most Breeze icons 
and there was some inconsistency between network/web related color icons that 
used globes.

REPOSITORY
  R266 Breeze Icons

BRANCH
  network-icon-consistency (branched from master)

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

AFFECTED FILES
  icons-dark/apps/48/internet-web-browser.svg
  icons-dark/categories/32/applications-internet.svg
  icons-dark/preferences/32/applications-internet.svg
  icons-dark/preferences/32/preferences-system-network.svg
  icons-dark/preferences/32/preferences-web-browser-cache.svg
  icons-dark/preferences/32/preferences-web-browser-cookies.svg
  icons-dark/preferences/32/preferences-web-browser-identification.svg
  icons-dark/preferences/32/preferences-web-browser-shortcuts.svg
  icons-dark/preferences/32/preferences-web-browser-ssl.svg
  icons-dark/preferences/32/preferences-web-browser-stylesheets.svg
  icons/apps/48/internet-web-browser.svg
  icons/categories/32/applications-internet.svg
  icons/preferences/32/applications-internet.svg
  icons/preferences/32/preferences-system-network.svg
  icons/preferences/32/preferences-web-browser-cache.svg
  icons/preferences/32/preferences-web-browser-cookies.svg
  icons/preferences/32/preferences-web-browser-identification.svg
  icons/preferences/32/preferences-web-browser-shortcuts.svg
  icons/preferences/32/preferences-web-browser-ssl.svg
  icons/preferences/32/preferences-web-browser-stylesheets.svg

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

D17466: Fix doxygen markdown rendering

[Aleix Pol Gonzalez]

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


  As discussed or IRC

REPOSITORY
  R264 KApiDox

BRANCH
  fix_yaml_rendering

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

To: argonel, apol
Cc: apol, kde-frameworks-devel, kde-doc-english, michaelh, ngraham, bruns, 
skadinna

D17466: Fix doxygen markdown rendering

[Aleix Pol Gonzalez]

apol added a comment.


  ~~~{.yaml}
  hallo
  ~~~
  
  But then it doesn't work here. Is it maybe possible to force the usage of 
github markdown on it?

REPOSITORY
  R264 KApiDox

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

To: argonel
Cc: apol, kde-frameworks-devel, kde-doc-english, michaelh, ngraham, bruns, 
skadinna

D17466: Fix doxygen markdown rendering

[Eli MacKenzie]

argonel added a comment.


  The use of tildes for doxygen code blocks is documented here: 
https://www.stack.nl/~dimitri/doxygen/manual/markdown.html#md_fenced

REPOSITORY
  R264 KApiDox

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

To: argonel
Cc: apol, kde-frameworks-devel, kde-doc-english, michaelh, ngraham, bruns, 
skadinna

Re: KDE Frameworks web page

[Albert Astals Cid]

El dilluns, 10 de desembre de 2018, a les 14:43:14 CET, Jonathan Riddell va 
escriure:
> Frameworks has no web page and the announcements still point to the
> 5.0 announcement for more information.
> 
> So here's a proposal for adding to kde.org/products/
> 
> https://www.kde.org/products/frameworks/

That Qt logo looks weird (too narrow?)

> 
> Are the highlights still the best we can choose?
> 
> Are there any example applications we can show using a KDE Framework
> outwith KDE?

gammaray, libreoffice, vlc

Cheers,
  Albert

> 
> There's links at the bottom for getting the frameworks, are they the best 
> ones?
> 
> Phab item:
> https://phabricator.kde.org/T10175
> 
> Jonathan
> 

D17495: [KMessageBox] Fix minimum dialog size when details are requested

[Christoph Feck]

cfeck edited the test plan for this revision.
cfeck added reviewers: ngraham, elvisangelaccio.

REPOSITORY
  R236 KWidgetsAddons

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

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

D17495: [KMessageBox] Fix minimum dialog size when details are requested

[Christoph Feck]

cfeck created this revision.
Herald added a project: Frameworks.
Herald edited subscribers, added: kde-frameworks-devel; removed: Frameworks.
cfeck requested review of this revision.

REVISION SUMMARY
  QLabel with wordWrap enabled still needs the minimumSize workaround, which 
conflicts with the dialog setting its own minimumSize.
  
  BUG: 401466

TEST PLAN
  kmessageboxtest: When clicking the "Details >>" button the dialog window now 
resizes to show the details.

REPOSITORY
  R236 KWidgetsAddons

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

AFFECTED FILES
  src/kmessagebox.cpp

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

D17460: fixups for `extern` and `pragma`

[Dominik Haumann]

dhaumann added a comment.


  Could you also extend the unit test in autotest/input/highlight.d?
  
  See e.g.: 
https://github.com/KDE/syntax-highlighting/commit/66367f8e636623a2dbb8ee6add9c7f36683605a2

REPOSITORY
  R216 Syntax Highlighting

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

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

D17441: tune editing actions for large number of small edits

[Christoph Cullmann]

cullmann added a comment.


  In D17441#374925 , @ngraham wrote:
  
  > In D17441#374924 , @dhaumann 
wrote:
  >
  > > I guess it's ok to have this in. But please use this version at your 
daily work ;) I think we don't have many testers...
  >
  >
  > I always use Kate from git master. Is there anything in particular I should 
keep an eye out for?
  
  
  No, if I messed this up, all things should be randomly broken in some cases 
;=)

REPOSITORY
  R39 KTextEditor

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

To: cullmann, dhaumann, #kate, loh.tar
Cc: ngraham, anthonyfieroni, kwrite-devel, kde-frameworks-devel, hase, 
michaelh, bruns, demsking, cullmann, sars, dhaumann

D17441: tune editing actions for large number of small edits

[Nathaniel Graham]

ngraham added a comment.


  In D17441#374924 , @dhaumann wrote:
  
  > I guess it's ok to have this in. But please use this version at your daily 
work ;) I think we don't have many testers...
  
  
  I always use Kate from git master. Is there anything in particular I should 
keep an eye out for?

REPOSITORY
  R39 KTextEditor

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

To: cullmann, dhaumann, #kate, loh.tar
Cc: ngraham, anthonyfieroni, kwrite-devel, kde-frameworks-devel, hase, 
michaelh, bruns, demsking, cullmann, sars, dhaumann

D17441: tune editing actions for large number of small edits

[Dominik Haumann]

dhaumann added a comment.


  I guess it's ok to have this in. But please use this version at your daily 
work ;) I think we don't have many testers...

REPOSITORY
  R39 KTextEditor

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

To: cullmann, dhaumann, #kate, loh.tar
Cc: anthonyfieroni, kwrite-devel, kde-frameworks-devel, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D17459: SearchBar: Add Cancel button to stop long running tasks

[Christoph Cullmann]

cullmann added a comment.


  You can't really do that in threads, you need to access the buffer, otherwise 
you can redo all things again in extra logic (like using the ranges for getting 
replacement ranges)
  
  I still think the normal single shot stuff should do the trick in most cases.
  
  It is more work than the processEvents call, as one needs to re-adjust the 
code to be able to have some callable slot that does the batch processing.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: brauch, cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, 
hase, michaelh, ngraham, bruns, demsking, sars, dhaumann

D17459: SearchBar: Add Cancel button to stop long running tasks

[Sven Brauch]

brauch added a comment.


  I would also advise against calling processEvents() to keep UIs responsive in 
all cases. It's tempting, but it is near impossible to get it right. What about 
conflicting actions, close/resize events, dbus calls, etc etc that are handled 
here?
  
  I think the right way to implement this would be something like, copy the 
required data, run the expensive S with QtConcurrent, and replace it back 
into the UI when done.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: brauch, cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, 
hase, michaelh, ngraham, bruns, demsking, sars, dhaumann

D17459: SearchBar: Add Cancel button to stop long running tasks

[loh tar]

loh.tar added a comment.


  Hmpf! Some googling didn't help. Just some thoughts.
  
  - Adding a couple of checks where ever just to block in case of a running S 
could be a little cumbersome
  - What we need would be an QObject::processEvents, so that only the Cancle 
button is a live but no other stuff
  - Can we install an eventFilter and ignore all except our Cancel button?
  - QCoreApplication::processEvents take an optional argument e.g. 
QEventLoop::ExcludeUserInputEvents, nice but didn't help
  
  I have tried the S plugin with the fat file. That's smart enough to see 
that the task is too heavy and stopped after a couple of hits when searching. 
The Cancel button there (almost) didn't appear in this use case when invoke the 
"replace all" action. Obviously is it only updated between file changes or so. 
Hence, we can not profit from his functionality.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, 
michaelh, ngraham, bruns, demsking, sars, dhaumann

D17491: Fix documentation, QValueList isn't a thing anymore

[Aleix Pol Gonzalez]

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

REPOSITORY
  R237 KConfig

BRANCH
  master

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

AFFECTED FILES
  src/kconfig_compiler/README.dox

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

D17466: Fix doxygen markdown rendering

[Aleix Pol Gonzalez]

apol added a comment.


  If we are doing markdown, it could make sense why such common markdown isn't 
working too, otherwise this is bound to happen anyway. I've actually never seen 
this ~~~ thing.

REPOSITORY
  R264 KApiDox

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

To: argonel
Cc: apol, kde-frameworks-devel, kde-doc-english, michaelh, ngraham, bruns, 
skadinna

D17479: Fix build without phonon

[Heiko Becker]

This revision was automatically updated to reflect the committed changes.
Closed by commit R305:78a1dcc794f5: Fix build without Phonon (authored by 
heikobecker).

REPOSITORY
  R305 KNotifyConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17479?vs=47283=47288

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

AFFECTED FILES
  src/CMakeLists.txt

To: heikobecker, sitter
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17469: applications-games icon for dark theme

[Andres Betts]

abetts added a comment.


  It's looking good. I would say change the last tone to a tad lighter. The 
gray has too much black in it. I would probably move the hue to the yellow side.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, #breeze, #vdg, ndavis
Cc: abetts, ngraham, pstefan, ndavis, kde-frameworks-devel, michaelh, bruns

D17479: Fix build without phonon

[Harald Sitter]

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


  Oh la la, I didn't even realize phonon was optional. New code is much better.

REPOSITORY
  R305 KNotifyConfig

BRANCH
  master

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

To: heikobecker, sitter
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: Added DCB settings

[Pranav Gade]

pranavgade retitled this revision from "dcb settings" to "Added DCB settings".
pranavgade edited the summary of this revision.

REPOSITORY
  R282 NetworkManagerQt

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

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

D17425: dcb settings

[Christoph Feck]

cfeck added a comment.


  Please keep the Repository settings. Additionally, please format the title 
and description according to 
https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch

REPOSITORY
  R282 NetworkManagerQt

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

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

D17480: Fix warnings

[Aleix Pol Gonzalez]

apol closed this revision.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: apol, #frameworks, mart, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17425: dcb settings

[Christoph Feck]

cfeck set the repository for this revision to R282 NetworkManagerQt.

REPOSITORY
  R282 NetworkManagerQt

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17479: Fix build without phonon

[Kai Uwe Broulik]

broulik added a reviewer: sitter.

REPOSITORY
  R305 KNotifyConfig

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

To: heikobecker, sitter
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17480: Fix warnings

[Kai Uwe Broulik]

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

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

BRANCH
  master

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

To: apol, #frameworks, mart, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17480: Fix warnings

[Aleix Pol Gonzalez]

apol created this revision.
apol added reviewers: Frameworks, mart.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
apol requested review of this revision.

REVISION SUMMARY
  Now it doesn't complain about being undefined anymore

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

BRANCH
  master

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

AFFECTED FILES
  org.kde.desktop/TextArea.qml
  org.kde.desktop/TextField.qml

To: apol, #frameworks, mart
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17479: Fix build without phonon

[Heiko Becker]

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

REVISION SUMMARY
  f6d55baf5aa88eaab6b2f96c025064f081d13cea 
 
replaced ${PHONON_LIBS} with
  Phonon's imported target. This breaks in the case when Phonon isn't
  found or disabled via -DCMAKE_DISABLE_FIND_PACKAGES_Phonon4Qt5=TRUE
  because the imported target isn't known. It worked previously because
  was ${PHONON_LIBS} just empty when Phonon wasn't available.

TEST PLAN
  Building without phonon works now and and it still builds
  with phonon available and successfully links to it.

REPOSITORY
  R305 KNotifyConfig

BRANCH
  master

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

AFFECTED FILES
  src/CMakeLists.txt

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

D17425: dcb settings

[Pranav Gade]

pranavgade updated this revision to Diff 47282.
pranavgade marked an inline comment as done.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17425?vs=47279=47282

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/dcbsettingtest.cpp
  autotests/settings/dcbsettingtest.h
  src/CMakeLists.txt
  src/settings/dcbsetting.cpp
  src/settings/dcbsetting.h
  src/settings/dcbsetting_p.h
  src/settings/setting.cpp
  src/settings/setting.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17469: applications-games icon for dark theme

[Noah Davis]

ndavis added a comment.


  In D17469#374557 , @guoyunhe wrote:
  
  > In D17469#374494 , @ndavis wrote:
  >
  > > Here's a rough draft of something similar to the Logitech F 710, but with 
colors that are more similar to the original gray DualShock controller 
:
  > >
  > > 100% F6468233: Screenshot_20181210_062352.png 

  > >
  > > 200% F6468236: Screenshot_20181210_062557.png 

  > >
  > > F6468246: applications-games2.svg 
  >
  >
  > Maybe it is better to remove the dark sides? Only F710 
 has this design. Most controllers, including 
PS4 and XBox One, are pure white or black or other colors.
  >
  > F6468713: image.png 
  
  
  We'll need something to make the handles look somewhat separate from the 
body. If we don't do that with side grips, maybe we can do that with gradients. 
I don't think we need to match the most commonly used controllers though since 
users will still know it's a game controller. I just used that look because I 
liked the contrast and symmetry.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, #breeze, #vdg, ndavis
Cc: ngraham, pstefan, ndavis, kde-frameworks-devel, michaelh, bruns

D17425: dcb settings

[Jan Grulich]

jgrulich added a comment.


  This was last round, otherwise it's ready to go, just fix the remaining 
issues. Thanks.

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

> dcbsettingtest.cpp:28
> +#include 
> +#include 
> +

Include not needed.

> dcbsetting.cpp:220
> +} else {
> +return false;
> +}

You can remove the else branch and have just return false.

> dcbsetting.cpp:254
> +} else {
> +return 0;
> +}

You can remove the else branch and have just return 0.

> dcbsetting.cpp:288
> +} else {
> +return 0;
> +}

You can remove the else branch and have just return 0.

> dcbsetting.cpp:322
> +} else {
> +return 0;
> +}

You can remove the else branch and have just return 0.

> dcbsetting.cpp:356
> +} else {
> +return false;
> +}

You can remove the else branch and have just return false.

> dcbsetting.cpp:390
> +} else {
> +return 0;
> +}

You can remove the else branch and have just return 0.

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17469: applications-games icon for dark theme

[Noah Davis]

ndavis added a comment.


  In D17469#374605 , @ngraham wrote:
  
  > This is another example where a subtle outline could help visibility for 
dark themes, without having to resort to drawing a whole new version of it.
  
  
  An outline would not look very subtle on this shape at this size. I think it 
needs to be redesigned anyway. As I said in a previous comment, the proportions 
look weird.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, #breeze, #vdg, ndavis
Cc: ngraham, pstefan, ndavis, kde-frameworks-devel, michaelh, bruns

D17425: dcb settings

[Pranav Gade]

pranavgade updated this revision to Diff 47279.
pranavgade marked 4 inline comments as done.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17425?vs=47270=47279

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/dcbsettingtest.cpp
  autotests/settings/dcbsettingtest.h
  src/CMakeLists.txt
  src/settings/dcbsetting.cpp
  src/settings/dcbsetting.h
  src/settings/dcbsetting_p.h
  src/settings/setting.cpp
  src/settings/setting.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Christoph Feck]

cfeck set the repository for this revision to R282 NetworkManagerQt.

REPOSITORY
  R282 NetworkManagerQt

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17469: applications-games icon for dark theme

[Nathaniel Graham]

ngraham added subscribers: pstefan, ngraham.
ngraham added a comment.


  This is another example where a subtle outline could help visibility for dark 
themes, without having to resort to drawing a whole new version of it.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, #breeze, #vdg, ndavis
Cc: ngraham, pstefan, ndavis, kde-frameworks-devel, michaelh, bruns

D17459: SearchBar: Add Cancel button to stop long running tasks

[loh tar]

loh.tar added a comment.


  
  
  > That always leads to evil things, like e.g. what happens if you press the X 
button of the view/window during that.
  
  I assume you have read that article a longer time ago and have these 
paragraph regarding `Manual event processing` in mind.
  
  > This approach has significant drawbacks. For example...
  
  There are three listed
  
  1. you can't distribute computing power among different tasks
  2. makes the application react with delays to events
  3. the code is difficult to read and analyze
  
  I can't see any hint in the whole article that would avoid to choose some 
action. It's all about keeping the GUI snappy. And that is always done by enter 
the event loop.
  
  I think the points 1+2 are not important here. Point 3 do I not understand, 
at least the "read" part, but I think we have here not much more to analyze 
than if the Cancel-button works or not.
  
  The S process is not broken in some unclear state. There will only 
probably remaining matches not replaced.
  
  You mean what happens e.g. when the user continue to edit while the S is 
running. I have just tried it with our good known fat file. It's funny. You can 
scroll, edit and almost watch how it progress.
  
  There may the need to add some blocking for that. Is it possible to flag the 
document as "read only" without to break the started S?
  
  There may the need to block other things too, like your mentioned view/window 
close.
  
  One hint there was that timers are not fire without the event loop. I have 
tried this, and yes, no effect to check if a single shot timer isActive() or 
not. So I updated/fix the modulo check slightly because I noticed a lag in some 
cases. But may still not the best solution.
  
  ATM is my conclusion: Your suggested re-design may fix probably responsive 
lags but not the problem to block ugly actions. Because this whole patch is 
only in rare cases really needed I would like avoid the effort to re-design the 
logic.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, 
michaelh, ngraham, bruns, demsking, sars, dhaumann

D17459: SearchBar: Add Cancel button to stop long running tasks

[loh tar]

loh.tar updated this revision to Diff 47275.
loh.tar added a comment.


  - Check every 500 lines or 100 machtes, whatever is first
  - Fix false check due to wrong math as long no match was found

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17459?vs=47208=47275

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

AFFECTED FILES
  src/search/katesearchbar.cpp
  src/search/katesearchbar.h
  src/search/searchbarpower.ui

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, 
michaelh, ngraham, bruns, demsking, sars, dhaumann

D17425: dcb settings

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

> dcbsettingtest.cpp:30
> +
> +typedef QList UintList;
> +

Why? This type is defined in generictypes.h.

> dcbsetting.h:30
> +
> +typedef QList UintList;
> +

Again, this is defined in generictypes.h.

> pranavgade wrote in dcbsetting.h:49
> I tried to do it in the same way as in ipv6 settings, so should i add 
> Q_DECLARE_FLAGS() ?

Yes, add Q_DECLARE_FLAGS.

> dcbsetting_p.h:27
> +
> +typedef QList UintList;
> +

Here as well.

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

KDE Frameworks web page

[Jonathan Riddell]

Frameworks has no web page and the announcements still point to the
5.0 announcement for more information.

So here's a proposal for adding to kde.org/products/

https://www.kde.org/products/frameworks/

Are the highlights still the best we can choose?

Are there any example applications we can show using a KDE Framework
outwith KDE?

There's links at the bottom for getting the frameworks, are they the best ones?

Phab item:
https://phabricator.kde.org/T10175

Jonathan

D17469: applications-games icon for dark theme

[Yunhe Guo]

guoyunhe added a comment.


  In D17469#374494 , @ndavis wrote:
  
  > Here's a rough draft of something similar to the Logitech F 710, but with 
colors that are more similar to the original gray DualShock controller 
:
  >
  > 100% F6468233: Screenshot_20181210_062352.png 

  >
  > 200% F6468236: Screenshot_20181210_062557.png 

  >
  > F6468246: applications-games2.svg 
  
  
  Maybe it is better to remove the dark sides? Only F710 
 has this design. Most controllers, including 
PS4 and XBox One, are pure white or black or other colors.
  
  F6468713: image.png 

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, #breeze, #vdg, ndavis
Cc: ndavis, kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Pranav Gade]

pranavgade added a comment.


  Sorry, I had forgot about that.

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Pranav Gade]

pranavgade updated this revision to Diff 47270.
pranavgade marked 6 inline comments as done.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17425?vs=47267=47270

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/dcbsettingtest.cpp
  autotests/settings/dcbsettingtest.h
  src/CMakeLists.txt
  src/settings/dcbsetting.cpp
  src/settings/dcbsetting.h
  src/settings/dcbsetting_p.h
  src/settings/setting.cpp
  src/settings/setting.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

KDE CI: Frameworks » networkmanager-qt » kf5-qt5 SUSEQt5.11 - Build # 6 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/networkmanager-qt/job/kf5-qt5%20SUSEQt5.11/6/
 Project:
kf5-qt5 SUSEQt5.11
 Date of build:
Mon, 10 Dec 2018 12:03:36 +
 Build duration:
14 min and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 test(s)Name: projectroot.autotests Failed: 1 test(s), Passed: 34 test(s), Skipped: 0 test(s), Total: 35 test(s)Failed: projectroot.autotests.settings.ipv4settingtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(6/6)53%
(159/300)53%
(159/300)58%
(7687/13321)37%
(1863/5056)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(3/3)100%
(3/3)100%
(236/236)47%
(107/230)autotests.settings100%
(35/35)100%
(35/35)99%
(1677/1688)73%
(199/272)src27%
(27/99)27%
(27/99)28%
(983/3557)22%
(425/1912)src.dbus21%
(16/78)21%
(16/78)10%
(94/912)100%
(0/0)src.fakenetwork75%
(6/8)75%
(6/8)74%
(494/665)46%
(52/114)src.settings94%
(72/77)94%
(72/77)67%
(4203/6263)43%
(1080/2528)

KDE CI: Frameworks » networkmanager-qt » kf5-qt5 SUSEQt5.9 - Build # 68 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/networkmanager-qt/job/kf5-qt5%20SUSEQt5.9/68/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Mon, 10 Dec 2018 12:03:36 +
 Build duration:
4 min 5 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 test(s)Name: projectroot.autotests Failed: 1 test(s), Passed: 34 test(s), Skipped: 0 test(s), Total: 35 test(s)Failed: projectroot.autotests.settings.ipv4settingtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(6/6)53%
(159/300)53%
(159/300)58%
(7671/13314)37%
(1861/5056)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(3/3)100%
(3/3)100%
(236/236)47%
(107/230)autotests.settings100%
(35/35)100%
(35/35)99%
(1677/1688)73%
(199/272)src27%
(27/99)27%
(27/99)27%
(973/3553)22%
(423/1912)src.dbus21%
(16/78)21%
(16/78)10%
(88/909)100%
(0/0)src.fakenetwork75%
(6/8)75%
(6/8)74%
(494/665)46%
(52/114)src.settings94%
(72/77)94%
(72/77)67%
(4203/6263)43%
(1080/2528)

D17425: dcb settings

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

> dcbsetting.h:89
> +bool priorityFlowControl(quint32 userPriority) const;
> +void setPriorityFlowControl(UintList list);
> +UintList priorityFlowControl() const;

const UintList 

> dcbsetting.h:94
> +quint32 priorityBandwidth(quint32 userPriority) const;
> +void setPriorityBandwidth(UintList list);
> +UintList priorityBandwidth() const;

const UintList 

> dcbsetting.h:99
> +quint32 priorityGroupBandwidth(quint32 groupId) const;
> +void setPriorityGroupBandwidth(UintList list);
> +UintList priorityGroupBandwidth() const;

const UintList 

> dcbsetting.h:104
> +quint32 priorityGroupId(quint32 userPriority) const;
> +void setPriorityGroupId(UintList list);
> +UintList priorityGroupId() const;

const UintList 

> dcbsetting.h:109
> +bool priorityStrictBandwidth(quint32 userPriority) const;
> +void setPriorityStrictBandwidth(UintList list);
> +UintList priorityStrictBandwidth() const;

const UintList 

> dcbsetting.h:114
> +quint32 priorityTrafficClass(quint32 userPriority) const;
> +void setPriorityTrafficClass(UintList list);
> +UintList priorityTrafficClass() const;

const UintList 

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17474: A minor bug in ipv4 and ipv6 test

[Jan Grulich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R282:2edc30be9f3e: A minor bug in ipv4 and ipv6 test (authored 
by jgrulich).

REPOSITORY
  R282 NetworkManagerQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17474?vs=47268=47269

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

AFFECTED FILES
  autotests/settings/ipv4settingtest.cpp
  autotests/settings/ipv6settingtest.cpp

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17474: A minor bug in ipv4 and ipv6 test

[Pranav Gade]

pranavgade added a comment.


  This change causes the ipv4 address test to fail due to a difference in size 
of list.
  [F6468448 ]

REPOSITORY
  R282 NetworkManagerQt

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17474: A minor bug in ipv4 and ipv6 test

[Pranav Gade]

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

REVISION SUMMARY
  Some QCOMPAREs were comparing a value in a map with itself.
  Fixed.

REPOSITORY
  R282 NetworkManagerQt

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

AFFECTED FILES
  autotests/settings/ipv4settingtest.cpp
  autotests/settings/ipv6settingtest.cpp

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Pranav Gade]

pranavgade updated this revision to Diff 47267.
pranavgade marked an inline comment as done.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17425?vs=47263=47267

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/dcbsettingtest.cpp
  autotests/settings/dcbsettingtest.h
  src/CMakeLists.txt
  src/settings/dcbsetting.cpp
  src/settings/dcbsetting.h
  src/settings/dcbsetting_p.h
  src/settings/setting.cpp
  src/settings/setting.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

> dcbsetting.h:106
> +/
> +void setPriorityFlowControlList(UintList list);
> +UintList priorityFlowControlList() const;

Move them please among the rest of methods, so all related methods are next to 
each other. Also use please "const UintList " and do not name the method 
as setFooList(), they can be just setFoo().

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17469: applications-games icon for dark theme

[Noah Davis]

ndavis added a comment.


  Here's a rough draft of something similar to the Logitech F 710, but with 
colors that are more similar to the original DualShock controller and white 
Xbox 360 controller:
  
  100% F6468233: Screenshot_20181210_062352.png 

  
  200% F6468236: Screenshot_20181210_062557.png 


REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, #breeze, #vdg, ndavis
Cc: ndavis, kde-frameworks-devel, michaelh, ngraham, bruns

D17419: Add view-private icon

[Noah Davis]

ndavis added a comment.


  Here's a rough draft of something similar to the F710 
, but with colors that are more similar to 
the original DualShock controller and white Xbox 360 controller:
  
  100% F6468233: Screenshot_20181210_062352.png 

  
  200% F6468236: Screenshot_20181210_062557.png 


REPOSITORY
  R266 Breeze Icons

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

To: GB_2, #breeze, #vdg, #falkon, ngraham, ndavis
Cc: drosca, ndavis, filipf, ngraham, #vdg, kde-frameworks-devel, #breeze, 
squeakypancakes, alexde, IohannesPetros, trickyricky26, michaelh, crozbo, 
firef, bruns, skadinna, aaronhoneycutt, mbohlender

D17425: dcb settings

[Pranav Gade]

pranavgade updated this revision to Diff 47263.
pranavgade marked 4 inline comments as done.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17425?vs=47261=47263

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/dcbsettingtest.cpp
  autotests/settings/dcbsettingtest.h
  src/CMakeLists.txt
  src/settings/dcbsetting.cpp
  src/settings/dcbsetting.h
  src/settings/dcbsetting_p.h
  src/settings/setting.cpp
  src/settings/setting.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Pranav Gade]

pranavgade added inline comments.

INLINE COMMENTS

> jgrulich wrote in dcbsetting.cpp:59
> Yes please.

Okay, im on it
But on the other hand, it will add 12 functions unnecessarily

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

> pranavgade wrote in dcbsetting.cpp:59
> Yeah, that would help.
> Do you think I should do that?

Yes please.

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Pranav Gade]

pranavgade updated this revision to Diff 47261.
pranavgade marked 2 inline comments as done.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17425?vs=47253=47261

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/dcbsettingtest.cpp
  autotests/settings/dcbsettingtest.h
  src/CMakeLists.txt
  src/settings/dcbsetting.cpp
  src/settings/dcbsetting.h
  src/settings/dcbsetting_p.h
  src/settings/setting.cpp
  src/settings/setting.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

> dcbsetting.cpp:59
> +
> +for (int i = 0; i < 8; ++i) setPriorityFlowControl(i, 
> other->priorityFlowControl(i));
> +for (int i = 0; i < 8; ++i) setPriorityBandwidth(i, 
> other->priorityBandwidth(i));

This is odd, maybe we should introduce getters and setters for the whole list, 
this would simplify it and you can use it also in fromMap() and toMap()

> dcbsetting.cpp:221
> +} else {
> +return 0;
> +}

Return false.

> dcbsetting.cpp:301
> +} else {
> +return 0;
> +}

Return false.

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17469: applications-games icon for dark theme

[Noah Davis]

ndavis added reviewers: VDG, ndavis.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, #breeze, #vdg, ndavis
Cc: ndavis, kde-frameworks-devel, michaelh, ngraham, bruns

D17469: applications-games icon for dark theme

[Noah Davis]

ndavis added a comment.


  In general, color icons are the same in Breeze and Breeze Dark while 
monochrome icons change their primary color. I think the light and dark 
versions of this icons should be the same.
  
  I think the overall design of this icon needs to be changed though. The 
proportions look odd to me. I think basing it on the F310 
 or F710 

 would be a good idea since it's already similar to those. Those controllers 
are a hybrid of the Xbox and DualShock controller designs.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, #breeze
Cc: ndavis, kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Pranav Gade]

pranavgade marked 12 inline comments as done.

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16694: Improve visibility for Konsole icon when using Breeze Dark

[Emirald Mateli]

emateli added a comment.


  @anishgiri What is the advantage of using this outlined version of the 
Konsole icon(which we haven't seen in a light scheme yet) compared to using one 
of the monochrome icons already available?

REPOSITORY
  R266 Breeze Icons

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

To: anishgiri, ngraham, #vdg
Cc: emateli, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17469: applications-games icon for dark theme

[Yunhe Guo]

guoyunhe updated this revision to Diff 47254.
guoyunhe added a comment.


  Clean up Inkscape metadata

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17469?vs=47251=47254

BRANCH
  master

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

AFFECTED FILES
  icons-dark/categories/32/applications-games.svg

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

D17425: dcb settings

[Pranav Gade]

pranavgade updated this revision to Diff 47253.
pranavgade marked 15 inline comments as done.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17425?vs=47243=47253

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/dcbsettingtest.cpp
  autotests/settings/dcbsettingtest.h
  src/CMakeLists.txt
  src/settings/dcbsetting.cpp
  src/settings/dcbsetting.h
  src/settings/dcbsetting_p.h
  src/settings/setting.cpp
  src/settings/setting.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Pranav Gade]

pranavgade added inline comments.

INLINE COMMENTS

> jgrulich wrote in dcbsetting.cpp:464
> Oh, right, you don't have methods for that. In that case you can again use 
> Q_D(const DcbSetting) and push there d->priorityFlowControl and others.

I can use Q_D in fromMap, but i get this error in toMap:

error: invalid conversion from ‘const NetworkManager::DcbSettingPrivate*’ to 
‘NetworkManager::DcbSettingPrivate*’ [-fpermissive] Q_D(DcbSetting);

so, changed frommap, but keeping this as is.

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17469: applications-games icon for dark theme

[Yunhe Guo]

guoyunhe updated this revision to Diff 47251.
guoyunhe added a comment.


  Add modified icon with light background

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17469?vs=47245=47251

BRANCH
  master

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

AFFECTED FILES
  icons-dark/categories/32/applications-games.svg

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

D17469: applications-games icon for dark theme

[Yunhe Guo]

guoyunhe edited the summary of this revision.
guoyunhe added a reviewer: Breeze.

REPOSITORY
  R266 Breeze Icons

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

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

D17469: applications-games icon for dark theme

[Yunhe Guo]

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

REPOSITORY
  R266 Breeze Icons

BRANCH
  master

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

AFFECTED FILES
  icons-dark/categories/32/applications-games.svg

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

D17425: dcb settings

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

> dcbsetting.cpp:53
> +
> +for (int i = 0; i < 8; ++i) setPriorityFlowControl(i, 
> other->priorityFlowControl(i));
> +for (int i = 0; i < 8; ++i) setPriorityBandwidth(i, 
> other->priorityBandwidth(i));

Not this way.

> pranavgade wrote in dcbsetting.cpp:31
> how do i do that?
> i am getting this error:
> error: class ‘NetworkManager::DcbSettingPrivate’ does not have any field 
> named ‘setPriorityFlowControl’

something like priorityFlowControl({0, 0, 0, 0, 0, 0, 0, 0, 0}) should work, or 
not? Same for othe lists.

> pranavgade wrote in dcbsetting.cpp:197
> I think it is better to have this as a safety measure, and initialising so 
> many values manually wold take 8*6=48 lines in the beginning of the file

It should simple by just:

  Q_D(DcbSetting)
  
  if (userPriority < 8) {
 d->priorityFlowControl[userPriority] = enabled;
  }

> pranavgade wrote in dcbsetting.cpp:212
> that may lead to a malloc assertion error sometimes, and thus a crash which 
> is confusing to understand

Not if you initialize it at the beginning as it should be.

> pranavgade wrote in dcbsetting.cpp:393
> I get a type casting error

You can have Q_D(DcbSetting) at the beginning of fromMap() function and then 
instead of setFoo() have d->foo = list.

> pranavgade wrote in dcbsetting.cpp:464
> How?

Oh, right, you don't have methods for that. In that case you can again use 
Q_D(const DcbSetting) and push there d->priorityFlowControl and others.

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Pranav Gade]

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

INLINE COMMENTS

> jgrulich wrote in dcbsetting.cpp:31
> You can initialize also all the lists here, because they have default values 
> as well.

how do i do that?
i am getting this error:
error: class ‘NetworkManager::DcbSettingPrivate’ does not have any field named 
‘setPriorityFlowControl’

> jgrulich wrote in dcbsetting.cpp:197
> If you initialize the list at the beginning, you will not need to have this 
> check and also do this weird thing below. Simply have the first check and 
> otherwise you can do just d->priorityFlowControl[userPriority] = enabled.

I think it is better to have this as a safety measure, and initialising so many 
values manually wold take 8*6=48 lines in the beginning of the file

> jgrulich wrote in dcbsetting.cpp:212
> Check just if userPriority is < 8.

that may lead to a malloc assertion error sometimes, and thus a crash which is 
confusing to understand

> jgrulich wrote in dcbsetting.cpp:393
> You can directly assign whole UintList. Same for the rest of options below.

I get a type casting error

> jgrulich wrote in dcbsetting.cpp:427
> Remove comments.

oops, i was debugging it

> jgrulich wrote in dcbsetting.cpp:428
> Shouldn't it save appFcoeMode() instead?

yeah, i uploaded some unfinished code

> jgrulich wrote in dcbsetting.cpp:464
> You can directly push the list there. Same for the lists below.

How?

> jgrulich wrote in dcbsetting.h:49
> You are missing Q_DECLARE_FLAGS()

I tried to do it in the same way as in ipv6 settings, so should i add 
Q_DECLARE_FLAGS() ?

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17425: dcb settings

[Pranav Gade]

pranavgade updated this revision to Diff 47243.
pranavgade marked 7 inline comments as done.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17425?vs=47235=47243

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/dcbsettingtest.cpp
  autotests/settings/dcbsettingtest.h
  src/CMakeLists.txt
  src/settings/dcbsetting.cpp
  src/settings/dcbsetting.h
  src/settings/dcbsetting_p.h
  src/settings/setting.cpp
  src/settings/setting.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns