Re: Plasma-nm: passing on maintainership

2022-10-04 Thread Ahmad Samir
. Best regards, Jan Grulich Thanks for all the work (features and bugs alike, as you know software is always a mixed bag :D), and yes, indeed a very core component of Plasma. As Jonathan said, don't be a stranger, KDE has very good memory :) Regards, Ahmad Samir OpenPGP_signature

Re: KickOff bug report page

2022-08-28 Thread Ahmad Samir
uot;plasmashell" product in bugzilla. Regards, Ahmad Samir OpenPGP_signature Description: OpenPGP digital signature

D26675: [sddm-theme] Don't have a broken reveal password button

2021-10-04 Thread Ahmad Samir
ahmadsamir added a comment. In D26675#678819 , @davidre wrote: > we could install an icon into hicolor as fallback Some time ago I tried finding which icon was being used, but I didn't get anywhere and moved on (I copied the theme

D26675: [sddm-theme] Don't have a broken reveal password button

2021-10-01 Thread Ahmad Samir
ahmadsamir added a comment. One was to fix this is if sddm have copies of the needed password buttons, then it should work regardless of the installed icon themes. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26675 To: davidedmundson, #plasma Cc:

D26675: [sddm-theme] Don't have a broken reveal password button

2021-10-01 Thread Ahmad Samir
ahmadsamir added a comment. In D26675#678811 , @bharadwaj-raju wrote: > Is there an SDDM issue to track this? Because I edited the file to make `revealPasswordButtonShown` true, and the button shows up just fine. Which makes me think this

Re: Using C++20 in Plasma

2021-08-06 Thread Ahmad Samir
?). Is there any distro planning to ship future Plasma releases that does not have gcc 10 or clang 10? Cheers Nico -- Ahmad Samir

D25694: Port away from DesktopIcon

2020-12-16 Thread Ahmad Samir
ahmadsamir added a comment. Ping :) REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D25694 To: nicolasfella, #plasma Cc: ahmadsamir, apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham,

D29710: [Fonts KCM] Remove last remnants of setNearestExistingFonts()

2020-06-05 Thread Ahmad Samir
ahmadsamir closed this revision. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D29710 To: ahmadsamir, #plasma, bport, broulik, ngraham Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot,

D29687: [IconApplet] Port KRun to ApplicationLauncherJob

2020-05-18 Thread Ahmad Samir
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R120:e3b251fd5b06: [IconApplet] Port KRun to ApplicationLauncherJob (authored by ahmadsamir). REPOSITORY R120 Plasma

D29687: [IconApplet] Port KRun to ApplicationLauncherJob

2020-05-17 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 83017. ahmadsamir added a comment. Remove one include REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29687?vs=82943=83017 BRANCH l-krun-port (branched from master) REVISION DETAIL

D29687: [IconApplet] Port KRun to ApplicationLauncherJob

2020-05-15 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > broulik wrote in iconapplet.cpp:435 > but "desktop file path" is what we're doing. what I believe is happening that > you're actually launching the original file, not the one the icon uses. I tried with: KService::Ptr service =

D29687: [IconApplet] Port KRun to ApplicationLauncherJob

2020-05-15 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 82943. ahmadsamir added a comment. Address comments REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29687?vs=82714=82943 BRANCH l-krun-port (branched from master) REVISION DETAIL

D29687: [IconApplet] Port KRun to ApplicationLauncherJob

2020-05-15 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > broulik wrote in iconapplet.cpp:435 > But why would `serviceByStorageId` work then? > Looks like this needs to be `KService::Ptr(new KService(m_localPath))` then? According to the kservice docs KService::serviceByStorageId() param is "the

D29687: [IconApplet] Port KRun to ApplicationLauncherJob

2020-05-15 Thread Ahmad Samir
ahmadsamir added a comment. Ping. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D29687 To: ahmadsamir, #plasma, broulik Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf,

D29710: [Fonts KCM] Remove last remnants of setNearestExistingFonts()

2020-05-13 Thread Ahmad Samir
ahmadsamir added a comment. Continuing the discussion from IRC, plasma 5.18 branch still uses QFontDialog, so https://bugs.kde.org/show_bug.cgi?id=420287 isn't directly related. However if users remove the ',Regular' bit from kdeglobals, QFontDialog will probably select the wrong style

D29710: [Fonts KCM] Remove last remnants of setNearestExistingFonts()

2020-05-13 Thread Ahmad Samir
ahmadsamir added a comment. In D29710#670151 , @ahmadsamir wrote: > Also, is it OK to backport https://commits.kde.org/plasma-desktop/0325d698181055cdaaa93b24ee80172132822d35 and this diff to Plasma/5.18? Scratch that, 5.18 min.

D29710: [Fonts KCM] Remove last remnants of setNearestExistingFonts()

2020-05-13 Thread Ahmad Samir
ahmadsamir added a comment. Also, is it OK to backport https://commits.kde.org/plasma-desktop/0325d698181055cdaaa93b24ee80172132822d35 and this diff to Plasma/5.18? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D29710 To: ahmadsamir, #plasma, bport,

D29710: [Fonts KCM] Remove last remnants of setNearestExistingFonts()

2020-05-13 Thread Ahmad Samir
ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, bport, broulik. Herald added a project: Plasma. ahmadsamir requested review of this revision. REVISION SUMMARY This cleans up after commit 0325d698181055cdaaa93

D29687: [IconApplet] Port KRun to ApplicationLauncherJob

2020-05-13 Thread Ahmad Samir
ahmadsamir added a comment. I forgot to submit my reply to your inline comment INLINE COMMENTS > broulik wrote in iconapplet.cpp:435 > `m_localPath` is a *path* so you want `serviceByDesktopPath` I tried that first, and it doesn't work; those .desktop files are in a

D29687: [IconApplet] Port KRun to ApplicationLauncherJob

2020-05-13 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 82714. ahmadsamir added a comment. Use KNotificationJobUiDelegate REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29687?vs=82681=82714 BRANCH l-krun-port (branched from master) REVISION DETAIL

D29687: [IconApplet] Port KRun to ApplicationLauncherJob

2020-05-12 Thread Ahmad Samir
ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, broulik. Herald added a project: Plasma. ahmadsamir requested review of this revision. TEST PLAN Open the Application Launcher, right click any app -> Add to Panel (Widget), then click the icon on the panel, it should

D29155: [Fonts KCM] Remove redundant nearestExistingFont()

2020-05-05 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes. Closed by commit R119:0325d6981810: [Fonts KCM] Remove redundant nearestExistingFont() (authored by ahmadsamir). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE

D29155: [Fonts KCM] Remove redundant nearestExistingFont()

2020-05-05 Thread Ahmad Samir
ahmadsamir added a comment. In D29155#663178 , @bport wrote: > I don't think we will have same behavior, we don't only check name but also size, type... > If we are ok to fallback in all case to the same font that can work. > > From your

D29156: [kcms/fonts] When adjusting all fonts, keep Small font size smaller

2020-04-28 Thread Ahmad Samir
ahmadsamir accepted this revision. ahmadsamir added a comment. This revision is now accepted and ready to land. The change makes sense; but please wait for @bport review too. REPOSITORY R119 Plasma Desktop BRANCH make-small-font-smaller-when-changing-all-font-sizes (branched from master)

D29156: [kcms/fonts] When adjusting all fonts, keep Small font size smaller

2020-04-28 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > fonts.cpp:271 > +// tiny, they want a tiny font everywhere. > +if (font.pointSize() > 8) { > +smallestFont.setPointSize(font.pointSize() - 2); Nit-pick: I'd store font.pointSize() in a const int, and use that

D29156: [kcms/fonts] When adjusting all fonts, keep Small font size smaller

2020-04-28 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > fonts.cpp:267 > +QFont smallerSmallestFont = font; > +smallerSmallestFont.setPointSize(font.pointSize() - 2); > + > m_settings->setSmallestReadableFont(applyFontDiff(m_settings->smallestReadableFont(), >

D29155: [Fonts KCM] Remove redundant nearestExistingFont()

2020-04-24 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 81110. ahmadsamir edited the summary of this revision. ahmadsamir removed a reviewer: ngraham. ahmadsamir added a comment. Tweak commit message REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE

D29155: [Fonts KCM] Remove redundant nearestExistingFont()

2020-04-24 Thread Ahmad Samir
ahmadsamir added a reviewer: ngraham. ahmadsamir added a comment. If that diff is a no-go, it'll have to be D27785 . REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D29155 To: ahmadsamir, #plasma, bport, ngraham Cc:

D29155: [Fonts KCM] Remove redundant nearestExistingFont()

2020-04-24 Thread Ahmad Samir
ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, bport. Herald added a project: Plasma. ahmadsamir requested review of this revision. REVISION SUMMARY It seems that nearestExistingFont() gets the same result of `fc-match font-name`, which is basically the sans serif

D29080: [kcms/fonts] Guide users towards KScreen KCM for making things bigger on-screen

2020-04-22 Thread Ahmad Samir
ahmadsamir added a reviewer: bport. ahmadsamir added a comment. Does InlineMessage have a "Don't show again" capability? it'll become irritating after seeing it 1-2 times. IIUC, the fontsHaveChanged() signal, nothing is listening to it in KDE code doesn't mean other 3rd party software

D28000: [kde-cli-tools] Port QRegExp to QRegularExpression

2020-04-21 Thread Ahmad Samir
ahmadsamir added a comment. Ping. REPOSITORY R126 KDE CLI Utilities REVISION DETAIL https://phabricator.kde.org/D28000 To: ahmadsamir, #plasma, apol, davidedmundson Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham,

D28135: Port away from deprecated QSet/QList methods in some places

2020-04-21 Thread Ahmad Samir
ahmadsamir added a comment. Ping. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D28135 To: ahmadsamir, #plasma, davidedmundson, apol Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen,

D28978: [PluginLoader] Replace one usage of QRegExp with QString::startsWith()

2020-04-20 Thread Ahmad Samir
ahmadsamir abandoned this revision. ahmadsamir added a comment. Talking with kbroulik on irc confirms your POV, the format of those patterns aren't documented anywhere so changing the matching now would change the behaviour... I'll abandon this, (I suggest that X-Plasma-DropUrlPatterns

D28978: [PluginLoader] Replace one usage of QRegExp with QString::startsWith()

2020-04-19 Thread Ahmad Samir
ahmadsamir added a reviewer: broulik. ahmadsamir added a comment. I had trouble finding what urlPatterns would look like, and only found trash:/ and run:/, also couldn't find any documentation about X-Plasma-DropUrlPatterns. Porting regex, especially with the pesky QRegExp::Wildcard,

D28978: [PluginLoader] Replace one usage of QRegExp with QString::startsWith()

2020-04-19 Thread Ahmad Samir
ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, apol. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ahmadsamir requested review of this revision. REVISION SUMMARY AFAICS, from grep'ing for "DropUrlPatterns" in

D27808: [Fonts KCM] Use KFontChooserDialog instead of QFontDialog

2020-04-09 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes. Closed by commit R119:90ac7c210411: [Fonts KCM] Use KFontChooserDialog instead of QFontDialog (authored by ahmadsamir). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE

D27808: [Fonts KCM] Use KFontChooserDialog instead of QFontDialog

2020-04-09 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 79736. ahmadsamir added a comment. Rebase REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27808?vs=78519=79736 BRANCH l-kfontchooserdialog (branched from master) REVISION DETAIL

D27518: If KHelpCenter isn't available fallback to opening doc at docs.kde.org

2020-04-07 Thread Ahmad Samir
ahmadsamir added a comment. In D27518#643502 , @sitter wrote: > My point is that this diff adds kguiaddons as a link library but doesn't actually use any symbol, so it may as well not be a link library (or something ought to call some function

D27518: If KHelpCenter isn't available fallback to opening doc at docs.kde.org

2020-04-07 Thread Ahmad Samir
ahmadsamir added a comment. In D27518#643466 , @sitter wrote: > FYI linking a library without actually using any symbols is not going to do anything in practice because many distros link with --as-needed which undoes unnecessary linking. also

D27808: [Fonts KCM] Use KFontChooserDialog instead of QFontDialog

2020-04-04 Thread Ahmad Samir
ahmadsamir added a comment. Ping. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27808 To: ahmadsamir, #plasma, davidedmundson, broulik, meven, cfeck, bport Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2,

D28534: [Klipper] Upate the klipper docbook after porting to QRegularExpression

2020-04-03 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes. ahmadsamir marked an inline comment as done. Closed by commit R120:a8fb4ce827ab: [Klipper] Upate the klipper docbook after porting to QRegularExpression (authored by ahmadsamir). REPOSITORY R120 Plasma Workspace CHANGES

D28534: [Klipper] Upate the klipper docbook after porting to QRegularExpression

2020-04-03 Thread Ahmad Samir
ahmadsamir marked an inline comment as done. ahmadsamir added inline comments. INLINE COMMENTS > yurchor wrote in index.docbook:34 > Can you bump the date and version? > > Thanks in advance for your work. Done. Thanks. REPOSITORY R120 Plasma Workspace REVISION DETAIL

D28534: [Klipper] Upate the klipper docbook after porting to QRegularExpression

2020-04-03 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 79200. ahmadsamir added a comment. Bump date and version REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28534?vs=79197=79200 BRANCH l-klipper-docs (branched from master) REVISION DETAIL

D28534: [Klipper] Upate the klipper docbook after porting to QRegularExpression

2020-04-03 Thread Ahmad Samir
ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, davidedmundson, apol, broulik. Herald added projects: Plasma, Documentation. Herald added a subscriber: kde-doc-english. ahmadsamir requested review of this revision. REPOSITORY R120 Plasma Workspace BRANCH l-klipper-docs

D28519: [KMenuEdit] Port QRegExp to QRegularExpression

2020-04-03 Thread Ahmad Samir
ahmadsamir added a comment. In D28519#640339 , @apol wrote: > It could make sense for you to test that the names are properly parsed besides just making sure it starts. Done. REPOSITORY R103 KMenu Editor REVISION DETAIL

D28519: [KMenuEdit] Port QRegExp to QRegularExpression

2020-04-03 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes. Closed by commit R103:1e30c84ab32b: [KMenuEdit] Port QRegExp to QRegularExpression (authored by ahmadsamir). REPOSITORY R103 KMenu Editor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28519?vs=79188=79194

D28519: [KMenuEdit] Port QRegExp to QRegularExpression

2020-04-03 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 79188. ahmadsamir edited the test plan for this revision. ahmadsamir removed a subscriber: apol. ahmadsamir added a comment. Add proper test plan REPOSITORY R103 KMenu Editor CHANGES SINCE LAST UPDATE

D28519: [KMenuEdit] Port QRegExp to QRegularExpression

2020-04-02 Thread Ahmad Samir
ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, davidedmundson, mlaurent. Herald added a project: Plasma. ahmadsamir requested review of this revision. TEST PLAN The code compiles and kmenuedit still starts as expected REPOSITORY R103 KMenu Editor BRANCH l-QRE

D27785: [Fonts KCM] Change how nearestExistingFonts() finds a matching font

2020-04-01 Thread Ahmad Samir
ahmadsamir abandoned this revision. ahmadsamir added a comment. Not needed with D27808 , which will use KFontChooserDialog everywhere. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27785 To: ahmadsamir, #plasma,

D27830: [Fonts KCM] Make the font selection dialog highlight the correct style

2020-04-01 Thread Ahmad Samir
ahmadsamir abandoned this revision. ahmadsamir added a comment. See D27808 . REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27830 To: ahmadsamir, #plasma, davidedmundson, broulik, ervin, meven, bport Cc: plasma-devel,

D28000: [kde-cli-tools] Port QRegExp to QRegularExpression

2020-04-01 Thread Ahmad Samir
ahmadsamir added a comment. Ping. REPOSITORY R126 KDE CLI Utilities REVISION DETAIL https://phabricator.kde.org/D28000 To: ahmadsamir, #plasma, apol, davidedmundson Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot,

D27914: [Kilpper] Port QRegExp to QRegularExpression

2020-04-01 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes. Closed by commit R120:bcfd7f28370d: [Kilpper] Port QRegExp to QRegularExpression (authored by ahmadsamir). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27914?vs=78634=79040

D28423: [kioclient] Fix 'kioclient5 --commands' usage messages

2020-03-30 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes. Closed by commit R126:30a713025624: [kioclient] Fix kioclient5 --commands usage messages (authored by ahmadsamir). REPOSITORY R126 KDE CLI Utilities CHANGES SINCE LAST UPDATE

D28423: [kioclient] Fix 'kioclient5 --commands' usage messages

2020-03-30 Thread Ahmad Samir
ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, dfaure, apol. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ahmadsamir requested review of this revision. REVISION SUMMARY It should use 'kioclient5'. REPOSITORY R126 KDE CLI Utilities

D27914: [Kilpper] Port QRegExp to QRegularExpression

2020-03-28 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > apol wrote in urlgrabber.h:185 > I think it's a silly way to do the port but okay, probably works and we get > to move on. What is silly about it? The original code in ClipCommandProcess used: const QStringList matches =

D27914: [Kilpper] Port QRegExp to QRegularExpression

2020-03-27 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > apol wrote in urlgrabber.h:185 > Then you want to store a match? The code stores the QRegularExpression::capturedTexts(), m_regexCapturedTexts, which is eventually what ClipCommandProcess uses. REPOSITORY R120 Plasma Workspace REVISION

D27914: [Kilpper] Port QRegExp to QRegularExpression

2020-03-27 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > apol wrote in urlgrabber.h:185 > Then call it m_regularExpression? Another point (which I forgot since it has been some time since I last looked at this patch), QRegExp stores the match info in itself, whereas QRegularExpression stores the

D27914: [Kilpper] Port QRegExp to QRegularExpression

2020-03-27 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > apol wrote in urlgrabber.h:185 > Why did you change how it works right here? I find it confusing to call the pattern "regExp" and the QRegExp object "m_myRegExp". REPOSITORY R120 Plasma Workspace REVISION DETAIL

D27914: [Kilpper] Port QRegExp to QRegularExpression

2020-03-27 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 78634. ahmadsamir added a comment. Rebase; and friendly ping REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27914?vs=77170=78634 BRANCH l-klipper (branched from master) REVISION DETAIL

D28232: [SpellChecking KCM] Fix the build

2020-03-26 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes. Closed by commit R119:646bd3eecc72: [SpellChecking KCM] Fix the build (authored by ahmadsamir). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28232?vs=78554=78555 REVISION

D28232: [SpellChecking KCM] Fix the build

2020-03-26 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 78554. ahmadsamir added a comment. Rebase REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28232?vs=78347=78554 BRANCH l-fix-build (branched from master) REVISION DETAIL https://phabricator.kde.org/D28232

D27808: [Fonts KCM] Use KFontChooserDialog instead of QFontDialog

2020-03-26 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 78519. ahmadsamir retitled this revision from "[Fonts KCM] Port KFontDialog/KFontChooser to QFontDialog" to "[Fonts KCM] Use KFontChooserDialog instead of QFontDialog". ahmadsamir edited the summary of this revision. ahmadsamir edited the test plan for

D28282: KCM/GlobalShortut: convert FOREACH and old signal syntax

2020-03-26 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > export_scheme_dialog.cpp:46 > int item=0; > -Q_FOREACH(QString component, mComponents) > +for(QString component : qAsConst(mComponents)) > { const QString & > export_scheme_dialog.cpp:74 > +const auto buttons =

D27808: [Fonts KCM] Port KFontDialog/KFontChooser to QFontDialog

2020-03-25 Thread Ahmad Samir
ahmadsamir added a comment. I am not so sure about the QPlatform integration bit, it looks easier/cleaner to just use KFontChooserDialog; QFontDialog isn't widely used in KDE code, so porting to KFontChooserDialog is doable (though that sounds like going back in time to using KFontDialog in

D28232: [SpellChecking KCM] Fix the build

2020-03-25 Thread Ahmad Samir
ahmadsamir added a comment. I don't know when the CI will be updated, probably soon based on what I read on IRC #kde-devel, but until then isn't it better to keep the CI working for plasma-desktop? I could rework the patch to make it conditional on 5.14 and keep the iterator-based ctors

D28232: [SpellChecking KCM] Fix the build

2020-03-25 Thread Ahmad Samir
ahmadsamir added a comment. In D28232#634281 , @bport wrote: > Next version of plasma will depend on Qt 5.14 AFAIK so it's not a problem to depend on it But the CI still uses 5.12 for the opensuse image, and probably 5.13 for the

D28135: Port away from deprecated QSet/QList methods in some places

2020-03-24 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 78355. ahmadsamir added a comment. QStringList has a sort() method REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28135?vs=77979=78355 BRANCH l-qset-fromlist (branched from master) REVISION DETAIL

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

2020-03-24 Thread Ahmad Samir
ahmadsamir added a comment. Should be fixed by D28232 . REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27503 To: bport, #plasma, meven, crossi, ervin Cc: usta, ahmadsamir, ngraham, plasma-devel, Orage, LeGast00n,

D28232: [SpellChecking KCM] Fix the build

2020-03-24 Thread Ahmad Samir
ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, bport, meven, crossi, ervin. Herald added a project: Plasma. ahmadsamir requested review of this revision. REVISION SUMMARY QList/QSet iterator-based ctors are available since Qt 5.14, so we could make the code conditional

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

2020-03-23 Thread Ahmad Samir
ahmadsamir added a comment. This failed to build on the CI https://build.kde.org/job/Plasma/job/plasma-desktop/, I guess you'll need to bump the min required version of KF5 to 5.69.0. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27503 To: bport, #plasma,

CI mail filtering (was Re: Manner in which kde-gtk-config development is conducted)

2020-03-22 Thread Ahmad Samir
suggest that if you commit regulary to a KDE repo to make a habit of checking the results of the unit tests on the CI every now and then (jenkins has a nice web interface). Unit tests could pass locally and fail on the CI for some reason. [...] My, beginner-dev, 2 p's, -- Ahmad Samir

D28079: [keditfiletype] Prevent removing the "main" glob pattern for mime types

2020-03-21 Thread Ahmad Samir
ahmadsamir abandoned this revision. ahmadsamir added a comment. OK. REPOSITORY R126 KDE CLI Utilities REVISION DETAIL https://phabricator.kde.org/D28079 To: ahmadsamir, #plasma, dfaure, davidedmundson, apol Cc: kde-frameworks-devel, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev,

D28026: further constrict line parsing of .so files

2020-03-20 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > backtraceparsergdb.cpp:75 > if (!regExp.cap(7).isEmpty()) { //we have file information (stuff > after from|at) > bool file = regExp.cap(8) == QLatin1String("at"); //'at' means > we have a source file (likely) >

D28098: remove pointless and arbitrary 4 line frame limit

2020-03-19 Thread Ahmad Samir
ahmadsamir added a comment. This makes sense; also it fixes a part of the backtraceparsertest unit test, test_bug168000, which currently fails on master (output after export'ing QT_LOGGING_RULES="*drkonqi*=true"): 3: QDEBUG : BacktraceParserTest::btParserUsefulnessTest(test_bug168000)

D28098: remove pointless and arbitrary 4 line frame limit

2020-03-19 Thread Ahmad Samir
ahmadsamir added a comment. In D28098#629338 , @bcooksley wrote: > There should be no further changes to Dr Konqi at this time as it fails to build from source on both FreeBSD and Windows. > Please see the relevant CI jobs for more

D28027: fix line rating for new format when function name is missing

2020-03-19 Thread Ahmad Samir
ahmadsamir added a comment. In D28027#630512 , @sitter wrote: > D28026 Ah, yes, that's where those "packagekit" lines are. Thanks :) REPOSITORY R871 DrKonqi BRANCH parse-rate REVISION DETAIL

D28027: fix line rating for new format when function name is missing

2020-03-19 Thread Ahmad Samir
ahmadsamir added a comment. This doesn't apply cleanly. REPOSITORY R871 DrKonqi BRANCH parse-rate REVISION DETAIL https://phabricator.kde.org/D28027 To: sitter, cfeck, ngraham Cc: ahmadsamir, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas,

D28042: [DrKonqi] Port QRegExp to QRegularExpression

2020-03-19 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes. Closed by commit R871:8d8062fe6bdc: [DrKonqi] Port QRegExp to QRegularExpression (authored by ahmadsamir). REPOSITORY R871 DrKonqi CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28042?vs=77744=77995 REVISION

D28135: Port away from deprecated QSet/QList methods in some places

2020-03-19 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > anthonyfieroni wrote in runnermodel.cpp:179 > But toSet() returns new container m_runners and runners are unmodified. I don't mind reverting that bit, but it seems wasteful to me to throw newRunners away, it is sorted and has no

D28135: Port away from deprecated QSet/QList methods in some places

2020-03-19 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > anthonyfieroni wrote in runnermodel.cpp:179 > Here should be `m_runners = runners` to be exactly same as previous. I don't > see much benefit of having duplicate items. IIUC, the original code used toSet() to remove duplicates from both

D28135: Port away from deprecated QSet/QList methods in some places

2020-03-19 Thread Ahmad Samir
ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, davidedmundson, apol. Herald added a project: Plasma. ahmadsamir requested review of this revision. TEST PLAN make && ctest REPOSITORY R120 Plasma Workspace BRANCH l-qset-fromlist (branched from master) REVISION DETAIL

D28132: [Calculator Runner] Minor code optimisation

2020-03-18 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes. Closed by commit R120:cd37cc6926c3: [Calculator Runner] Minor code optimisation (authored by ahmadsamir). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28132?vs=77945=77957

D28132: [Calculator Runner] Minor code optimisation

2020-03-18 Thread Ahmad Samir
ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, apol. Herald added a project: Plasma. ahmadsamir requested review of this revision. REVISION SUMMARY Remove redunant QString::contains() checks. Create one QRegularExpression object and use setPattern(). TEST PLAN

D27857: Port some usage of QRegExp to QRegularExpression

2020-03-18 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes. Closed by commit R120:f444ebe595a2: Port some usage of QRegExp to QRegularExpression (authored by ahmadsamir). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE

D27857: Port some usage of QRegExp to QRegularExpression

2020-03-18 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 77906. ahmadsamir added reviewers: davidedmundson, apol, broulik. ahmadsamir added a comment. Verbatim REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27857?vs=77905=77906 BRANCH l-qregexp-QRE (branched

D27857: Port some usage of QRegExp to QRegularExpression

2020-03-18 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 77905. ahmadsamir added a comment. Verbatim REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27857?vs=77898=77905 BRANCH l-qregexp-QRE (branched from master) REVISION DETAIL

D27857: Port some usage of QRegExp to QRegularExpression

2020-03-18 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > apol wrote in server.cpp:671 > Why the changes from [0-9] to \d? Less cluttered; but maybe not as readable. I'll revert it. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D27857 To: ahmadsamir, #plasma Cc:

D28075: [kstyle] Properly unregister widgets in ShadowHelper

2020-03-18 Thread Ahmad Samir
ahmadsamir added a comment. Could be something on my system no idea what though. Sorry for the noise. Anyway, the important thing is that the test passes on the CI system. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D28075 To: zzag, #plasma, cblack Cc:

D28075: [kstyle] Properly unregister widgets in ShadowHelper

2020-03-18 Thread Ahmad Samir
ahmadsamir added a comment. FWIW, I have the kdatetimeedittest unit test from KWidgetAddons fail with a SIGSEGV even after building breeze with this diff: Thread 1 "kdatetimeeditte" received signal SIGSEGV, Segmentation fault. 0x70c6c770 in qDeleteAll::const_iterator>

D27857: Port some usage of QRegExp to QRegularExpression

2020-03-18 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 77898. ahmadsamir marked 2 inline comments as done. ahmadsamir removed reviewers: davidedmundson, apol, broulik. ahmadsamir added a comment. Address comments REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE

D27857: Port some usage of QRegExp to QRegularExpression

2020-03-18 Thread Ahmad Samir
ahmadsamir added a comment. Ping. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D27857 To: ahmadsamir, #plasma, davidedmundson, apol, broulik Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen,

D27808: [Fonts KCM] Port KFontDialog/KFontChooser to QFontDialog

2020-03-17 Thread Ahmad Samir
ahmadsamir added a comment. I spent some time looking (and hacking at a kdeplatformfontdialoghelper...) at the code in kdeplatformfiledialoghelper, KFontChooser and KFontDialog, and I am starting to think that moving KFontDialog to KWidgetAddons (where KFontChooser and KFontRequester live)

D28095: Bump required version of KF5 to 5.69.0

2020-03-17 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes. Closed by commit R871:ce461918868d: Bump required version of KF5 to 5.69.0 (authored by ahmadsamir). REPOSITORY R871 DrKonqi CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28095?vs=77812=77813 REVISION DETAIL

D28095: Bump required version of KF5 to 5.69.0

2020-03-17 Thread Ahmad Samir
ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, dfaure, apol, sitter. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ahmadsamir requested review of this revision. REVISION SUMMARY KCrash::setErrorMessage() was introduced in 5.69.0. REPOSITORY

D28079: [keditfiletype] Prevent removing the "main" glob pattern for mime types

2020-03-17 Thread Ahmad Samir
ahmadsamir added a comment. For background see: https://bugs.kde.org/show_bug.cgi?id=414742 I may be wrong, but personally I think QMimeDatabase is acting sensibly by not allowing to remove the "main" glob pattern of a mimetype. Also IIUC it seems other tools are sort of doing the

D28079: [keditfiletype] Prevent removing the "main" glob pattern for mime types

2020-03-16 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 77754. ahmadsamir added a comment. Tweak comment REPOSITORY R126 KDE CLI Utilities CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28079?vs=77753=77754 BRANCH l-mainglobpattern (branched from master) REVISION DETAIL

D28079: [keditfiletype] Prevent removing the "main" glob pattern for mime types

2020-03-16 Thread Ahmad Samir
ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, dfaure, davidedmundson, apol. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ahmadsamir requested review of this revision. REVISION SUMMARY QMimeDatabase re-adds/prepends the "main" glob pattern (the

D28042: [DrKonqi] Port QRegExp to QRegularExpression

2020-03-16 Thread Ahmad Samir
ahmadsamir added a comment. In D28042#628179 , @sitter wrote: > I wouldn't terribly mind a second pair of eyes. > > Code generally looks good to me though, so if nobody else comments feel free to land in a couple days. OK, thanks.

D28042: [DrKonqi] Port QRegExp to QRegularExpression

2020-03-16 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 77744. ahmadsamir added a comment. Fix comment grammar REPOSITORY R871 DrKonqi CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28042?vs=77606=77744 BRANCH l-QRE-port (branched from master) REVISION DETAIL

D27675: feat: avoid duplicated text when assembling user-facing output names

2020-03-16 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > utils.cpp:64 > +return append(name, QLatin1Char('(') + output->name() + > QLatin1Char(')')); > } > } If I am reading this correctly, you could keep the old code and use simplified(): const QString outName =

  1   2   >