Retiring

[Christoph Feck]

Hello developers,

my personal situation allows for much less time for KDE
related work, at least during the Corona times.

I would like to retire as soon as possible from these
responsibilities:
- bug triaging
- release service
- KIconTheme maintainer
- KPlotting maintainer
- KWidgetAddons maintainer

For bugzilla, I see many new and old contributors doing awesome
work with triaging, so departing here shouldn't be a big issue.
I hope someone can continue checking responses to NEEDINFO bugs
and REPORTED bugs that didn't get a reply within about two weeks.

Regarding the release service work, I have made sure the load
isn't all back to Albert. For future release work, Heiko Becker
volunteered to help, but maybe another hand would be awesome, too.

For the frameworks modules, I initially assumed each module
must have a separate maintainer, so I volunteered. Today I
see many modules still just community-maintained, and I hope
the community can take over maintainance. Not that I did any
relevant work here in the recent years...

I might eventually continue to prepare some patches, so please
keep all my accounts intact, but only de-list me as the maintainer.

Thank you,
Christoph

Re: idea

[Christoph Feck]

On 12/12/20 16:19, imindmap wrote:

Why Kde dont create a 
[DAW](https://en.wikipedia.org/wiki/Digital_audio_workstation) for linux ??
ardour not a 100% free (open source but not free) & lmms not a complete DAW
hey guys please create a DAW with mix ardour & lmms source code


Why do you expect you get different answers than you got on reddit?

Re: GSoC and Open Source Contribution

[Christoph Feck]

On 11/18/20 19:09, Vaibhav Gupta wrote:

I am Vaibhav Gupta, a second-year undergraduate student at IIIT Allahabad.
I am an intermediate in C++ and a beginner in OpenCV. I am keen to
contribute to this organization. Please guide me on how to begin and help
me with how to contact you for further queries.


Thank you for your interest in helping our community! We have collected
information how to contribute at https://community.kde.org/Get_Involved

If there is anything unclear, please ask more specific questions.

BG,
Christoph

Re: Hello everyone

[Christoph Feck]

Hi Вадим,

On 05/29/20 16:40, Березин Вадим wrote:

Hello everyone! I am new to the community and don't know how I can be useful in
it. I study C ++ and have programming experience in courses in Russian. I solved
some mathematical problems using C ++ 11. My first language is Russian, but I
can read English forums. At the moment I work in a telecommunications company in
Russia. I will be glad to receive your ideas!


Thank you for considering to contribute!

Please read https://community.kde.org/Get_Involved

D29050: WIP KRunner fix prepare/teardown signals

[Christoph Feck]

cfeck added a project: Plasma.
cfeck added a comment.


  If this is no longer Work-In-Progress, please remove "WIP" from the title.

REPOSITORY
  R308 KRunner

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

To: alex, meven, ngraham, broulik
Cc: cfeck, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Christoph Feck]

cfeck added a comment.


  Sorry if I don't understand the scope, but does this mean I am forced to 
install systemsettings to be able to use KCMs?

REPOSITORY
  R296 KDeclarative

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

To: ngraham, #plasma, mart
Cc: cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29502: kwidgetsaddons: Add a named colors support in KColorCombo.

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> kcolorcombo.cpp:89
> +int unused, v;
> +innercolor.getHsv(, , );
> +textColor = v > 128 ? Qt::black : Qt::white;

Please don't use "value" component to calculate the brightness of a color. 
#81 is much darker than #808080. To decide, use qGray(). The threshold also 
needs to be higher than 128; I use 170, but this mostly depends on correctness 
of monitor gamma settings.

See  
https://stackoverflow.com/questions/3942878/how-to-decide-font-color-in-white-or-black-depending-on-background-color

> kcolorcombo.cpp:277
> +list.reserve(d->colorList.size());
> +for (auto iColor : d->colorList) {
> +list.append({iColor.second});

Variables declared in 'for' are local, so just use 'color'.

> kcolorcombo.h:61
>  
> +/** Struct used in named colors list */
> +using ColorList = QList>;

The comment still says "struct". Maybe clarify that this list is actually used 
as a map.

(I guess since mapping would happen in both directions, using a QMap isn't 
useful?)

> kcolorcombo.h:91
> + * standard list.
> + * @param colors map os named colors. If empty, the selction list
> + *  reverts to the standard list.

typos: of; selection

> kcolorcombo.h:97
> +/**
> + * Inserts a named color at a specific position in the standard list
> + * @param index position in the list

Sentence misses a full stop.

> kcolorcombo.h:110
> +/**
> + * Return the list of named colors available for selecion.
> + * If no named color, return namedColor is empty.

typo: selection

> kcolorcombo.h:111
> + * Return the list of named colors available for selecion.
> + * If no named color, return namedColor is empty.
> + * @return list of named colors

This sentence is confusing. I guess you wanted to write "if there are no named 
colors, the returned list is empty." (to clarify that it won't return the 
standard list).

REPOSITORY
  R236 KWidgetsAddons

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

To: araujoluis, tcanabrava, patrickelectric, hindenburg, ngraham
Cc: broulik, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29502: kwidgetsaddons: Add a named colors support in KColorCombo.

[Christoph Feck]

cfeck added a comment.


  Does the delegate ensure the text is rendered in a color visible over the 
colored background?

REPOSITORY
  R236 KWidgetsAddons

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

To: araujoluis, tcanabrava, patrickelectric, hindenburg, ngraham
Cc: cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D25814: [KColorScheme] Add SeparatorColor

[Christoph Feck]

cfeck added a comment.


  There is QPalette::Button, but I don't see any hover/focus colors in QPalette.
  
  If we want more roles, we seriously need to contribute them upstream. Qt 6 is 
a chance to avoid diverging more.

REPOSITORY
  R265 KConfigWidgets

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

To: ndavis, #frameworks, #vdg
Cc: ahiemstra, broulik, manueljlin, alexde, ngraham, davidedmundson, filipf, 
cfeck, hpereiradacosta, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns

Re: Adding the Abstract Games Suite to the KDE Applications

[Christoph Feck]

On 05/05/20 23:37, Albert Astals Cid wrote:

About remaining anonymous, that's a more picky subject and we are not sure it's 
a yes or it's a no yet.

Not sure what to tell you here to be honest.


We already have contributions where the authors explicitly stated that 
the committer name is not the real name, so I don't think it's a problem.


I haven't checked the Abstract Games yet (e.g. whether they compete 
against or complement the existing KDE Games, or how they are coded), so 
I cannot decide if we should incubate them to playground first, or if 
they can go directly to release service (after code review).


--
Christoph Feck

Re: Is kdeinit still actual?

[Christoph Feck]

On 04/27/20 12:53, Alexander Volkov wrote:

So, does it still makes sense to use kdeinit?


https://phabricator.kde.org/T12140

D25267: Improve XCF support

[Christoph Feck]

cfeck added a comment.


  Thanks for your work, Martin! Could you please add a note (or resolve) bug 
360821?

REPOSITORY
  R287 KImageFormats

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

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

D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on slow fs

[Christoph Feck]

cfeck added a comment.


  Are more changes planned?

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, ngraham, broulik, dfaure
Cc: cfeck, anthonyfieroni, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D27455: FileWidgets: Ignore Return events from KDirOperator

[Christoph Feck]

cfeck edited the summary of this revision.

REPOSITORY
  R241 KIO

BRANCH
  master

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

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

Re: kpat card decks

[Christoph Feck]

On 03/07/20 21:42, 0...@caiway.net wrote:

I tried to reverse engineer kpat to find the image of the backside of
the cards. I want to customize it.


See https://cgit.kde.org/libkdegames.git/tree/carddecks

D27802: smb: fix ipv6 support

[Christoph Feck]

cfeck added a comment.


  Well, cannot delay the release any longer.

REPOSITORY
  R320 KIO Extras

BRANCH
  smb-smburl-static-autotest-ipv6

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

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

Re: start contribute to KDE

[Christoph Feck]

On 03/04/20 23:01, Konrad Czapla wrote:

Yes I read this section and I set up kdesrc-build script and I built the
dolphin file manager. I want to know How I can take my first task ? and
should I focus on a particular project? if yes, which would be beginner
friendly?


It is best to work on something that bothers you. A bug, or a missing
feature.  If you really have no idea what you could improve, there is a
list of open bugs and wishlist items at bugzilla.

Dolphin isn't exactly beginner-friendly, though, because it has complex
custom view classes, and actual operations happen in KIO.  I suggest to
also look at kdeutils, kdegames, and kdeedu projects for starting, but
as I said, your motivation decides where and what to work on.

--
Christoph Feck

D27802: smb: fix ipv6 support

[Christoph Feck]

cfeck added a comment.


  Should this be in 19.12 branch? I am doing a respin of kio-extras anyway.

REPOSITORY
  R320 KIO Extras

BRANCH
  smb-smburl-static-autotest-ipv6

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

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

kreversi 19.12 fails to build at IconSize call

[Christoph Feck]

Hi,

kreversi release/19.12 branch fails to build with this error on CI:


pageItem->setIcon(QIcon::fromTheme(icon).pixmap(IconSize(KIconLoader::Toolbar)));
error: IconSize was not declared in this scope

See 
https://build.kde.org/job/Applications/view/Everything%20-%20stable-kf5-qt5/job/kreversi/


Additionally, kwordquiz fails to build on Windows CI,
see 
https://build.kde.org/job/Applications/view/Everything%20-%20stable-kf5-qt5/job/kwordquiz/


I would be glad if someone could investigate these issues.

Thanks,
Christoph

--
Christoph Feck
KDE Release Team

Re: bug 385814

[Christoph Feck]

On 02/27/20 14:24, Stefan Champailler wrote:

import org.kde.plasma.private.kicker 0.1 as Kicker


Kicker is in plasma-workspace, see 
https://cgit.kde.org/plasma-workspace.git/tree/applets/kicker

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

[Christoph Feck]

cfeck accepted this revision.
cfeck added a comment.


  Merci :)

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  no-css (branched from master)

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

To: davidre, #frameworks, ngraham, cfeck
Cc: cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

Re: Review Kid3

[Christoph Feck]

On 02/13/20 22:14, Albert Astals Cid wrote:

You should change the bug reporting to use bugzilla not your email address in 
KAboutData


Right. Urs, could you please request creation of a bugzilla product?

Thanks,
Christoph

Re: Review Kid3

[Christoph Feck]

On 02/13/20 22:14, Albert Astals Cid wrote:

You should change the bug reporting to use bugzilla not your email address in 
KAboutData


Right. Urs, could you please request creation of a bugzilla product?

Thanks,
Christoph

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> kmessagewidget.cpp:155
>  };
> -
> +// Add bordersize to the margin so it starts from the inner border and 
> doesn't look to cramped
> +q->layout()->setContentsMargins(q->layout()->contentsMargins() + 
> borderSize);

too

> kmessagewidget.cpp:193
> +textLabel->setPalette(palette);
> +// update the Icon in case its recolorable
> +q->setIcon(icon);

typo: it is

> kmessagewidget.cpp:325
> +constexpr float radius = 4 * 0.6;
> +const QRect innerRect = rect().marginsRemoved(QMargins() + borderSize/2);
> +const QColor color = palette().color(QPalette::Window);

missing spaces around `/`

> kmessagewidget.cpp:397
>  
> -if (isVisible() && (d->timeLine->state() == QTimeLine::NotRunning) && 
> (height() == d->bestContentHeight()) && (d->content->pos().y() == 0)) {
> +if (isVisible() && (d->timeLine->state() == QTimeLine::NotRunning) && 
> (height() == d->bestContentHeight())){
>  emit showAnimationFinished();

missing space before `{`

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  no-css (branched from master)

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

To: davidre, #frameworks, ngraham
Cc: cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

Re: Review Kid3

[Christoph Feck]

On 02/13/20 07:11, Urs Fleisch wrote:

As you may know, Kid3, an audio tagger, has moved from SourceForge.net to
kde.org. Jonathan Riddell is attending the incubation process as a sponsor.
To complete the incubation, I would kindly ask you to execute a KDE review
on the project.

Website: https://kid3.kde.org/
Git, Issues: https://invent.kde.org/kde/kid3/


Is there a way to subscribe to bugs reported there?


Incubator: https://community.kde.org/Incubator/Projects/kid3
Translation state: https://l10n.kde.org/stats/gui/trunk-kf5/po/kid3_qt.po/
Documentation state: https://l10n.kde.org/stats/doc/trunk-kf5/po/kid3.po/

Kid3 exists since 2003 and I would say that it is quite mature. As it is
also available as a Qt-only version (without KDE dependencies) and for
macOS, Windows and Android, the build system differs a bit from the typical
KDE application.


--
Christoph Feck
KDE Bug Triaging Team

Re: Review Kid3

[Christoph Feck]

On 02/13/20 07:11, Urs Fleisch wrote:

As you may know, Kid3, an audio tagger, has moved from SourceForge.net to
kde.org. Jonathan Riddell is attending the incubation process as a sponsor.
To complete the incubation, I would kindly ask you to execute a KDE review
on the project.

Website: https://kid3.kde.org/
Git, Issues: https://invent.kde.org/kde/kid3/


Is there a way to subscribe to bugs reported there?


Incubator: https://community.kde.org/Incubator/Projects/kid3
Translation state: https://l10n.kde.org/stats/gui/trunk-kf5/po/kid3_qt.po/
Documentation state: https://l10n.kde.org/stats/doc/trunk-kf5/po/kid3.po/

Kid3 exists since 2003 and I would say that it is quite mature. As it is
also available as a Qt-only version (without KDE dependencies) and for
macOS, Windows and Android, the build system differs a bit from the typical
KDE application.


--
Christoph Feck
KDE Bug Triaging Team

Re: 2 kirigami fixes for a point release

[Christoph Feck]

On 02/13/20 08:42, Ben Cooksley wrote:

Part of the issue here is that Plasma has been known to add API to
Frameworks and then immediately, without any delay, start using it
(pretty much always breaking CI in the process)

This means that other changes are likely being pushed into Frameworks
by Plasma with very little delay as well.

Consequently this means stuff is landing in Framework repositories up
to the very moment it is released - a release that the next version of
Plasma (LTS) then depends on.

A better way of approaching this would be to freeze the Frameworks
version you are going to require API wise at an earlier point in the
Plasma development cycle. This would allow for a full Frameworks
release cycle to pass where bugs encountered during the lead up to the
Plasma release can be fixed.


You can find bugs in new code best if you are actually using it. I doubt 
that delaying using new API would help much.

D27195: Change "Redisplay" to "Refresh"

[Christoph Feck]

cfeck added a comment.


  Refresh is fine, Reload could be irritating for applications that don't load 
anything, but still know the user could be faced with a view state that needs 
redrawing, e.g. when cleaning corruption caused by non-integer scale factors.

REPOSITORY
  R265 KConfigWidgets

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

To: ngraham, #vdg, The-Feren-OS-Dev, ndavis
Cc: cfeck, kde-frameworks-devel, The-Feren-OS-Dev, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27087: Add HEIF thumbnailer

[Christoph Feck]

cfeck added a comment.


  Where can we see the status of the HEIF Qt loader? If it correctly implements 
scaled image loading, then a separate thumbnailer indeed makes not much sense. 
If, however, it always loads the full image, then scales it down, we got a case 
for a separate thumbnailer such as this one.

REPOSITORY
  R320 KIO Extras

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

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

Re: Recommendation: drop ProvidersUrl entry to rely on default value

[Christoph Feck]

On 01/30/20 13:53, Friedrich W. H. Kossebau wrote:

as found out by discussion on irc, a good solution for everyone relying on the
default GHNS storage as provided by KDE is to just not hard-code any value for
ProvidersUrl, but leave it out and let KNewStuff default to what is built into
the KNewStuff library as current value.


Does it work with all KNewStuff 5.x versions? Otherwise, the required
KF5 version would need to be bumped where this change was made.

--
Christoph Feck

Re: Recommendation: drop ProvidersUrl entry to rely on default value

[Christoph Feck]

On 01/30/20 13:53, Friedrich W. H. Kossebau wrote:

as found out by discussion on irc, a good solution for everyone relying on the
default GHNS storage as provided by KDE is to just not hard-code any value for
ProvidersUrl, but leave it out and let KNewStuff default to what is built into
the KNewStuff library as current value.


Does it work with all KNewStuff 5.x versions? Otherwise, the required
KF5 version would need to be bumped where this change was made.

--
Christoph Feck

Re: Recommendation: drop ProvidersUrl entry to rely on default value

[Christoph Feck]

On 01/30/20 13:53, Friedrich W. H. Kossebau wrote:

as found out by discussion on irc, a good solution for everyone relying on the
default GHNS storage as provided by KDE is to just not hard-code any value for
ProvidersUrl, but leave it out and let KNewStuff default to what is built into
the KNewStuff library as current value.


Does it work with all KNewStuff 5.x versions? Otherwise, the required
KF5 version would need to be bumped where this change was made.

--
Christoph Feck

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> kmessagewidget.cpp:39
> +
> +constexpr int borderSize = 2;
> +

Does it need to be `static` to avoid an external symbol?

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  no-css (branched from master)

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

To: davidre, #frameworks, ngraham
Cc: cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
bruns

D25814: [KColorScheme] Add SeparatorColor

[Christoph Feck]

cfeck added a comment.


  > Why don't focus and hover colors belong?
  
  Because an application cannot know if and how a style does indicate focus or 
hovering.

REPOSITORY
  R265 KConfigWidgets

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

To: ndavis, #frameworks, #vdg
Cc: broulik, manueljlin, alexde, ngraham, davidedmundson, filipf, cfeck, 
hpereiradacosta, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D26648: Improved quality of JPEG thumbnails

[Christoph Feck]

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


  Thanks for the detailed investigation, Stefan!

REPOSITORY
  R320 KIO Extras

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

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

D26648: Improved quality of JPEG thumbnails

[Christoph Feck]

cfeck added a comment.


  @meven, this isn't about save quality, but load quality.

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

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

D26648: Improved quality of JPEG thumbnails

[Christoph Feck]

cfeck added a comment.


  Did you time a performance difference? I think the idea was to get the 
downscaled result as fast as possible, by using integer IDCT instead of float 
IDCT, and by using only the DC coefficient instead of performing the IDCT at 
all for scale factors <= 1/8.

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

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

D26448: Add KRecentFilesMenu to replace KRecentFileAction

[Christoph Feck]

cfeck added a comment.


  Nice :)

INLINE COMMENTS

> krecentfilesmenu.cpp:34
> +
> +KRecentFilesMenu::KRecentFilesMenu(const QString& title, QWidget* parent)
> +: QMenu(title, parent)

Here you switch from `Type *var` to `Type* var`

> krecentfilesmenu.cpp:41
> +
> +KRecentFilesMenu::KRecentFilesMenu(QWidget* parent)
> +: QMenu(tr("Recent Files"), parent)

same

> krecentfilesmenu.cpp:81
> +
> +void KRecentFilesMenu::addUrl(const QUrl url, const QString& name)
> +{

same

> krecentfilesmenu.cpp:103
> +
> +void KRecentFilesMenu::removeUrl(const QUrl& url)
> +{

same

> krecentfilesmenu.cpp:215
> +{
> +d->m_maximumItems = maximumItems;
> +}

Should we truncate the current list if the new max items is smaller?

> krecentfilesmenu.h:67
> + */
> +void addUrl(const QUrl url, const QString  = QString());
> +

Missing reference

> krecentfilesmenu.h:79
> + *
> + * When the manimum url count is reached and a new URL is added the
> + * oldest will be replaced.

manimum url → maximum URL

> krecentfilesmenu.h:104
> +void rebuildMenu();
> +std::list>::const_iterator findUrl(const QUrl url);
> +

Weren't there ABI issues with std::list?

Also, missing reference on url.

REPOSITORY
  R236 KWidgetsAddons

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

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

Re: Keysmith in kdereview

[Christoph Feck]

On 12/28/19 19:17, Jesus Varela wrote:

I am new and have tons of questions about all these emails I see.

From what I understand there is software being added to the core KDE
framework. Will this make the framework less desired due to it being
bloated with tons of features? Or is it desired to have more features even
if it makes the framework heavier?

I hear about a KDE/QT powered phone in the works but then I wonder how a
huge framework like KDE will work on a mobile device. Is there a way to
break the framework into parts that only deploy the necessary pieces?

Sorry for the noob questions. Please refer me to any docs I should read.


Our framework is already splitted into dozens of individual libraries, 
see https://techbase.kde.org/KDE_Frameworks


--
Christoph Feck

D25814: [KColorScheme] Add SeparatorColor

[Christoph Feck]

cfeck added a comment.


  I also agree with the concerns rised by Hugo. An application will use various 
frame primitives, and the style decides how to draw them. It doesn't belong in 
the API, though (but neither do Focus and Hover colors).

REPOSITORY
  R265 KConfigWidgets

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

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

D25697: Port away from KIconLoader::SizeSmallMedium

[Christoph Feck]

cfeck added a comment.


  Traditionally, 22 was used for toolbar icons. You could replace 22 with 
QStyle::PM_ToolBarIconSize, but it really depends on where you want the icons 
to appear. Generally, QStyle::PM_SmallIconSize is for icons that are placed 
next to a single line of text, unless there is a more specifc PixelMetric enum, 
e.g. QStyle::PM_ButtonIconSize.

REPOSITORY
  R241 KIO

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

To: nicolasfella, #frameworks
Cc: cfeck, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25561: Remove unused signal we can use directly "signal(const QUrl&)

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> kurllabel.h:281
>   * Emitted when the mouse has passed over the label.
> + * @deprecated Since 5.65, use leftUrl(const QString );
>   */

Wrong comment?

REPOSITORY
  R236 KWidgetsAddons

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

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

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

[Christoph Feck]

cfeck resigned from this revision.
cfeck added a comment.
This revision is now accepted and ready to land.


  Yes, I would prefer the check condition. Feel free to commandeer; it looks 
like the original author didn't find time to further investigate.

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

To: brauch, dfaure
Cc: rkflx, dfaure, dhaumann, aacid, #frameworks, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D25444: Fix assistant dialog margins

[Christoph Feck]

cfeck added a comment.


  Since KAssistantDialog is a KPageDialog, what happens to other users of that 
class? I had hoped the fix is in KPageDialog, and it would only omit the 
margins when the caller requested it (e.g. KCMs etc.).

INLINE COMMENTS

> kassistantdialog.cpp:103
> +q->pageWidget()->layout()->setContentsMargins(
> +style->pixelMetric(QStyle::PM_LayoutLeftMargin),
> +style->pixelMetric(QStyle::PM_LayoutTopMargin),

Please also pass styleOptions and widget. Some styles might compute margins 
from font sizes, and without any context, they can only assume defaults.

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, #frameworks, cfeck, mart
Cc: kossebau, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24993: Add integrated inline spelling menu to KTextEdit

[Christoph Feck]

cfeck added a comment.


  Oh, I didn't read the code, only replied to the comment. If this is new code, 
then changing the visibility is no ABI problem.

REPOSITORY
  R310 KTextWidgets

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

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

D24993: Add integrated inline spelling menu to KTextEdit

[Christoph Feck]

cfeck added a comment.


  I am not sure if changing the visibility of members is ABI compatible. This 
might need to wait for KF6.

REPOSITORY
  R310 KTextWidgets

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

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

Re: New Framework Review: KDAV

[Christoph Feck]

Hi Volker,

On 11/09/19 12:33, Volker Krause wrote:

during Akademy there was a request to promote KDAV from KDE PIM to Frameworks
for use by Plasma Mobile. KDAV is a framework that implements the CalDav/
CardDav/GroupDav protocol on top of KIO's WebDav support. It would be
classified as a functional tier 3 framework.


Could you clarify if the review is about 
https://api.kde.org/kdepim/kdav/html/index.html or 
https://api.kde.org/kdepim/kdav2/html/index.html ?


Christoph Feck

D24672: GIT_SILENT Run uncrustify-kf5 on the whole tree

[Christoph Feck]

cfeck added a reviewer: mkoller.

REPOSITORY
  R374 KolourPaint

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

To: ahmadsamir, kde-frameworks-devel, mkoller
Cc: ognarb, kde-frameworks-devel

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

[Christoph Feck]

cfeck added a comment.


  Does it happen with every code that uses QPropertyAnimation, or just with 
this KBusyIndicator?

REPOSITORY
  R236 KWidgetsAddons

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

To: sitter, cfeck, apol
Cc: rjvbb, ngraham, kossebau, broulik, kde-frameworks-devel, apol, LeGast00n, 
GB_2, michaelh, bruns

D24367: Some sanity verification

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> pcx.cpp:312
> +qWarning() << "Failed to get scanline for" << y << "might be out 
> of bounds";
> +}
>  for (int x = 0; x < header.width(); ++x) {

If `p` indeed could be zero, the next statements need to be in an `else` block 
(or a add a `return`).

REPOSITORY
  R287 KImageFormats

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

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

D24252: Make OK button configurable in KMessageBox::sorry/detailedSorry

[Christoph Feck]

cfeck added a comment.


  I think it needs to be named 'buttonOk' instead of 'buttonOK'.

REPOSITORY
  R236 KWidgetsAddons

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

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

D24254: [KCollapsibleGroupBox] Fix QTimeLine::start warning at runtime

[Christoph Feck]

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

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  2019_09_fix_qtimeline_warning (branched from master)

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

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

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

[Christoph Feck]

cfeck added a comment.


  I would suggest to commit it either sooner, or after 5.63 is tagged. If you 
commit on 3rd, there are only two days to test and decide how to improve or 
revert before tars are made.

REPOSITORY
  R304 KNewStuff

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

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

Re: lzip vs. xz

[Christoph Feck]

On 09/25/19 20:12, Tobias Leupold wrote:

Hi devs!

We provide all source tarballs xz-compressed. Now, apparently, xz is quite a
badly designed format (cf. http://lzip.nongnu.org/xz_inadequate.html ), and
lzip provides the same compression, but an apparently distinctly more
reasonable format.

I must admit that I personally neither heard about lzip, nor about xz being no
good choice until today, but the current tar even supports it through the --
lzip switch.

It seems to me that xz has more attention and popularity than it deserves, and
lzip has less. Did you ever think about promoting lzip by using it for
tarballs?

Just in case the criticism about xz is justified.


Everyone likes to promote their own compression tools. There are 
hundreds, see e.g. http://mattmahoney.net/dc/text.html


We should support the technology that is most commonly installed on a 
user's system and on a server's toolchain.


It needed a long time until xz was established. Maybe lzip will 
establish, or maybe zstd will. Or razor[1] will be open sourced and 
spreads like a fire. Or someone makes paq8 1000 times faster. Only time 
will tell, but until then, xz is what we have.


Christoph Feck
(who uses bcm for his compression needs :)

[1] https://encode.su/threads/2829-RAZOR-strong-LZ-based-archiver

D24113: xcf: Properly read image resolution

[Christoph Feck]

cfeck added a comment.


  That's bug 362484, please close. Merci!

REPOSITORY
  R287 KImageFormats

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

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

D23811: [KImageFormats] Port HDR (Radiance RGBE) image loader to Qt5

[Christoph Feck]

cfeck added a comment.


  I somehow knew autotest would fail on FreeBSD ... I cannot investigate why it 
fails, maybe too old MIME database?

REPOSITORY
  R287 KImageFormats

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

To: cfeck, #frameworks, aacid
Cc: aacid, kde-frameworks-devel, LeGast00n, tommo, GB_2, clintmoyer, tdarboux, 
huoni, michaelh, muhlenpfordt, rkflx, ngraham, bruns

Re: KDE Frameworks 5.62.0 released

[Christoph Feck]

On 09/14/19 13:35, David Faure wrote:

14th September 2019. KDE today announces the release of KDE Frameworks 5.62.0.

http://kde.org/announcements/kde-frameworks-5.62.0.php


This web page says the release number is 5.1.0.

Re: KDE Frameworks 5.62.0 released

[Christoph Feck]

On 09/14/19 13:35, David Faure wrote:

14th September 2019. KDE today announces the release of KDE Frameworks 5.62.0.

http://kde.org/announcements/kde-frameworks-5.62.0.php


This web page says the release number is 5.1.0.

D23811: [KImageFormats] Port HDR (Radiance RGBE) image loader to Qt5

[Christoph Feck]

This revision was automatically updated to reflect the committed changes.
Closed by commit R287:68bb1a0ee7f9: Port HDR (Radiance RGBE) image loader to 
Qt5 (authored by cfeck).

REPOSITORY
  R287 KImageFormats

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23811?vs=65784=66041

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/read/hdr/rgb.hdr
  autotests/read/hdr/rgb.png
  src/imageformats/CMakeLists.txt
  src/imageformats/hdr.cpp
  src/imageformats/hdr.json
  src/imageformats/hdr_p.h

To: cfeck, #frameworks, aacid
Cc: aacid, kde-frameworks-devel, LeGast00n, tommo, GB_2, clintmoyer, tdarboux, 
huoni, michaelh, muhlenpfordt, rkflx, ngraham, bruns

D23904: Add QIcon setters for the password dialogs

[Christoph Feck]

cfeck added a comment.


  A getter would be nice. I am also fine with deprecating the pixmap calls. You 
can create an icon from a pixmap using QIcon.
  
  (Why is there no setIcon() in QLabel?)

REPOSITORY
  R236 KWidgetsAddons

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

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

D23811: [KImageFormats] Port HDR (Radiance RGBE) image loader to Qt5

[Christoph Feck]

cfeck updated this revision to Diff 65784.
cfeck added a comment.


  Add hdr to autotests

REPOSITORY
  R287 KImageFormats

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23811?vs=65713=65784

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/read/hdr/rgb.hdr
  autotests/read/hdr/rgb.png
  src/imageformats/CMakeLists.txt
  src/imageformats/hdr.cpp
  src/imageformats/hdr.json
  src/imageformats/hdr_p.h

To: cfeck, #frameworks
Cc: aacid, kde-frameworks-devel, LeGast00n, tommo, GB_2, clintmoyer, tdarboux, 
huoni, michaelh, muhlenpfordt, rkflx, ngraham, bruns

D23811: [KImageFormats] Port HDR (Radiance RGBE) image loader to Qt5

[Christoph Feck]

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

TEST PLAN
  Tested with HDR images from hdrihaven.com
  
  - Loading in KolourPaint works
  - Thumbnails in Dolphin work

REPOSITORY
  R287 KImageFormats

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

AFFECTED FILES
  src/imageformats/CMakeLists.txt
  src/imageformats/hdr.cpp
  src/imageformats/hdr.json
  src/imageformats/hdr_p.h

To: cfeck, #frameworks
Cc: kde-frameworks-devel, LeGast00n, tommo, GB_2, clintmoyer, tdarboux, huoni, 
michaelh, muhlenpfordt, rkflx, ngraham, bruns

D23797: [KWidgetAddons] port to non-deprecated Qt API

[Christoph Feck]

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


  Merci!

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  master

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

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

D23797: [KWidgetAddons] port to non-deprecated Qt API

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> kpopupframe.cpp:24
>  
> -#include 
> +#include 
>  #include 

Please keep this sorted.

REPOSITORY
  R236 KWidgetsAddons

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

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

D23597: Bulk port away from foreach

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> kacceleratormanagertest.cpp:35
> +const auto menuActions = menu.actions();
> +for (const QAction* action : menuActions) {
>  if (action->isSeparator()) {

Please use KF5 coding style: `Type *var` instead of `Type* var` (also for `&` 
references),

REPOSITORY
  R236 KWidgetsAddons

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

To: kossebau, #frameworks, cfeck
Cc: dhaumann, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D22884: [RFC] Don't show title on page by default

[Christoph Feck]

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


  The header title is usually more descriptive (longer) than the icon name, so 
no.

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, #vdg, mart, cfeck
Cc: cfeck, mmustac, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, 
ngraham, bruns

D22525: kioclient: Don't convert `:x:y` to `?line=x=y` for URLs starting with remote schemes.

[Christoph Feck]

cfeck added a comment.


  Oh, if the latter syntax also works, then you are right.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd
Cc: cfeck, plasma-devel, #frameworks, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D13867: [KMessageWidget] Pass widget to standardIcon()

[Christoph Feck]

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

REPOSITORY
  R236 KWidgetsAddons

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

To: broulik, #frameworks, cfeck, ngraham
Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns

D22525: kioclient: Don't convert `:x:y` to `?line=x=y` for URLs starting with remote schemes.

[Christoph Feck]

cfeck added a comment.


  Could we only apply the `:xx` check on the filename part, not on the server 
part? Someone might expect that `http://path.to/file.txt:99` also works for 
remote files.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd
Cc: cfeck, plasma-devel, #frameworks, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

[Christoph Feck]

cfeck accepted this revision.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  master

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

To: sitter, cfeck, apol
Cc: ngraham, kossebau, broulik, kde-frameworks-devel, apol, LeGast00n, 
sbergeron, michaelh, bruns

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> kbusyindicatorwidget.h:52
> + *
> + * @since 5.60.0
> + */

5.61.0

> kbusyindicatorwidgettest.cpp:43
> +
> +auto toggle = new QPushButton(QStringLiteral("Toggle Visibile"), 
> );
> +

typo

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  master

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

To: sitter, cfeck, apol
Cc: ngraham, kossebau, broulik, kde-frameworks-devel, apol, LeGast00n, 
sbergeron, michaelh, bruns

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

[Christoph Feck]

cfeck resigned from this revision.
cfeck added a comment.


  You wrote "ported from Device Notifier". I haven't check in detail yet, but 
do other copyright holders need to be mentioned?

REPOSITORY
  R244 KCoreAddons

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

To: hallas, davidedmundson, broulik, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> sitter wrote in kbusyindicatorwidget.h:48
> Mimics the QML API, haven't given it any thought TBH. You can have the 
> spinner visible but paused, I am not sure why exactly you'd want a paused 
> spinner but that's what the property does anyway.
> 
> i.e.
> 
> - you can have a visible but !running spinner which would simply be the 
> spinner at whatever the last rotation update was.
> - a visibile and running spinner = animation
> - a running but !visible spinner = effectively noop
> 
> That said, **I** have no use case for it so we can remove it for the time 
> being if you'd prefer.

Please remove it unless you find a justification. I don't like its name either 
(the widget isn't running).

> kbusyindicatorwidget.h:72
> +class Private;
> +Private *d;
> +};

`*const d`

REPOSITORY
  R236 KWidgetsAddons

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

To: sitter, cfeck
Cc: broulik, kde-frameworks-devel, apol, LeGast00n, michaelh, ngraham, bruns

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> kbusyindicatorwidget.h:48
> + */
> +Q_PROPERTY(bool running READ running WRITE setRunning NOTIFY 
> runningChanged)
> +public:

Why is this property needed? If the (parent) widget is no longer busy, this 
spinner needs to be hidden anyway.

REPOSITORY
  R236 KWidgetsAddons

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

To: sitter, cfeck
Cc: broulik, kde-frameworks-devel, apol, LeGast00n, michaelh, ngraham, bruns

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

[Christoph Feck]

cfeck added a comment.


  Ideally you can create it with any size as an overlay to an existing widgets 
(also to block input there), but the spinner itself is only rendered at a 
smaller centered position.

REPOSITORY
  R236 KWidgetsAddons

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

To: sitter, cfeck
Cc: broulik, kde-frameworks-devel, apol, LeGast00n, michaelh, ngraham, bruns

D22083: introduce concept of header and footer for kpageview

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> kpageview.cpp:458
> +
> +if (d->pageHeader == header) {
> +return;

This check is duplicated.

> kpageview.cpp:476
> +}
> +
> +}

Please remove this empty line.

> kpageview.h:163
> +/**
> + * Set a widget as the header for this Page view
> + * It will replace the standard page title

Does this transfer ownership of the widget? If yes, is ownership transferred 
back for any previously set widget? Or is it even automatically deleted? It 
needs to be clarified in the docs.

> kpageview_p.h:81
>  
> +QPointer  pageHeader;
> +QPointer  pageFooter;

No space before `<`

REPOSITORY
  R236 KWidgetsAddons

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

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

D21894: set the focusPolicy of kpasswordlineedit to the policy of its proxy

[Christoph Feck]

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

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  master

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

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

Re: How is the Font DPI Scale stored?

[Christoph Feck]

On 06/12/19 08:01, Elias Mårtenson wrote:

I am a developer for the McCLIM <https://github.com/McCLIM/McCLIM> graphics
toolkit, and I would like it to be able to integrate better with KDE.

Currently, McCLIm has a hardcoded font scale which is problematic when on a
high DPI screen. It's possible for a user to customise it but recomputing
the default fonts, but that's not ideal.

What I'd like to do is to read the KDE "Force Fonts DPI" setting, and use
it to automatically compute the new font sizes. However, it's unclear to me
how this value is propagated to applications. There must be a standard for
this, since a lot of applications is able to pick it up, but I haven't been
able to figure it out.

What is the proper way to read this value?


https://wiki.archlinux.org/index.php/Font_configuration says:

$ xrdb -query | grep dpi
Xft.dpi:144

You can read XResources via libxcb calls, code in Qt is here:
https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/xcb/qxcbscreen.cpp#n357

Note that this value is deprecated, because it isn't per-screen.

--
Christoph Feck

D18161: [kioslave/file] Add a codec for legacy filenames

[Christoph Feck]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:6738a8b2f71c: [kioslave/file] Add a codec for legacy 
filenames (authored by cfeck).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D18161?vs=49684=58399#toc

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18161?vs=49684=58399

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

AFFECTED FILES
  src/ioslaves/file/CMakeLists.txt
  src/ioslaves/file/file.cpp
  src/ioslaves/file/legacycodec.cpp
  src/ioslaves/file/legacycodec.h

To: cfeck, #frameworks, #dolphin, dfaure
Cc: frispete, nathanshearer, nerdopolist, ngraham, kde-frameworks-devel, 
michaelh, bruns

D18161: [kioslave/file] Add a codec for legacy filenames

[Christoph Feck]

cfeck retitled this revision from "[WIP/RFC] [kioslave/file] Add a codec for 
legacy filenames" to "[kioslave/file] Add a codec for legacy filenames".
cfeck edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: cfeck, #frameworks, #dolphin, dfaure
Cc: frispete, nathanshearer, nerdopolist, ngraham, kde-frameworks-devel, 
michaelh, bruns

D21028: add multiple gestures and a handler class to KWidgetsAddons

[Christoph Feck]

cfeck added a comment.


  Did you try to submit the additional gesture types to Qt? We should only add 
them here if they rejected the idea to add new types in Qt.

REPOSITORY
  R236 KWidgetsAddons

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

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

D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames

[Christoph Feck]

cfeck added a comment.


  The encode/decode functions were already reviewed for kdelibs4. It's the 
remaining code that needs review.

REPOSITORY
  R241 KIO

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

To: cfeck, #frameworks, #dolphin, dfaure
Cc: frispete, nathanshearer, nerdopolist, ngraham, kde-frameworks-devel, 
michaelh, bruns

D20181: Add Binary Data units (bits, kilobytes, kibibytes ... yottabytes)

[Christoph Feck]

cfeck added a comment.


  KFormat knows about the prefixes, but doesn't know their name. I would say 
adding translations for "megabytes" etc. to KCoreAddons is out of scope.

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

To: JJRcop, broulik, #plasma, ngraham
Cc: cfeck, apol, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D20766: Use appropriate background color for text previews

[Christoph Feck]

cfeck requested changes to this revision.
cfeck added a comment.


  Before we can support both dark and light themes we need to investigate the 
KSyntaxHighlighting issue found by Friedrich.
  
  Later we could request a light or dark theme depending on the lightness of 
the QPalette entry, but I would suggest to use View instead of Window, because 
window backgrounds are gray with some themes, while view backgrounds usually 
have more contrast (either darker or brighter than gray).

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

To: eshalygin, kossebau, cfeck
Cc: vkrause, cfeck, meven, broulik, kde-frameworks-devel, kfm-devel, alexde, 
feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
bruns, emmanuelp, mikesomov

Re: May 10: Removing l10n support for kde4 playground-* repos

[Christoph Feck]

On 04/21/19 12:47, Albert Astals Cid wrote:

We have a huge list of playground kde4 repos that are being processed for l10n.

This takes some resources that make no sense since you should really not be 
developing for KDE4 anymore, and if you really really really need it, it 
shouldn't be in playground.

So on May 10 i'll make sure scripty doesn't waste CPU on them.

Cheers,
  Albert

P.S: Anyone interested in the list https://ghostbin.com/paste/823jo


https://cgit.kde.org/smaragd.git/tree/CMakeLists.txt says that 
playground-artwork_smaragd is a KF5 repo. Where does the KDE4 detection 
come from?


Thanks, and Happy Easter,
Christoph Feck

D20489: [KIO] Make it compile without foreach (Step 1)

[Christoph Feck]

cfeck added a comment.


  Do we understand all possible regressions that this can cause? If yes, where 
are they documented so that we can verify? See bug 406426.

REPOSITORY
  R241 KIO

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

To: mlaurent, dfaure
Cc: cfeck, aacid, cgiboudeaux, kde-frameworks-devel, michaelh, ngraham, bruns

D20506: KCharSelect's internal model: ensure rowCount() is 0 for valid indexes

[Christoph Feck]

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


  KF5 coding style:
  
if (...) {
...
}

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  master

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

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

D20434: KateIconBorder: Use UTF-8 char instead of special pixmap as dyn wrap indicator

[Christoph Feck]

cfeck added a comment.


  The proposed character is in Unicode since version 1.1 (1993).

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, cullmann, #ktexteditor
Cc: cfeck, ngraham, kwrite-devel, kde-frameworks-devel, #ktexteditor, gennad, 
domson, michaelh, bruns, demsking, cullmann, sars, dhaumann

Announcement text for the KDE Applications 19.04 release

[Christoph Feck]

Hello developers,

the Applications/19.04 branches have been created, which means the
branches are feature-frozen. Our first release for 2019 is nearing!

Please help our promotion team to write an announcement text that
summarizes all the work that went into KDE Applications since our 18.12
release.

I created https://phabricator.kde.org/T10636 which has a link for the
Etherpad document at https://notes.kde.org/p/applications_19.04_new_features

Please fill this document in the next few days.

--
Christoph Feck
KDE Release Team

D19771: Use placeholder instead of label

[Christoph Feck]

cfeck added a comment.


  In D19771#431674 , @ngraham wrote:
  
  > - "Find" is limited to items in the current view only, and usually pertains 
to text.
  
  
  I prefer the term "Filter" for this. If there is no filter, you see 
everything, if there is a filter, you only see the items that match. No 
additional search is performed.

REPOSITORY
  R39 KTextEditor

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

To: ognarb, #ktexteditor, #vdg
Cc: cfeck, loh.tar, ngraham, kwrite-devel, kde-frameworks-devel, gennad, 
domson, michaelh, bruns, demsking, cullmann, sars, dhaumann

D18915: Fix batchrename changing extension to lower case

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> cfeck wrote in batchrenamejob.cpp:165
> Function/method names are usually lowercase. Also, we don't add `get` for 
> getters, only `set` for setters.
> 
> ⇒ `fileExtension()` ?
> 
> Additionally, we pass QString via reference
> 
> ⇒ `QString ` ?

Actually, const reference:

`const QString `

REPOSITORY
  R241 KIO

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

To: cfoster, #dolphin, #frameworks, abalaji
Cc: cfeck, bruns, ngraham, elvisangelaccio, chinmoyr, kde-frameworks-devel, 
michaelh

D18915: Fix batchrename changing extension to lower case

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> bruns wrote in batchrenamejob.cpp:165
> `QString extension = GetFileExtension(url.fileName());`
> ...
> `static QString BatchRenameJobPrivate::GetFileExtension(QString filename)`

Function/method names are usually lowercase. Also, we don't add `get` for 
getters, only `set` for setters.

⇒ `fileExtension()` ?

Additionally, we pass QString via reference

⇒ `QString ` ?

REPOSITORY
  R241 KIO

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

To: cfoster, #dolphin, #frameworks, abalaji
Cc: cfeck, bruns, ngraham, elvisangelaccio, chinmoyr, kde-frameworks-devel, 
michaelh

T10554: KDiff3 to join Applications

[Christoph Feck]

cfeck added a comment.


  Moving KDiff3 to KDE Applications was suggested at 
https://marc.info/?l=kde-core-devel=155055818529475=2
  
  In https://phabricator.kde.org/T10546 I instructed Michael to ask developers 
for a quick review of the (many) changes made for the KF5 port.
  
  Apparently Michael prefers Phabricator task to mailing list postings ...

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

To: cfeck
Cc: cfeck, aacid, kde-frameworks-devel, mreeves

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

[Christoph Feck]

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

REPOSITORY
  R320 KIO Extras

BRANCH
  addsyntaxhighlighttotextpreview

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

To: kossebau, broulik, cfeck
Cc: vkrause, cfeck, kde-frameworks-devel, kfm-devel, alexde, feverfew, 
michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, mikesomov

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> textcreator.cpp:169
> +
> syntaxHighlighter.setDefinition(m_highlightingRepository.definitionForFileName(path));
> +
> syntaxHighlighter.setTheme(m_highlightingRepository.defaultTheme(KSyntaxHighlighting::Repository::LightTheme));
> +

KSyntaxHighlighting::Theme also provides a background color. Can that be used 
instead of the hardcoded 245? I cannot remember reason why I disabled (or never 
enabled) the QPalette code; I suggest to remove it.

REPOSITORY
  R320 KIO Extras

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

To: kossebau, broulik
Cc: cfeck, kde-frameworks-devel, kfm-devel, alexde, feverfew, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D19170: Fix crash while moving files

[Christoph Feck]

cfeck added a comment.


  That a nested event loop of an error dialog caused the crashes makes sense, 
because they were reported for alien drives (NTFS) or transfers with permission 
problems. What actual error dialogs did you get and are they reproducible?

REPOSITORY
  R241 KIO

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

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

D18849: [KPropertiesDialog] Fix group combobox

[Christoph Feck]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:2bba2c0795d7: [KPropertiesDialog] Fix group combobox 
(authored by cfeck).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18849?vs=51180=52017

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

AFFECTED FILES
  src/widgets/kpropertiesdialog.cpp

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

Re: KDE release

[Christoph Feck]

On 02/18/19 15:00, Michael Reeves wrote:

I'm looking to tag for 1.8 at the end of the week. If more time is needed


More time for what? Do you plan to include KDiff in the KDE Applications 
19.04 bundle?



let me know. Binary files are ignored during diff generation due tomorrow
assumptions in the under laying code the require a text file. The plan is
allow simple is equal comparison with but binaries. However the current
comparison code does not respond well to such files.


ENOPARSE

Christoph Feck

D18968: Word-wrap KMessageWidget text

[Christoph Feck]

cfeck added a comment.


  Dolphin could simply call KStringHandler::*squeeze(), maybe only on the 
actual filepath, if squeezing is preferred to wrapping.

REPOSITORY
  R318 Dolphin

BRANCH
  word-wrap-long-kmessagewidget-text (branched from Applications/18.12)

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

To: ngraham, #dolphin, #frameworks, cfeck
Cc: elvisangelaccio, cfeck, kfm-devel, alexde, feverfew, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, mikesomov

D19056: Tell people they should mostly be using KF5::AuthCore

[Christoph Feck]

cfeck retitled this revision from "Tell people they should mostly be using 
KF5::Auth" to "Tell people they should mostly be using KF5::AuthCore".

REPOSITORY
  R283 KAuth

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

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