D29397: KPreviewJob : Support for DeviceRatioPixel

meven marked an inline comment as done. meven added inline comments. INLINE COMMENTS > dfaure wrote in previewjob.cpp:717 > This here also breaks compatibility. Add a KF6 TODO to start the > serialization with a version number. > > Meanwhile a hack is needed, like `if (iFormat & 0x1000) {

D29397: KPreviewJob : Support for DeviceRatioPixel

broulik added inline comments. INLINE COMMENTS > thumbcreator.h:139 > */ > -virtual bool create(const QString , int width, int height, QImage > ) = 0; // KF6 TODO: turn first arg into QUrl (see > thumbnail/htmlcreator.cpp) > +virtual bool create(const QString , int width, int

D29397: KPreviewJob : Support for DeviceRatioPixel

meven updated this revision to Diff 81971. meven marked 5 inline comments as done. meven added a comment. Add Binary compatibility workarounds REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29397?vs=81836=81971 BRANCH preview-dpr REVISION DETAIL

D29406: Add X-KDE-Original-Executable to Applications properties

meven added a comment. In D29406#663112 , @apol wrote: > > If I understand correctly, it is necessary to use dbus spectacle in this case, so that dbus can manage the application instance and make sure we end up with only one, whether we launch

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

rjvbb added a comment. > It's "KTextEditor", not "KCodeEditor". Yes, but look at the traditional meaning of a text editor, which typically means "plain text" editor. KTextEditor's design decision to use a single lineheight puts it squarely in that domain - to reply in style: `It's

D29381: Thumbnail text: use libmagic to detect encoding

meven added inline comments. INLINE COMMENTS > sitter wrote in textcreator.cpp:38 > TBH, I would make libmagic required for building the thumbnail plugin. I > can't see much of a rationale for why we'd want to support > "broken"/insufficient encoding detection when there's code that makes it

D29420: Generate DBus interface

broulik added inline comments. INLINE COMMENTS > notifybypopup.cpp:115 > > - bool connected = QDBusConnection::sessionBus().connect(QString(), // > from any service > - > QString::fromLatin1(dbusPath), Previously it would

D29397: KPreviewJob : Support for DeviceRatioPixel

dfaure added a comment. Oh, I thought it was sent as an int. But 8 is QImage::Format_ARGB8565_Premultiplied. Did you mean 0x80? INLINE COMMENTS > thumbcreator.h:191 > + * @class ThumbCreatorV3 thumbcreator.h > + * @since 5.70 > + */ 5.70 is tagged already REPOSITORY R241 KIO REVISION

D29231: Add keyboard_shortcuts_inhibit protocol

bport abandoned this revision. bport added a comment. Follow up on gitlab https://invent.kde.org/kde/kwayland-server/-/merge_requests/1 REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D29231 To: bport, zzag, davidedmundson, apol Cc: romangg, crossi,

D29397: KPreviewJob : Support for DeviceRatioPixel

meven added inline comments. INLINE COMMENTS > previewjob.cpp:433 > +{ > +d_func()->devicePixelRatio = dpr; > +} Maybe make this static, so that apps have to do it once per app sort of like we do with `Qt::AA_UseHighDpiPixmaps`, rather than by KPreviewJob. REPOSITORY R241 KIO REVISION

D29397: KPreviewJob : Support for DeviceRatioPixel

meven marked an inline comment as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29397 To: meven, dfaure, broulik, #frameworks Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29397: KPreviewJob : Support for DeviceRatioPixel

meven updated this revision to Diff 81982. meven added a comment. Improve doc, fix @since REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29397?vs=81971=81982 BRANCH preview-dpr REVISION DETAIL https://phabricator.kde.org/D29397 AFFECTED FILES

D29445: [KOpenWithDialog] When pointing at a non-executable file print more meaningful error

dfaure added inline comments. INLINE COMMENTS > apol wrote in kopenwithdialog.cpp:1006 > This patch D29170 suggests that > findExecutable won't find non-executables. > > Something's wrong. Right, hence the isEmpty() here. This check passes if 1) it doesn't

D29447: Fix showing updates when the option is selected

alex added inline comments. INLINE COMMENTS > itemsmodel.cpp:71 > +bool duplicate{false}; > +for (const EntryInternal : qAsConst(m_entries)) { > +if (existingEntry == entry) { On second thought why not just use: `bool duplicate = m_entries.contains(entry);` REPOSITORY R304

D29447: Fix showing updates when the option is selected

ngraham accepted this revision. This revision is now accepted and ready to land. REPOSITORY R304 KNewStuff BRANCH fix-show-only-updates (branched from master) REVISION DETAIL https://phabricator.kde.org/D29447 To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks Cc: alex,

D29445: [KOpenWithDialog] When pointing at a non-executable file print more meaningful error

ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in kopenwithdialog.cpp:1006 > Right, hence the isEmpty() here. This check passes if 1) it doesn't exist, or > 2) it's not executable. I was wondering how the QString returned by KIO::DesktopExecParser::executablePath() would

D29447: Fix showing updates when the option is selected

leinir added reviewers: KNewStuff, Plasma, bugseforuns, ngraham, Frameworks. leinir added projects: Plasma, KNewStuff. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D29447 To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks Cc: kde-frameworks-devel,

D29451: KNS: Do not mark entry as installed if install script failed

alex created this revision. alex added reviewers: KNewStuff, ngraham, leinir. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. alex requested review of this revision. REVISION SUMMARY If the install script failed/was aborted the entry gets marked as

D29381: Thumbnail text: use libmagic to detect encoding

sitter added inline comments. INLINE COMMENTS > meven wrote in textcreator.cpp:38 > Without libmagic, it is current state basically UTF-8 with bom detection > otherwise local codec. > > I did not test exhaustive encodings so I wanted to let the door open for > users to not rely on libmagic. >

D29447: Fix showing updates when the option is selected

leinir created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. leinir requested review of this revision. REVISION SUMMARY This patch primarily fixes the issue that when picking the "Updateable Only" option on NewStuff.Page, we would be shown

D29050: WIP KRunner fix prepare/teardown signals

alex added a comment. How should I proceed with this patch? REPOSITORY R308 KRunner REVISION DETAIL https://phabricator.kde.org/D29050 To: alex, meven, ngraham, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

brauch added a comment. In D25339#663915 , @fakefred wrote: > I second the as-an-option proposal. Hey, why not automatically increase the line height when CJK characters are detected? This issue has been around for years and actually

D29415: Add holiday file for DE-BE (Germany/Berlin)

This revision was automatically updated to reflect the committed changes. Closed by commit R175:c39d1eb12217: Add holiday file for DE-BE (Germany/Berlin) (authored by vkrause). REPOSITORY R175 KHolidays CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29415?vs=81905=81999 REVISION

D29406: Add X-KDE-Original-Executable to Applications properties

meven abandoned this revision. meven added a comment. This not necessary, spectacle should have its executable in its Exec desktop entry. REPOSITORY R309 KService REVISION DETAIL https://phabricator.kde.org/D29406 To: meven, #frameworks, davidedmundson, apol, bport Cc:

D29447: Fix showing updates when the option is selected

leinir updated this revision to Diff 82003. leinir added a comment. Address comment by @alex - Fix style (and consty things) REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29447?vs=81993=82003 BRANCH fix-show-only-updates (branched from master)

D29451: KNS: Do not mark entry as installed if install script failed

alex edited the summary of this revision. alex edited the test plan for this revision. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D29451 To: alex, #knewstuff, ngraham, leinir Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29445: [KOpenWithDialog] When pointing at a non-executable file print more meaningful error

apol added inline comments. INLINE COMMENTS > kopenwithdialog.cpp:1006 > // Ensure that the typed binary name actually exists (#81190) > if (QStandardPaths::findExecutable(binaryName).isEmpty()) { > +// Maybe it exists but isn't executable This patch D29170

D29447: Fix showing updates when the option is selected

alex added inline comments. INLINE COMMENTS > itemsmodel.cpp:71 > +bool duplicate{false}; > +for (const EntryInternal& existingEntry : m_entries) { > +if (existingEntry == entry) { Use qAsConst(m_entries) and space before & not after :-) REPOSITORY R304 KNewStuff REVISION

D29017: WIP: Introduce three new methods that return all "entries" in a folder

blaze added a comment. Check this commit message https://phabricator.kde.org/R32:f56cdda54b7f88b119f2c7cfb2f534b4fe74478f REPOSITORY R311 KWallet REVISION DETAIL https://phabricator.kde.org/D29017 To: ahmadsamir, #frameworks, dfaure Cc: blaze, kde-frameworks-devel, LeGast00n, cblack,

D29461: Fix kio-extras build on Windows

brute4s99 updated this revision to Diff 82039. brute4s99 added a comment. needs testing on windows, will update in a while REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29461?vs=82036=82039 BRANCH arcpatch-D29461 REVISION DETAIL

D29463: Fix Kirigami.Units.devicePixelRatio=1.3 when it should be 1.0 at 96dpi

Zren added reviewers: Kirigami, mart. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D29463 To: Zren, #kirigami, mart Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29461: Fix kio-extras build on Windows

brute4s99 created this revision. brute4s99 added a reviewer: vonreth. Herald added projects: Dolphin, Frameworks. Herald added subscribers: kfm-devel, kde-frameworks-devel. brute4s99 requested review of this revision. REPOSITORY R320 KIO Extras BRANCH master REVISION DETAIL

D25814: [KColorScheme] Add SeparatorColor

ndavis added a comment. In D25814#598380 , @cfeck wrote: > > Why don't focus and hover colors belong? > > Because an application cannot know if and how a style does indicate focus or hovering. I don't understand this objection. What

D29420: Generate DBus interface

nicolasfella added inline comments. INLINE COMMENTS > broulik wrote in notifybypopup.cpp:115 > Previously it would accept the signal from any service which I find odd, > though. Could you maybe check git logs to see if there was a reason for this? > It should survive restarts anyway and the

D29463: Fix Kirigami.Units.devicePixelRatio=1.3 when it should be 1.0 at 96dpi

Zren created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. Zren requested review of this revision. REVISION SUMMARY I recently noticed that `Kirigami.Units.devicePixelRatio` was `1.3` even though I was at the default 96dpi when writing a

D29454: [TaglibExtractor] Add support for Audible "Enhanced Audio" audio books

This revision was automatically updated to reflect the committed changes. Closed by commit R286:0975ed45af79: [TaglibExtractor] Add support for Audible Enhanced Audio audio books (authored by bruns). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE

D25814: [KColorScheme] Add SeparatorColor

ngraham added a comment. I have to agree with @ndavis here. Adding a separator color to the color scheme seems sensible enough to me, for the reasons previously provided. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis, #frameworks, #vdg Cc:

D29447: Fix showing updates when the option is selected

leinir added inline comments. INLINE COMMENTS > alex wrote in itemsmodel.cpp:71 > On second thought why not just use: > `bool duplicate = m_entries.contains(entry);` Hmm... i do wonder slight of the cost of that, but also... much simpler code, so... basically can just go if

D28221: Don't write default value to configuration file when default value came from /etc/* file

bport added a comment. In D28221#657482 , @dfaure wrote: > Ah! Now it actually makes sense to me. If we are changing what revertToDefault() does, then it makes sense to change the if() condition for calling it. Basically, now that it does the

D29454: [TaglibExtractor] Add support for Audible "Enhanced Audio" audio books

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Nice! REPOSITORY R286 KFileMetaData BRANCH master REVISION DETAIL https://phabricator.kde.org/D29454 To: bruns, #frameworks, #baloo, ngraham, astippich, mgallien Cc:

D28221: Don't write default value to configuration file when default value came from /etc/* file

bport updated this revision to Diff 82028. bport added a comment. - Fix comments - Ensure we don't have problem if we set value to "defaultcpp" and a global setting override it Regarding other comments: - I guess you mean if (mDefault == mReference) { with if (value ==

D29454: [TaglibExtractor] Add support for Audible "Enhanced Audio" audio books

bruns added a comment. In D29454#664222 , @ngraham wrote: > Nice! and when you combine it with https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/577, Elisa can play Audible audio books ;-) REPOSITORY R286

D29447: Fix showing updates when the option is selected

leinir updated this revision to Diff 82021. leinir added a comment. As @alex suggests, just use qlist::contains, it is supposed to be reasonably cheap, so... yup, trust the framework! ;) - Just use qlist::contains REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE

D29455: KNS: Deprecate isRemote method and handle parse error properly

alex updated this revision to Diff 82024. alex retitled this revision from "KNS: Remove isRemote method and handle parse error properly" to "KNS: Deprecate isRemote method and handle parse error properly". alex added a comment. Make isRemote deprecated REPOSITORY R304 KNewStuff CHANGES

D29363: Use UI marker context in more tr() calls

kossebau added a comment. Thanks Albert (& Luigi) for review. With no-one objecting, I would proceed to push this upcoming WE, 9/10th May, to be early in the cycle still. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D29363 To: kossebau, #frameworks,

D29454: [TaglibExtractor] Add support for Audible "Enhanced Audio" audio books

bruns created this revision. bruns added reviewers: Frameworks, Baloo, ngraham, astippich, mgallien. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY Audible AAX files are standard ISO Base Media

D29455: KNS: Remove isRemote method and handle parse error properly

alex created this revision. alex added reviewers: KNewStuff, ngraham, leinir. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. alex requested review of this revision. REVISION SUMMARY The isRemote check and the FIXMEs are removed, instead the result from the

D29447: Fix showing updates when the option is selected

leinir marked 2 inline comments as done. leinir added a comment. Thanks for making me realise that it doesn't have to be quite so elaborate, @alex ;) REPOSITORY R304 KNewStuff BRANCH fix-show-only-updates (branched from master) REVISION DETAIL https://phabricator.kde.org/D29447 To:

D29447: Fix showing updates when the option is selected

alex added a comment. No problem, always happy to help  REPOSITORY R304 KNewStuff BRANCH fix-show-only-updates (branched from master) REVISION DETAIL https://phabricator.kde.org/D29447 To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks Cc: alex, kde-frameworks-devel,

D29455: KNS: Remove isRemote method and handle parse error properly

leinir requested changes to this revision. leinir added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > installation.h:76 > bool readConfig(const KConfigGroup ); > -bool isRemote() const; > i'm afraid this is an exported class, at most we can mark

D28221: Don't write default value to configuration file when default value came from /etc/* file

bport planned changes to this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D28221 To: bport, ervin, dfaure, davidedmundson Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29434: Use small font for ExpandableListItem subtitle

ndavis accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) BRANCH ExpandableListItem-small-subtitle-font (branched from master) REVISION DETAIL https://phabricator.kde.org/D29434 To: ngraham, #plasma, #vdg, broulik, ndavis

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

pshinjo added a comment. In D25339#663833 , @rjvbb wrote: > doesn't it give you US-ASCII canonical representations of every possible glyph? Whether it is possible or not, no CJK users will write TeX document using US-ASCII

D29397: KPreviewJob : Support for DeviceRatioPixel

davidedmundson added a comment. > Allow users of KPreviewJob to request a devicePixelRatio for generated thumbnails. At the risk of asking a stupid question, why? As opposed to just having a width and height always be in device pixels. We're always working with pixmaps is there a

D29397: KPreviewJob : Support for DeviceRatioPixel

meven added a comment. In D29397#663902 , @broulik wrote: > > At the risk of asking a stupid question, why? > > The text thumbnailer should be able to produce readable text on high dpi, or the folder thumbnailer should render the folder icon

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

fakefred added a comment. I second the as-an-option proposal. Hey, why not automatically increase the line height when CJK characters are detected? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D25339 To: xuetianweng, #ktexteditor, cullmann, dhaumann,

D29397: KPreviewJob : Support for DeviceRatioPixel

broulik added a comment. > At the risk of asking a stupid question, why? The text thumbnailer should be able to produce readable text on high dpi, or the folder thumbnailer should render the folder icon sharply and the picture frames non-pixelated REPOSITORY R241 KIO REVISION DETAIL

D29445: [KOpenWithDialog] When pointing at a non-executable file print more meaningful error

broulik created this revision. broulik added reviewers: Frameworks, VDG. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. broulik requested review of this revision. REVISION SUMMARY `QStandardPaths::findExecutable` only finds executable binaries, so when