D29050: WIP KRunner fix prepare/teardown signals

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,

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

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, LeGa

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

cfeck added inline comments. INLINE COMMENTS > kcolorcombo.cpp:89 > +int unused, v; > +innercolor.getHsv(&unused, &unused, &v); > +textColor = v > 128 ? Qt::black : Qt::white; Please don't use "value" component to calculate the brightness of a color. #81

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

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-framewor

D25814: [KColorScheme] Add SeparatorColor

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.kd

D25267: Improve XCF support

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, mic

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

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

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

D27802: smb: fix ipv6 support

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,

D27802: smb: fix ipv6 support

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,

kreversi 19.12 fails to build at IconSize call

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

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

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,

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

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 > kmessagewidg

Re: 2 kirigami fixes for a point release

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 Pl

D27195: Change "Redisplay" to "Refresh"

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 RE

D27087: Add HEIF thumbnailer

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

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

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

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

D25814: [KColorScheme] Add SeparatorColor

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, manu

D26648: Improved quality of JPEG thumbnails

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,

D26648: Improved quality of JPEG thumbnails

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, M

D26648: Improved quality of JPEG thumbnails

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 DET

D26448: Add KRecentFilesMenu to replace KRecentFileAction

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::KRecentF

D25814: [KColorScheme] Add SeparatorColor

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 htt

D25697: Port away from KIconLoader::SizeSmallMedium

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

kde-frameworks-devel@kde.org

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 &url); > */ Wrong comment? REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

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

D25444: Fix assistant dialog margins

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

D24993: Add integrated inline spelling menu to KTextEdit

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

D24993: Add integrated inline spelling menu to KTextEdit

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-framewo

Re: New Framework Review: KDAV

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

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

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, ap

D24367: Some sanity verification

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` bl

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

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

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

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

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: l

D24113: xcf: Properly read image resolution

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

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, LeGast

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

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&id=66041 REVISI

D23904: Add QIcon setters for the password dialogs

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, d

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

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&id=65784 REVISION DETAIL https://phabricator.kde.org/D23811 AFFECTED FILES autotests/CMakeLists.

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

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 Ko

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

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, mi

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

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, micha

D23597: Bulk port away from foreach

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 `&`

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

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,

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

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, himcesj

D13867: [KMessageWidget] Pass widget to standardIcon()

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&column=y` for URLs starting with remote schemes.

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 C

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

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

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"), > &window); > + typo REPOSITORY R236 KWidgetsAddons BRANCH maste

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

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, davidedmundso

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

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. > >

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

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. R

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

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:

D22083: introduce concept of header and footer for kpageview

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 fo

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

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

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

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&id=58399#toc REPOSITORY R241 KIO CHANGES SI

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

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, #do

D21028: add multiple gestures and a handler class to KWidgetsAddons

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,

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

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, ngr

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

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, ngraha

D20766: Use appropriate background color for text previews

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

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

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-f

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

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-framewo

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

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, m

D19771: Use placeholder instead of label

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

D18915: Fix batchrename changing extension to lower case

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 &filenam

D18915: Fix batchrename changing extension to lower case

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`

T10554: KDiff3 to join Applications

cfeck added a comment. Moving KDiff3 to KDE Applications was suggested at https://marc.info/?l=kde-core-devel&m=155055818529475&w=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

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

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, fev

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

cfeck added inline comments. INLINE COMMENTS > textcreator.cpp:169 > + > syntaxHighlighter.setDefinition(m_highlightingRepository.definitionForFileName(path)); > + > syntaxHighlighter.setTheme(m_highlightingRepository.defaultTheme(KSyntaxHighlighting::Repository::LightThem

D19170: Fix crash while moving files

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

D18849: [KPropertiesDialog] Fix group combobox

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&id=52017 REVISION DETAIL http

D18968: Word-wrap KMessageWidget text

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://phabricat

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

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

D18968: Word-drap KMessageWidget text

cfeck added a comment. Well, fix the TODO, and you can remove the comment ;) I guess QLabel needs new API to set https://doc.qt.io/qt-5/qtextoption.html#WrapMode-enum But those plans should be tracked elsewhere, not in source. IMHO, you or maintainer decides, not me. REPOSITORY R

D18968: Word-drap KMessageWidget text

cfeck accepted this revision. cfeck added a comment. This revision is now accepted and ready to land. I would just remove the comment. The commit references the bug anyway, and a bug reference in the code is only helpful if the code is wrong, and someone wants to understand why. REPOSITORY

D18731: Replace KIconThemes dependency with equivalent QIcon usage

cfeck accepted this revision. cfeck added a comment. But KDE also supports REPOSITORY R294 KBookmarks BRANCH master REVISION DETAIL https://phabricator.kde.org/D18731 To: vkrause, davidedmundson, cfeck Cc: broulik, cfeck, kde-frameworks-devel, michaelh, ngraham, bruns

D18849: [KPropertiesDialog] Fix group combobox

cfeck created this revision. cfeck added reviewers: Frameworks, Dolphin. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. cfeck requested review of this revision. REVISION SUMMARY `groupList` was never initialized with `KUser::groupNames()`, causing the subse

D18731: Replace KIconThemes dependency with equivalent QIcon usage

cfeck added a comment. Are application-specific icons now accessible with QIcon::fromTheme? Think Konqueror favicons in bookmark menu. Also, according to https://api.kde.org/frameworks/kbookmarks/html/kbookmarks-dependencies.html KBookmarks indirectly depends on KIconThemes via KTextWid

D18699: [KIO/RenameDialog] Add new apply behaviour

cfeck added a comment. The "Apply to All" button is used for all choices, including "Overwrite", "Skip", and "Rename". REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18699 To: chinmoyr, dfaure, ngraham, #vdg Cc: cfeck, broulik, kde-frameworks-devel, michaelh, ngraham,

D18563: Don't create directory tree when a new folder has a '/' in the name

cfeck added a comment. It could ask what to do. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18563 To: shubham, ngraham, #frameworks, #dolphin, dfaure, elvisangelaccio, pino Cc: cfeck, acrouthamel, markg, ndavis, dfaure, elvisangelaccio, pino, kde-frameworks-devel, m

D18380: KIO: make file dialog columns resizable again (and movable)

cfeck added a comment. As far as I know, using qobject_cast is faster than comparing class names, because it only compares metaclass pointers. Additionally, it allows subclasses. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham, #frameworks, #dol

D18380: KIO: make file dialog columns resizable again (and movable)

cfeck added inline comments. INLINE COMMENTS > kdiroperatordetailview.cpp:54 > +} > +m_isFileWidget = pw ? strcmp(pw->metaObject()->className(), > "KFileWidget") == 0 : false; > +// install the section resize handler Can we use qobject_cast here? REPOSITORY R241 KIO

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

cfeck added a comment. Thanks for testing, btw. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18161 To: cfeck, #frameworks, #dolphin, dfaure Cc: nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns

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

cfeck added a comment. It's possible that Kate doesn't use KIO for local files, but falls back to QFile. It _would_ be possible somehow (e.g. via the QPA plugins) to inject the hack into all Qt applications, but I suggest to improve Dolphin in a way that it shows an error message or even a r

D17816: Support for xattrs on kio copy/move

cfeck added a comment. I am still not fond of having a local event loop in KIO. The whole point of KIO is that it should work asynchronously. If the tests fail, could they be adapted? Or is the reason why the tests fail unfixable? REPOSITORY R241 KIO REVISION DETAIL https://phabric

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

cfeck updated this revision to Diff 49684. cfeck added a comment. Fix worst case estimate. We need up to two UTF-16 code units for each byte. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18161?vs=49176&id=49684 REVISION DETAIL https://phabricator.kde.org/D

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

cfeck created this revision. cfeck added reviewers: Frameworks, Dolphin. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. cfeck requested review of this revision. REVISION SUMMARY **[WIP/RFC] Please let me know if what I propose is sane** UNIX filenames c

D18138: KRatingPainter: Delete copy constructor and assignment operator

cfeck accepted this revision. cfeck added a comment. This revision is now accepted and ready to land. Right. If someone would copy the object using the default implementations, they would only get two instances pointing to the same Private. REPOSITORY R236 KWidgetsAddons BRANCH mast

D17816: Support for xattrs on kio copy/move

cfeck added inline comments. INLINE COMMENTS > copyjob.cpp:1119 > +KJob *job = KIO::copy_xattr((*it).uSource, (*it).uDest); > +job->exec(); > //this is required for the undo feature Here, too. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17816

D17816: Support for xattrs on kio copy/move

cfeck added inline comments. INLINE COMMENTS > filecopyjob.cpp:519 > +KJob *job = KIO::copy_xattr(d->m_src, d->m_dest); > +job->exec(); > if (d->m_move) { This waits (i.e. spawns a separate event loop) until the job is finished. Should use a subjob. REPOSITORY R241 K

D17596: [KDirOperator] Allow renaming files from the context menu

cfeck added a comment. What is the status of this patch? It missed the 5.54 deadline, so the version would need to be adjusted. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17596 To: ngraham, #frameworks, #dolphin Cc: cfeck, emateli, elvisangelaccio, markuss, dhauman

D15573: replace custom backtracing in SlaveBase with KCrash

cfeck added a comment. This seems to pull in QtGui dependency, but that's also dragged in by KService (which also links to KF5::Crash) and KF5DBusAddons (which only needs QtX11Extras, which unfortunately also needs QtGui). Can the KCrash dependency be added to the slaves instead of to th

D17907: [KWidgetsAddons] Do not use light font styles for headings (3/3)

cfeck added inline comments. INLINE COMMENTS > ktitlewidget.cpp:242 > d->textLabel->setStyleSheet(d->textStyleSheet()); > -//Qt stylesheet doesn't support lighter font-weight > -QFont font(d->textLabel->font()); > -if (d->level <= 4) { > -font.setWeight(QFont::Light); > -

D17828: Don't check twice if the icon exists from ::availableSizes

cfeck added a comment. Maybe add 'const found = ', while you are at it. REPOSITORY R302 KIconThemes BRANCH master REVISION DETAIL https://phabricator.kde.org/D17828 To: apol, #frameworks, cfeck Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17828: Don't check twice if the icon exists from ::availableSizes

cfeck accepted this revision. This revision is now accepted and ready to land. REPOSITORY R302 KIconThemes BRANCH master REVISION DETAIL https://phabricator.kde.org/D17828 To: apol, #frameworks, cfeck Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17190: Add level api from Kirigami.Heading

cfeck added inline comments. INLINE COMMENTS > ktitlewidget.cpp:132 > titleFrame->setBackgroundRole(QPalette::Base); > +titleFrame->setContentsMargins(0, 0, 0, 0); > Setting 0 margins effectively disables the frame; the Breeze setting to keep frames around page titles is no longer re

  1   2   3   4   5   6   7   >