D6331: Make sure that the tsfiles target is generated

2017-07-04 Thread Luigi Toscano 
ltoscano added a comment.


  It seems to work, just one question though:

INLINE COMMENTS

> KF5I18NMacros.cmake:178
> +  set(pmapc_target "pmapfiles-${lang}")
> +  add_custom_target(${pmapc_target} ALL DEPENDS ${pmapc_files})
> +  add_dependencies(pmapfiles ${pmapc_target})

Before this line, in the original version before the changes, there was this 
line:

  string(REPLACE "@" "_" pmapc_target ${pmapc_target})

Could it be still relevant?

REPOSITORY
  R249 KI18n

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

To: apol, #frameworks, ltoscano, lueck, aacid
Cc: aacid, asturmlechner

Re: KTitleWidget and the native Mac style

2017-07-04 Thread René J . V . Bertin 
On Tuesday July 04 2017 20:16:55 Sebastian Kügler wrote:

>The frame in my understanding is old weight, and can go (do check with the VDG 

I noticed it was already there in KDE4, indeed, though not usually rendered.

>but to me, more importantly, reducing its size will 
>lead to regressions in Plasma. 

Avoiding regressions is evident, but how could changing the size lead to 
regressions in properly written code (which should be able to handle changes in 
font weight, style or size)?

>I have no problems with moving its rendering to the style so it can depend on 
>the platform used, or another technically sound solution, but a change should 
>not introduce regressions in Plasma.

Note that I wasn't particularly concerned with the size, but I did think about 
this. I think widget styles can influence the size of the font used in QLabels 
but that could be tricky because it affects the widget size. There are 
typically more QLabels in a view than QFrames which may make runtime detection 
of KTitleWidget labels (through inheritance) a bit expensive.

Adding a font role (or repurposing the window titlebar font role) would be the 
most logical approach, but not easily deployed.

Maybe we could simply use QFontDatabase::SystemFont::TitleFont instead of 
imposing a hardcoded size increment?

R.

Re: KTitleWidget and the native Mac style

2017-07-04 Thread Sebastian Kügler 
On dinsdag 4 juli 2017 17:55:01 CEST René J.V. Bertin wrote:
> Me neither for myself, I'm happy with how my QtCurve theme looks but I'm
> also trying to improve the way KDE software looks using the native style.

The frame in my understanding is old weight, and can go (do check with the VDG 
though!), but the larger font isn't. It's semantically a sane choice (titles 
often have larger fonts), but to me, more importantly, reducing its size will 
lead to regressions in Plasma. 

I have no problems with moving its rendering to the style so it can depend on 
the platform used, or another technically sound solution, but a change should 
not introduce regressions in Plasma.

Cheers,
-- 
sebas

http://www.kde.org | http://vizZzion.org

Re: KTitleWidget and the native Mac style

2017-07-04 Thread René J . V . Bertin 
Hi,

How about this?

In KWidgetAddons (out-commented code shows "stock" 5.35.0):

KTitleWidget::KTitleWidget(QWidget *parent)
: QWidget(parent),
  d(new Private(this))
{
QFrame *titleFrame = new QFrame(this);
// titleFrame->setAutoFillBackground(true);
// titleFrame->setFrameShape(QFrame::StyledPanel);
titleFrame->setFrameShadow(QFrame::Plain);
// titleFrame->setBackgroundRole(QPalette::Base);

In breeze/kstyle/breezestyle.cpp (Style::polish(QWidget*)):

} else if( qobject_cast( widget ) && widget->parent() && 
widget->parent()->inherits( "KTitleWidget" ) ) {

// widget->setAutoFillBackground( false );
// if( !StyleConfigData::titleWidgetDrawFrame() )
// { widget->setBackgroundRole( QPalette::Window ); }
QFrame *frame = qobject_cast( widget );
if( StyleConfigData::titleWidgetDrawFrame() ) {
frame->setAutoFillBackground( true );
frame->setFrameShape( QFrame::StyledPanel );
frame->setBackgroundRole( QPalette::Base );
} else {
frame->setAutoFillBackground( false );
frame->setFrameShape( QFrame::NoFrame );
frame->setBackgroundRole( QPalette::Window );
}

}

That moves the entire choice of the title frame rendering style to the style 
plugin and the end result looks the same (I didn't to a pixel analysis yet).

R.

Re: KTitleWidget and the native Mac style

2017-07-04 Thread René J . V . Bertin 
On Tuesday July 04 2017 18:02:13 Hugo Pereira Da Costa wrote:

> One should really consult with vdg here, since all these points (larger 
> fonts, padding, no frame), was already discussed with them, and agreed 
> upon, back in the days.

As far as the official 1 or 2 KDE styles are concerned, I presume (hope).

R.

Re: KTitleWidget and the native Mac style

2017-07-04 Thread René J . V . Bertin 
On Tuesday July 04 2017 15:49:28 Kevin Funk wrote:

>With regards to the "Widget Style and Behaviour" label in the `kcmshell5 
>style` dialog: I don't find that particularly attracting under Breeze as well.
>
>It's too "bulky" for my taste. 

Indeed. I just notice that contrary to what you'd expect, unticking "draw a 
frame around page titles" in the Breeze settings only makes the frame invisible 
but does NOT remove the space allocated for it. It would probably look a lot 
less bulky without that useless padding, or if it adopted the same height as 
the text to look more like a banner.

I agree it isn't particularly necessary to use a bigger font, but that too is a 
bit subject to personal taste. It would probably have made more sense to 
introduce a new font role/category for this (or repurpose the Window Title font 
role).

>Maybe we can find a solution that works well under both styles?

Hmmm, Breeze already checks if widgets inherit KTitleWidget in order to decide 
if it must apply the aforementioned setting 
(StyleConfigData::titleWidgetDrawFrame()). That means one could obtain the same 
result as far as the frame is concerned even if KTitleWidget does NOT create a 
filled and outlined frame itself. The QtCurve style also checks for 
KTitleWidgets (styles shipped by Qt evidently won't do that).

IOW, KTitleWidget only needs to create a transparent/invisible QFrame. Breeze 
and QtCurve can decide whether and how to render it much like they already do; 
Oxygen already skips renders it invisibly.

Alternatively the KTitleWidget ctor could try to read the TitleWidgetDrawFrame 
key from breezerc with a default of False. That should cover things on 
non-Plasma DEs, but it's not very elegant.

I don't think the platform theme plugin can be of real influence here, can it? 
FWIW, I do have a standalone fork of the Macintosh widget style (from Qt 5.9, 
works with 5.8 too) in which I could add KDE-specific code but that would 
benefit only those applications that embed the plugin.

>This patch removes the bold weight from KTitleWidget and makes the text
> a bit bigger, improving focus. This is more in line with common
> expectations of a title, and it's more in line with Plasma 5's
> typography.

I'm not sure what my common expectations are for a dialog title but I think 
they'd put the title text in the window titlebar. Or else centred horizontally, 
possibly with underlining. Maybe a style could underline text over the entire 
widget width? Both (centring and underlining) would not look as much deviant on 
Mac as the current title look.

FWIW and beyond the topic at hand: Mac dialogs that adopt a tabbed design 
usually have the tabs (icons or text labels) in a horizontal top row so they 
already have something like a title at the top. If I were to add a title to 
such dialogs I'd experiment with putting it in a completely different position, 
at the lower right, above or under the OK/Cancel buttons. That's often where 
you look first when a dialog opens, and also the last region (if you favour the 
mouse over the keyboard).

>PS: I've no incentive for getting this fixed, just wanted to add my 2c and 
>connect people.

Me neither for myself, I'm happy with how my QtCurve theme looks but I'm also 
trying to improve the way KDE software looks using the native style.

R.

D6492: Also use m_currentRequest when checking for installed and updates

2017-07-04 Thread Aleix Pol Gonzalez 
apol closed this revision.
apol added a comment.


  Submitted 
https://commits.kde.org/knewstuff/8d94a0cd28dc4897535197a363193c914f59a9ad

REPOSITORY
  R304 KNewStuff

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

To: apol, #frameworks, leinir

D6492: Also use m_currentRequest when checking for installed and updates

2017-07-04 Thread Dan Leinir Turthra Jensen 
leinir accepted this revision.
leinir added a comment.
This revision is now accepted and ready to land.


  Different approach, but yeah, telling people what'll actually happen works :)

REPOSITORY
  R304 KNewStuff

BRANCH
  master

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

To: apol, #frameworks, leinir

D6492: Also use m_currentRequest when checking for installed and updates

2017-07-04 Thread Aleix Pol Gonzalez 
apol updated this revision to Diff 16178.
apol added a comment.


  Fix doc

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6492?vs=16176=16178

BRANCH
  master

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

AFFECTED FILES
  src/core/engine.h

To: apol, #frameworks, leinir

D6492: Also use m_currentRequest when checking for installed and updates

2017-07-04 Thread Aleix Pol Gonzalez 
apol updated this revision to Diff 16176.
apol added a comment.


  Just document stuff

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6492?vs=16173=16176

BRANCH
  master

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

AFFECTED FILES
  src/core/engine.h

To: apol, #frameworks, leinir

Re: KTitleWidget and the native Mac style

2017-07-04 Thread Kevin Funk 
On Tuesday, 4 July 2017 14:53:15 CEST René J. V. Bertin wrote:
> René J.V. Bertin wrote:
> > style. I think I figured out the how/where once but can't seem to find the
> > info anymore so I'd appreciate a pointer.
> 
> Found it.
> 
> > FWIW, drawing this kind of label in a frame and/or with a different
> > background colour isn't appropriate for the native Macintosh style.
> 
> And this is where we have a problem if you're adamant about being as much in
> line with native look-and-feel as possible, as I know some KDE devs are.
> 
> I haven't found a single example of a "native" Mac application that uses
> comparable page/dialog titles rendered with a bigger font and in a frame
> with a different background colour.
> 
> I myself don't really care one way or another, if anything I find it more
> important that the user has a choice at some level. Ideally this would go
> through the widget style (as Breeze allows and QtCurve probably too) but the
> Macintosh style is not configurable and I don't think Qt would consider
> adding the rather complex patch required to render KTitleWidgets
> appropriately.
> 
> That leaves us with the only option of modifying the KTitleWidget code.

With regards to the "Widget Style and Behaviour" label in the `kcmshell5 
style` dialog: I don't find that particularly attracting under Breeze as well.

It's too "bulky" for my taste. 

CC'ing Sebas, who've reworked the widget to look like this in

commit 510236aa9cea6bae48e548b42b5b721195bed121
Author: Sebastian Kügler 
Date:   Sat May 24 01:36:51 2014 +0200

Change titlewidget from bold to increased font size

This patch removes the bold weight from KTitleWidget and makes the text
a bit bigger, improving focus. This is more in line with common
expectations of a title, and it's more in line with Plasma 5's
typography.


Maybe we can find a solution that works well under both styles?

PS: I've no incentive for getting this fixed, just wanted to add my 2c and 
connect people.

Cheers,
Kevin

> I'll
> submit a patch for review that adds a runtime check of the widget style in
> use but if anyone has a better idea I wouldn't mind hearing it and avoiding
> unnecessary work.
> 
> R.


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

KDE CI: Frameworks kirigami kf5-qt5 XenialQt5.7 - Build # 7 - Still Unstable!

2017-07-04 Thread no-reply 
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/7/
 Project:
Frameworks kirigami kf5-qt5 XenialQt5.7
 Date of build:
Tue, 04 Jul 2017 13:39:33 +
 Build duration:
1 min 7 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 2 test(s)Failed: TestSuite.qmltests
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  Cobertura Coverage Report

build.log
Description: Binary data

KDE CI: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 - Build # 6 - Still Unstable!

2017-07-04 Thread no-reply 
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/6/
 Project:
Frameworks kirigami kf5-qt5 FreeBSDQt5.7
 Date of build:
Tue, 04 Jul 2017 13:39:33 +
 Build duration:
1 min 0 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 test(s)Failed: TestSuite.qmltests

build.log
Description: Binary data

D6492: Also use m_currentRequest when checking for installed and updates

2017-07-04 Thread Aleix Pol Gonzalez 
apol updated this revision to Diff 16173.
apol added a comment.


  Oops

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6492?vs=16172=16173

BRANCH
  master

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

AFFECTED FILES
  src/core/engine.cpp
  src/core/engine.h

To: apol, #frameworks, leinir

D6492: Also use m_currentRequest when checking for installed and updates

2017-07-04 Thread Aleix Pol Gonzalez 
apol updated this revision to Diff 16172.
apol added a comment.


  Improve documentation and consistency

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6492?vs=16150=16172

BRANCH
  master

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

AFFECTED FILES
  src/core/engine.cpp
  src/core/engine.h

To: apol, #frameworks, leinir

Re: KTitleWidget and the native Mac style

2017-07-04 Thread René J . V . Bertin 
René J.V. Bertin wrote:

> style. I think I figured out the how/where once but can't seem to find the
> info anymore so I'd appreciate a pointer.

Found it.

> FWIW, drawing this kind of label in a frame and/or with a different background
> colour isn't appropriate for the native Macintosh style.

And this is where we have a problem if you're adamant about being as much in 
line with native look-and-feel as possible, as I know some KDE devs are.

I haven't found a single example of a "native" Mac application that uses 
comparable page/dialog titles rendered with a bigger font and in a frame with a 
different background colour.

I myself don't really care one way or another, if anything I find it more 
important that the user has a choice at some level. Ideally this would go 
through the widget style (as Breeze allows and QtCurve probably too) but the 
Macintosh style is not configurable and I don't think Qt would consider adding 
the rather complex patch required to render KTitleWidgets appropriately.

That leaves us with the only option of modifying the KTitleWidget code. I'll 
submit a patch for review that adds a runtime check of the widget style in use 
but if anyone has a better idea I wouldn't mind hearing it and avoiding 
unnecessary work.

R.

D6492: Also use m_currentRequest when checking for installed and updates

2017-07-04 Thread Dan Leinir Turthra Jensen 
leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.


  derp, not accepted, my bad...

REPOSITORY
  R304 KNewStuff

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

To: apol, #frameworks, leinir

D6492: Also use m_currentRequest when checking for installed and updates

2017-07-04 Thread Dan Leinir Turthra Jensen 
leinir accepted this revision.
leinir added a comment.
This revision is now accepted and ready to land.


  That is indeed a very good point. I think we might possibly have an issue 
here, though, in that since these two searches are no longer stand-alone, if 
the filter is not explicitly reset before attempting to perform a search we 
will only search installed and updateable items. I don't think it is 
necessarily an enormous or insurmountable issue here, as far as i can tell the 
only code which currently uses these two functions is in KNS' own 
DownloadManagers (at least, lxr suggests as much), and if we ensure those two 
are fixed to reset the filter before launching a search, i think we're probably 
ok.
  
  PS: These two functions also lack documentation, which shall need fixing 
(though not necessarily as a part of this D)

REPOSITORY
  R304 KNewStuff

BRANCH
  master

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

To: apol, #frameworks, leinir

D5208: Allow loading i18n catalogs from arbitrary locations

2017-07-04 Thread Chusslove Illich 
ilic accepted this revision.
ilic added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> davidedmundson wrote in kcatalog.cpp:124
> I think I still need it.
> 
> QHash::value() isn't marked as being atomic so we still need to make sure 
> we're not reading at the same time as someone else is modifying the hash 
> table.

Hm, right... That makes me wonder where else there should be a lock and it's 
missing :) But that's for another story.

REPOSITORY
  R249 KI18n

BRANCH
  master

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

To: davidedmundson, #frameworks, ilic
Cc: ilic

D6495: Don't list KF5::WindowSystem in public libraries

2017-07-04 Thread David Edmundson 
This revision was automatically updated to reflect the committed changes.
Closed by commit R242:f8de13c67421: Don't list KF5::WindowSystem in public 
libraries (authored by davidedmundson).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6495?vs=16153=16162

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

AFFECTED FILES
  autotests/CMakeLists.txt
  src/plasmaquick/CMakeLists.txt

To: davidedmundson, #plasma
Cc: asturmlechner, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas

D5208: Allow loading i18n catalogs from arbitrary locations

2017-07-04 Thread David Edmundson 
davidedmundson marked an inline comment as done.
davidedmundson added inline comments.

INLINE COMMENTS

> ilic wrote in kcatalog.cpp:124
> Also the lock should be removed, now that only read access is used.

I think I still need it.

QHash::value() isn't marked as being atomic so we still need to make sure we're 
not reading at the same time as someone else is modifying the hash table.

REPOSITORY
  R249 KI18n

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

To: davidedmundson, #frameworks, ilic
Cc: ilic