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

ahmadsamir added a comment. In D27785#628110 , @bport wrote: > Ok for me but need to wait approval on font dialog review (to ensure correct font style is selected when editing) Thanks. The font dialog review, D27808

D28042: [DrKonqi] Port QRegExp to QRegularExpression

ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, davidedmundson, sitter, apol. Herald added a project: Plasma. ahmadsamir requested review of this revision. REVISION SUMMARY Port QRegExp::exactMatch() by using QRegularExpression::anchoredPattern(), unless the pattern is

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

ahmadsamir added inline comments. INLINE COMMENTS > apol wrote in kstart.cpp:184 > Wouldn't it be easier here to just do `windowtitle.compare(info.name(), > Qt::CaseInsensitive)` and skip the regex? I thought about that, but looking at the code I found: void KStart::sendRule() {

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

ahmadsamir updated this revision to Diff 77481. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. Fix commit message REPOSITORY R126 KDE CLI Utilities CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28000?vs=77480=77481 BRANCH l-qregexp (branched from

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

ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, apol, davidedmundson. Herald added a project: Plasma. ahmadsamir requested review of this revision. TEST PLAN make && ctest REPOSITORY R126 KDE CLI Utilities BRANCH l-qregexp (branched from master) REVISION DETAIL

D27914: [Kilpper] Port QRegExp to QRegularExpression

ahmadsamir added a comment. Ping. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D27914 To: ahmadsamir, #plasma Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf,

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

ahmadsamir updated this revision to Diff 77442. ahmadsamir retitled this revision from "[Fonts KCM] Change setNearestExistingFonts() to set the fonts only when necessary" to "[Fonts KCM] Change how nearestExistingFonts() finds a matching font". ahmadsamir edited the summary of this revision.

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

ahmadsamir added a comment. In D27808#625227 , @cfeck wrote: > We already lost fractional point sizes when porting from KFontDialog to QFontDialog. Yeah, I never understood why they opted for integer values only (I will dig in upstream

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

ahmadsamir added a comment. In D27808#625186 , @meven wrote: > In D27808#625181 , @ahmadsamir wrote: > > > And another question, "General", "Menu", "Toolbar", how many users actually set these

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

ahmadsamir added a comment. And another question, "General", "Menu", "Toolbar", how many users actually set these three to individual font families/styles/sizes? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27808 To: ahmadsamir, #plasma, davidedmundson,

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

ahmadsamir added a comment. In D27808#625052 , @meven wrote: > Porting with such feature regression is hard to justify. I honestly don't know how many users actually use that feature; if it's still widely used, then yeah, that would

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

ahmadsamir planned changes to this revision. ahmadsamir added a comment. It seems I messed up the branches stuff (or phabricator works in a way I don't understand), so I think it's best to work at one diff at a time, instead of wasting time trying to make sense of this situation :/ (I

D27785: [Fonts KCM] Change setNearestExistingFonts() to set the fonts only when necessary

ahmadsamir updated this revision to Diff 77268. ahmadsamir retitled this revision from "[Fonts KCM] Change how nearestExistingFonts() finds a matching font" to "[Fonts KCM] Change setNearestExistingFonts() to set the fonts only when necessary". ahmadsamir edited the summary of this revision.

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

ahmadsamir updated this revision to Diff 77265. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. Tweak commit message REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27785?vs=77264=77265 BRANCH arcpatch-D27785 (branched

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

ahmadsamir updated this revision to Diff 77264. ahmadsamir marked 3 inline comments as done. ahmadsamir retitled this revision from "[Fonts KCM] Change setNearestExistingFonts() to set the fonts only when necessary" to "[Fonts KCM] Change how nearestExistingFonts() finds a matching font".

D27785: [Fonts KCM] Change setNearestExistingFonts() to set the fonts only when necessary

ahmadsamir marked 3 inline comments as done. ahmadsamir added inline comments. INLINE COMMENTS > bport wrote in fonts.cpp:115 > You can use && to have only one if there OK. > bport wrote in fonts.cpp:552 > Those change will compare font twice, here and on setters so I will keep old > code

D27785: [Fonts KCM] Change setNearestExistingFonts() to set the fonts only when necessary

ahmadsamir updated this revision to Diff 77258. ahmadsamir edited the summary of this revision. ahmadsamir edited the test plan for this revision. ahmadsamir removed a reviewer: bport. ahmadsamir added a comment. More detailed commit message REPOSITORY R119 Plasma Desktop CHANGES SINCE

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

ahmadsamir added a comment. In D27808#624506 , @bport wrote: > With your change, we are not allowed anymore to change for example only font and keeping different size, style for each kind of font (window, menu, toolbar...), right ?

D27914: [Kilpper] Port QRegExp to QRegularExpression

ahmadsamir created this revision. ahmadsamir added a reviewer: Plasma. Herald added a project: Plasma. ahmadsamir requested review of this revision. TEST PLAN make && ctest klipper still seems to work REPOSITORY R120 Plasma Workspace BRANCH l-klipper (branched from master) REVISION

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

ahmadsamir added a comment. FTR, I had submitted this https://codereview.qt-project.org/c/qt/qtbase/+/292695 REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27830 To: ahmadsamir, #plasma, davidedmundson, broulik, ervin, meven, bport Cc: plasma-devel, Orage,

D27857: Port some usage of QRegExp to QRegularExpression

ahmadsamir created this revision. ahmadsamir added a reviewer: Plasma. Herald added a project: Plasma. ahmadsamir requested review of this revision. REVISION SUMMARY [0-9] is \d TEST PLAN make && ctest REPOSITORY R120 Plasma Workspace BRANCH l-qregexp-QRE (branched from master)

D27785: [Fonts KCM] Change setNearestExistingFonts() to set the fonts only when necessary

ahmadsamir added a comment. FTR, removing setNeedsSave(false) from load(), it seems to work sometimes, and then some other times it doesn't work (i.e. changing anything, Apply doesn't get enabled)... REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27785 To:

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

ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, davidedmundson, broulik, ervin, meven. Herald added a project: Plasma. ahmadsamir requested review of this revision. REVISION SUMMARY KConfig will clear the styleName font property for "Regular"-like font styles (so that

D27785: [Fonts KCM] Change setNearestExistingFonts() to set the fonts only when necessary

ahmadsamir updated this revision to Diff 76868. ahmadsamir added reviewers: broulik, ervin, meven. ahmadsamir added a comment. Tweak REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27785?vs=76779=76868 BRANCH l-fonts-kcm-nearest (branched from

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

ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, davidedmundson, broulik, ervin, meven. Herald added a project: Plasma. ahmadsamir requested review of this revision. REVISION SUMMARY When using ajustAllFonts(), try to set the fixed width/Monospace font from the font

D27785: [Fonts KCM] Change setNearestExistingFonts() to set the fonts only when necessary

ahmadsamir added a comment. TODO: fix the font dialog to select the correct "Regular"-like style (I have a working patch but needs more work). REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27785 To: ahmadsamir, #plasma, davidedmundson Cc: plasma-devel,

D27785: [Fonts KCM] Change setNearestExistingFonts() to set the fonts only when necessary

ahmadsamir updated this revision to Diff 76779. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. Remove setNeedsSave(false) from load(), doesn't seem to be needed any more REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE

D27785: [Fonts KCM] Change setNearestExistingFonts() to set the fonts only when necessary

ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, davidedmundson. Herald added a project: Plasma. ahmadsamir requested review of this revision. REVISION SUMMARY A "Regular" font will have an empty styleName e.g. "DejaVu Sans,12,-1,5,50,0,0,0,0,0" so that setBold(true) can

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

This revision was automatically updated to reflect the committed changes. Closed by commit R124:0be65944f311: If KHelpCenter isnt available fallback to opening doc at docs.kde.org (authored by ahmadsamir). REPOSITORY R124 System Settings CHANGES SINCE LAST UPDATE

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

ahmadsamir updated this revision to Diff 76447. ahmadsamir added a comment. target_link_libraries() KGuiAddons in the sysetmsettings5 target REPOSITORY R124 System Settings CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27518?vs=76355=76447 BRANCH l-khelp (branched from

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

ahmadsamir updated this revision to Diff 76355. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. target_link_libraries() KGuiAddons REPOSITORY R124 System Settings CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27518?vs=76266=76355 BRANCH l-khelp

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

ahmadsamir added a comment. In D27518#617038 , @apol wrote: > Makes sense. Have you tested that this works? You should probably pass KF5::GuiAddons to the target_link_libraries as well. I tested a good number of the KCMs, and it seems to

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

ahmadsamir updated this revision to Diff 76266. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. find_package() KGuiAddons REPOSITORY R124 System Settings CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27518?vs=76265=76266 BRANCH l-khelp (branched

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

ahmadsamir added a comment. I wonder how to make systemsettings depend on KGuiAddons (if that's needed here)? REPOSITORY R124 System Settings REVISION DETAIL https://phabricator.kde.org/D27518 To: ahmadsamir, #plasma, apol Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack,

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

ahmadsamir updated this revision to Diff 76265. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. Change BUG: REPOSITORY R124 System Settings CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27518?vs=76259=76265 BRANCH l-khelp (branched from master)

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

ahmadsamir updated this revision to Diff 76259. ahmadsamir retitled this revision from "Only enable the "Help" button if khelpcenter is available" to "If KHelpCenter isn't available fallback to opening doc at docs.kde.org". ahmadsamir edited the summary of this revision. ahmadsamir removed a

D27518: Only enable the "Help" button if khelpcenter is available

ahmadsamir added a comment. openHelp() will need to be changed, because the "application=" part of the url will be "systemsettings" (QCoreApplication::applicationName()), which would open the handbook of systemsettings instead of e.g. /kcontrol/fonts/index.html. Is the API for the

D27518: Only enable the "Help" button if khelpcenter is available

ahmadsamir added a comment. You mean open the local .docbook with kio_help to get an HTML then open it in a browser? or opening docs.kde.org e.g. https://docs.kde.org/stable5/en/kde-workspace/kcontrol/fonts/index.html ? REPOSITORY R124 System Settings REVISION DETAIL

D27518: Only enable the "Help" button if khelpcenter is available

ahmadsamir created this revision. ahmadsamir added a reviewer: Plasma. Herald added a project: Plasma. ahmadsamir requested review of this revision. REVISION SUMMARY Use a static const bool to check if khelpcenter is available on the system, so as not to check every time a module is opened.

D26902: Fix systemsettings5 crash if kscreenlocker shortcut is empty

ahmadsamir abandoned this revision. ahmadsamir added a comment. OK. REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D26902 To: ahmadsamir, #plasma, davidedmundson, mart Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus,

D26902: Fix systemsettings5 crash if kscreenlocker shortcut is empty

ahmadsamir created this revision. ahmadsamir added reviewers: Plasma, davidedmundson, mart. Herald added a project: Plasma. ahmadsamir requested review of this revision. TEST PLAN - Make the kscreenlocker shortcut empty - Try opening systemsettings -> workspace behaviour, it crashes - Apply

D24615: Fix crash with sortAllByName/Description

This revision was automatically updated to reflect the committed changes. Closed by commit R103:3d4640bea622: Fix crash with sortAllByName/Description (authored by ahmadsamir). REPOSITORY R103 KMenu Editor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24615?vs=67855=67891 REVISION

D24615: Fix crash with sortAllByName/Description

ahmadsamir created this revision. ahmadsamir added a reviewer: mlaurent. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ahmadsamir requested review of this revision. REVISION SUMMARY Calling QTreeWidgetItem::takeChildren() on invisibleRootItem() results in a crash

D24603: Port away from deprecated KDELibs4Support

This revision was automatically updated to reflect the committed changes. Closed by commit R103:e3b8910670e3: Port away from deprecated KDELibs4Support (authored by ahmadsamir). REPOSITORY R103 KMenu Editor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24603?vs=67820=67831

D24603: Port away from deprecated KDELibs4Support

ahmadsamir added a comment. F7582001: kmenuedit-vs-dolphin-settings-dialog.png Looking at kmenuedit and dolphin settings dialog, the margin and spacing look comparable, so those setContentMargins and setSpacing calls aren't needed any more.

D24603: Port away from deprecated KDELibs4Support

ahmadsamir created this revision. ahmadsamir added a reviewer: mlaurent. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ahmadsamir requested review of this revision. REVISION SUMMARY kmenuedit seems to work, adding new submenus and items REPOSITORY R103 KMenu Editor

<    1   2