Re: Review Request 113858: Add .reviewboardrc
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113858/#review44661 --- This review has been submitted with commit 9a7d7e0c8afea1dfcad8e235103c22b83daf7671 by Kevin Funk to branch master. - Commit Hook On Nov. 14, 2013, 1:22 p.m., Kevin Funk wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113858/ --- (Updated Nov. 14, 2013, 1:22 p.m.) Review request for kdelibs. Repository: kdelibs Description --- Add .reviewboardrc Diffs - .reviewboardrc PRE-CREATION Diff: http://git.reviewboard.kde.org/r/113858/diff/ Testing --- Thanks, Kevin Funk
Re: Review Request 113858: Add .reviewboardrc
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113858/ --- (Updated Nov. 28, 2013, 9:35 a.m.) Status -- This change has been marked as submitted. Review request for kdelibs. Repository: kdelibs Description --- Add .reviewboardrc Diffs - .reviewboardrc PRE-CREATION Diff: http://git.reviewboard.kde.org/r/113858/diff/ Testing --- Thanks, Kevin Funk
Re: Review Request 114105: kcm proxy: fix for noProxyFor setting when 'system proxy' type
On Nov. 28, 2013, 5:17 a.m., Dawit Alemayehu wrote: konqueror/settings/kio/kproxydlg.cpp, line 295 http://git.reviewboard.kde.org/r/114105/diff/1/?file=219633#file219633line295 nitpick: no need for KSaveIOConfig here. ?. Not sure what do you mean On Nov. 28, 2013, 5:17 a.m., Dawit Alemayehu wrote: konqueror/settings/kio/kproxydlg.cpp, line 444 http://git.reviewboard.kde.org/r/114105/diff/1/?file=219633#file219633line444 This does not make sense. I explicitly check show me the value and you uncheck it as a result of me clicking on the Auto Detect button? No. This is my (user) point of view: if i'm configuring system proxy and i click Auto Detect button i'm interested to know/see what env vars i'm actually going to use, not really their values. If i'm a curious user i could ask the gui to show me their *actual* values (those values can also not be the same later on, for example because i re-assigned the env vars being configured). This change is also a fast and simple fix for the following use case: suppose i have set the following env vars on my system $export my_proxy=http://localhost: $export http_proxy=http://localhost: and i have configured system proxy to use my_proxy (in kioslaverc httpProxy=my_proxy). I open proxy config module and check the Show the value of the environment variables checkbox, http://localhost:; is showed in HTTP Proxy:, then i push Auto Detect, HTTP Proxy: changes to http://localhost:;, i save and exit: nothing is really saved because of the logic adopted in KProxyDialog::save(). - Andrea --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114105/#review44646 --- On Nov. 27, 2013, 9:22 p.m., Andrea Iacovitti wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114105/ --- (Updated Nov. 27, 2013, 9:22 p.m.) Review request for KDE Base Apps and Dawit Alemayehu. Repository: kde-baseapps Description --- In case of 'system proxy' proxyType, NoProxyFor config key holds the name of the env variable (e.g. no_proxy) and not its value. Because of this KProtocolManager::noProxyFor() can not be used to get NoProxyFor config setting in KProxyDialog::load(), as it returns the resolved value of the environment variable and not its name: i added helper method KSaveIOConfig::noProxyFor() to read that value directly from config file. Also make sure to uncheck showEnvValueCheckBox before filling proxy edit fields with environment variable names in KProxyDialog::on_autoDetectButton_clicked(). Diffs - konqueror/settings/kio/kproxydlg.cpp e80afeb konqueror/settings/kio/ksaveioconfig.h 2318198 konqueror/settings/kio/ksaveioconfig.cpp c822f7b Diff: http://git.reviewboard.kde.org/r/114105/diff/ Testing --- To reproduce the issue: $ export no_proxy=kde.org $ kcmshell4 proxy choose Use system proxy configuration, push Auto Detect button, close the gui interface and reopen it: $ kcmshell4 proxy see how Exceptions fields contains kde.org and not no_proxy Thanks, Andrea Iacovitti
Re: Review Request 111342: make kinfocenter compile on non x11 systems and Windows
On July 1, 2013, 3:27 p.m., Martin Gräßlin wrote: from my side it looks OK, though I won't give a ship-it. This is a decision to the kinfocenter developers whether they are fine with the ifdefs. we'd like to include it in the next release and since the last message in this review request is 4 months old, I'd like to ping it again... - Nico --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111342/#review35374 --- On July 1, 2013, 5:17 p.m., Patrick von Reth wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111342/ --- (Updated July 1, 2013, 5:17 p.m.) Review request for kde-workspace, KInfoCenter and kwin. Repository: kde-workspace Description --- make kinfocenter compile on non x11 systems and Windows Kinfocenter is quite useful to test a solid backend Diffs - CMakeLists.txt 57cd82c56539b93fafe7866a259c155eebcc86a0 kinfocenter/Modules/CMakeLists.txt 0a87eb48d97df2e0224819225ba0af6bf0d93f39 kinfocenter/Modules/base/os_base.h f09202d9d0c592238735dc1b2d5041a921358adb kinfocenter/Modules/devinfo/soldevicetypes.cpp d3387d972b14368e9fa2b5ad1f97d5210d2beb01 Diff: http://git.reviewboard.kde.org/r/111342/diff/ Testing --- windows Thanks, Patrick von Reth
Re: Review Request 111342: make kinfocenter compile on non x11 systems and Windows
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111342/#review44671 --- Ship it! Besides the i18n misssing, sure, ship it, i don't see any huge problem (not kinfoncenter myself but not sure we have one) kinfocenter/Modules/devinfo/soldevicetypes.cpp http://git.reviewboard.kde.org/r/111342/#comment31894 i18n missing - Albert Astals Cid On July 1, 2013, 5:17 p.m., Patrick von Reth wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111342/ --- (Updated July 1, 2013, 5:17 p.m.) Review request for kde-workspace, KInfoCenter and kwin. Repository: kde-workspace Description --- make kinfocenter compile on non x11 systems and Windows Kinfocenter is quite useful to test a solid backend Diffs - CMakeLists.txt 57cd82c56539b93fafe7866a259c155eebcc86a0 kinfocenter/Modules/CMakeLists.txt 0a87eb48d97df2e0224819225ba0af6bf0d93f39 kinfocenter/Modules/base/os_base.h f09202d9d0c592238735dc1b2d5041a921358adb kinfocenter/Modules/devinfo/soldevicetypes.cpp d3387d972b14368e9fa2b5ad1f97d5210d2beb01 Diff: http://git.reviewboard.kde.org/r/111342/diff/ Testing --- windows Thanks, Patrick von Reth
Re: Review Request 114105: kcm proxy: fix for noProxyFor setting when 'system proxy' type
On Nov. 28, 2013, 5:17 a.m., Dawit Alemayehu wrote: konqueror/settings/kio/kproxydlg.cpp, line 295 http://git.reviewboard.kde.org/r/114105/diff/1/?file=219633#file219633line295 nitpick: no need for KSaveIOConfig here. Andrea Iacovitti wrote: ?. Not sure what do you mean Disregard this. I thought the function was being called from KSaveIOConfig itself. My mistake. - Dawit --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114105/#review44646 --- On Nov. 27, 2013, 9:22 p.m., Andrea Iacovitti wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114105/ --- (Updated Nov. 27, 2013, 9:22 p.m.) Review request for KDE Base Apps and Dawit Alemayehu. Repository: kde-baseapps Description --- In case of 'system proxy' proxyType, NoProxyFor config key holds the name of the env variable (e.g. no_proxy) and not its value. Because of this KProtocolManager::noProxyFor() can not be used to get NoProxyFor config setting in KProxyDialog::load(), as it returns the resolved value of the environment variable and not its name: i added helper method KSaveIOConfig::noProxyFor() to read that value directly from config file. Also make sure to uncheck showEnvValueCheckBox before filling proxy edit fields with environment variable names in KProxyDialog::on_autoDetectButton_clicked(). Diffs - konqueror/settings/kio/kproxydlg.cpp e80afeb konqueror/settings/kio/ksaveioconfig.h 2318198 konqueror/settings/kio/ksaveioconfig.cpp c822f7b Diff: http://git.reviewboard.kde.org/r/114105/diff/ Testing --- To reproduce the issue: $ export no_proxy=kde.org $ kcmshell4 proxy choose Use system proxy configuration, push Auto Detect button, close the gui interface and reopen it: $ kcmshell4 proxy see how Exceptions fields contains kde.org and not no_proxy Thanks, Andrea Iacovitti
Re: Review Request 114105: kcm proxy: fix for noProxyFor setting when 'system proxy' type
On Nov. 28, 2013, 5:17 a.m., Dawit Alemayehu wrote: konqueror/settings/kio/kproxydlg.cpp, line 444 http://git.reviewboard.kde.org/r/114105/diff/1/?file=219633#file219633line444 This does not make sense. I explicitly check show me the value and you uncheck it as a result of me clicking on the Auto Detect button? No. Andrea Iacovitti wrote: This is my (user) point of view: if i'm configuring system proxy and i click Auto Detect button i'm interested to know/see what env vars i'm actually going to use, not really their values. If i'm a curious user i could ask the gui to show me their *actual* values (those values can also not be the same later on, for example because i re-assigned the env vars being configured). This change is also a fast and simple fix for the following use case: suppose i have set the following env vars on my system $export my_proxy=http://localhost: $export http_proxy=http://localhost: and i have configured system proxy to use my_proxy (in kioslaverc httpProxy=my_proxy). I open proxy config module and check the Show the value of the environment variables checkbox, http://localhost:; is showed in HTTP Proxy:, then i push Auto Detect, HTTP Proxy: changes to http://localhost:;, i save and exit: nothing is really saved because of the logic adopted in KProxyDialog::save(). Well when a user explicitly checks an option, which by default is not checked, it should be not reverted as a result of another action. I as a user do no want to see that. After all I purposefully checked that option to begin with, which can only mean one thing. I actually wanted to see the values and not the *actual* proxy variables. Otherwise, I would not have checked it to begin with. So I do not agree with this particular change and would like to see it removed from this patch. Having said that the logic for the auto detection button needs to be fixed to ensure that it shows the proper text in those line edits. Currently it does not and that is what breaks KProxyDialog::save(), but that is easy enough to fix and something that needs to be absent this patch. I will deal with that after the rest of this patch is committed. - Dawit --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114105/#review44646 --- On Nov. 27, 2013, 9:22 p.m., Andrea Iacovitti wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114105/ --- (Updated Nov. 27, 2013, 9:22 p.m.) Review request for KDE Base Apps and Dawit Alemayehu. Repository: kde-baseapps Description --- In case of 'system proxy' proxyType, NoProxyFor config key holds the name of the env variable (e.g. no_proxy) and not its value. Because of this KProtocolManager::noProxyFor() can not be used to get NoProxyFor config setting in KProxyDialog::load(), as it returns the resolved value of the environment variable and not its name: i added helper method KSaveIOConfig::noProxyFor() to read that value directly from config file. Also make sure to uncheck showEnvValueCheckBox before filling proxy edit fields with environment variable names in KProxyDialog::on_autoDetectButton_clicked(). Diffs - konqueror/settings/kio/kproxydlg.cpp e80afeb konqueror/settings/kio/ksaveioconfig.h 2318198 konqueror/settings/kio/ksaveioconfig.cpp c822f7b Diff: http://git.reviewboard.kde.org/r/114105/diff/ Testing --- To reproduce the issue: $ export no_proxy=kde.org $ kcmshell4 proxy choose Use system proxy configuration, push Auto Detect button, close the gui interface and reopen it: $ kcmshell4 proxy see how Exceptions fields contains kde.org and not no_proxy Thanks, Andrea Iacovitti
Re: Review Request 114105: kcm proxy: fix for noProxyFor setting when 'system proxy' type
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114105/#review44690 --- Ship it! Minus this change = mUi.showEnvValueCheckBox-setChecked(false); - Dawit Alemayehu On Nov. 27, 2013, 9:22 p.m., Andrea Iacovitti wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114105/ --- (Updated Nov. 27, 2013, 9:22 p.m.) Review request for KDE Base Apps and Dawit Alemayehu. Repository: kde-baseapps Description --- In case of 'system proxy' proxyType, NoProxyFor config key holds the name of the env variable (e.g. no_proxy) and not its value. Because of this KProtocolManager::noProxyFor() can not be used to get NoProxyFor config setting in KProxyDialog::load(), as it returns the resolved value of the environment variable and not its name: i added helper method KSaveIOConfig::noProxyFor() to read that value directly from config file. Also make sure to uncheck showEnvValueCheckBox before filling proxy edit fields with environment variable names in KProxyDialog::on_autoDetectButton_clicked(). Diffs - konqueror/settings/kio/kproxydlg.cpp e80afeb konqueror/settings/kio/ksaveioconfig.h 2318198 konqueror/settings/kio/ksaveioconfig.cpp c822f7b Diff: http://git.reviewboard.kde.org/r/114105/diff/ Testing --- To reproduce the issue: $ export no_proxy=kde.org $ kcmshell4 proxy choose Use system proxy configuration, push Auto Detect button, close the gui interface and reopen it: $ kcmshell4 proxy see how Exceptions fields contains kde.org and not no_proxy Thanks, Andrea Iacovitti
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 21, 2013, 11:22 a.m., Kevin Ottens wrote: To get in this patch would benefit from being based on the frameworks branch and go into kdeclarative. Kevin Ottens wrote: Any chance for an update? Denis Kuplyakov wrote: Yes I will finish it, when have time. There are many pre-exams in university. Kevin Ottens wrote: Any news? we need to get in or discard all the old patches now. Last warning before getting discarded. Patches will soon not be accepted in kdelibs/frameworks in preparation of the repository split. - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review42069 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: kdelibs Description --- It is wrapper to access KColorScheme's methods from QML code. Also added Q_GADGET to KColorScheme to enable Q_ENUMS using, to make them accessible from QML code. As it will be accepted, QML-clone of KgPopupItem will be posted for review to libkdegames, as it uses it to access KDE's color theme. More info: * search for KDE theme colors API for QML thread at kdelibs and kdegames mailinglists * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION kdeui/colors/kcolorscheme.h 17570fd kdeui/colors/kcolorscheme.cpp a6650ac kdeui/colors/kcolorschemetoken.h PRE-CREATION kdeui/colors/kcolorschemetoken.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/112880/diff/ Testing --- I've tested it with KReversi's deniskup/gsoc2013/newdesign branch. Thanks, Denis Kuplyakov
Re: Review Request 114105: kcm proxy: fix for noProxyFor setting when 'system proxy' type
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114105/#review44709 --- This review has been submitted with commit f1699adb2aab963dc9d0841a8f9b2ea5a10c7dcd by Andrea Iacovitti to branch KDE/4.11. - Commit Hook On Nov. 27, 2013, 9:22 p.m., Andrea Iacovitti wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114105/ --- (Updated Nov. 27, 2013, 9:22 p.m.) Review request for KDE Base Apps and Dawit Alemayehu. Repository: kde-baseapps Description --- In case of 'system proxy' proxyType, NoProxyFor config key holds the name of the env variable (e.g. no_proxy) and not its value. Because of this KProtocolManager::noProxyFor() can not be used to get NoProxyFor config setting in KProxyDialog::load(), as it returns the resolved value of the environment variable and not its name: i added helper method KSaveIOConfig::noProxyFor() to read that value directly from config file. Also make sure to uncheck showEnvValueCheckBox before filling proxy edit fields with environment variable names in KProxyDialog::on_autoDetectButton_clicked(). Diffs - konqueror/settings/kio/kproxydlg.cpp e80afeb konqueror/settings/kio/ksaveioconfig.h 2318198 konqueror/settings/kio/ksaveioconfig.cpp c822f7b Diff: http://git.reviewboard.kde.org/r/114105/diff/ Testing --- To reproduce the issue: $ export no_proxy=kde.org $ kcmshell4 proxy choose Use system proxy configuration, push Auto Detect button, close the gui interface and reopen it: $ kcmshell4 proxy see how Exceptions fields contains kde.org and not no_proxy Thanks, Andrea Iacovitti
Re: Review Request 114105: kcm proxy: fix for noProxyFor setting when 'system proxy' type
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114105/ --- (Updated Nov. 28, 2013, 4:16 p.m.) Status -- This change has been marked as submitted. Review request for KDE Base Apps and Dawit Alemayehu. Repository: kde-baseapps Description --- In case of 'system proxy' proxyType, NoProxyFor config key holds the name of the env variable (e.g. no_proxy) and not its value. Because of this KProtocolManager::noProxyFor() can not be used to get NoProxyFor config setting in KProxyDialog::load(), as it returns the resolved value of the environment variable and not its name: i added helper method KSaveIOConfig::noProxyFor() to read that value directly from config file. Also make sure to uncheck showEnvValueCheckBox before filling proxy edit fields with environment variable names in KProxyDialog::on_autoDetectButton_clicked(). Diffs - konqueror/settings/kio/kproxydlg.cpp e80afeb konqueror/settings/kio/ksaveioconfig.h 2318198 konqueror/settings/kio/ksaveioconfig.cpp c822f7b Diff: http://git.reviewboard.kde.org/r/114105/diff/ Testing --- To reproduce the issue: $ export no_proxy=kde.org $ kcmshell4 proxy choose Use system proxy configuration, push Auto Detect button, close the gui interface and reopen it: $ kcmshell4 proxy see how Exceptions fields contains kde.org and not no_proxy Thanks, Andrea Iacovitti
Re: Review Request 112463: Port SMB kioslave to KF5/Qt5
On Nov. 26, 2013, 5:12 p.m., Kevin Ottens wrote: It's been stalled for almost three months now, any chance to see progress or should it be discarded? Mark Gaiser wrote: No, it should most certainly not be disgarded. It was even working when i posted this post up for review. Dawit, how are you doing in the SMB improvements that you had in mind? Dawit Alemayehu wrote: Sorry, I forgot to inform you. I have already merged my changes into 4.12 and master branches. Awesome! Then i will update this review asap with an up to date patch. - Mark --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112463/#review44515 --- On Sept. 2, 2013, 7:16 p.m., Mark Gaiser wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112463/ --- (Updated Sept. 2, 2013, 7:16 p.m.) Review request for KDE Runtime and KDE Frameworks. Repository: kde-runtime Description --- This is the initial port! I added two TODO lines in the diff for parts where i'm not sure if I've ported them correctly. Also, i needed a change in FindSamba.cmake to even get the samba detection working. That reviewrequest is waiting here: https://git.reviewboard.kde.org/r/112448/ you're probably OK if you still use samba 3.x Once i know that this is actually working then i will comment some qDebug lines. Diffs - kioslave/CMakeLists.txt ff66ab6 kioslave/smb/CMakeLists.txt a3a2265 kioslave/smb/kio_smb.h 55efb44 kioslave/smb/kio_smb.cpp 2c2523a kioslave/smb/kio_smb_auth.cpp 4d236b4 kioslave/smb/kio_smb_browse.cpp fec6449 kioslave/smb/kio_smb_config.cpp 81ce29c kioslave/smb/kio_smb_dir.cpp 5573266 kioslave/smb/kio_smb_file.cpp 827a519 kioslave/smb/kio_smb_internal.h b895b81 kioslave/smb/kio_smb_internal.cpp 3c35583 kioslave/smb/kio_smb_mount.cpp a5a7e8e Diff: http://git.reviewboard.kde.org/r/112463/diff/ Testing --- It compiles and gets loaded just fine. I tried testing this on an actual samba share, but i kept getting a 111 error (connection refused) from kio_smb so i'm hoping that is a local issue here. If someone else could try this out and verify that it's either working or broken. Thanks, Mark Gaiser
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 21, 2013, 11:22 a.m., Kevin Ottens wrote: To get in this patch would benefit from being based on the frameworks branch and go into kdeclarative. Kevin Ottens wrote: Any chance for an update? Denis Kuplyakov wrote: Yes I will finish it, when have time. There are many pre-exams in university. Kevin Ottens wrote: Any news? we need to get in or discard all the old patches now. Kevin Ottens wrote: Last warning before getting discarded. Patches will soon not be accepted in kdelibs/frameworks in preparation of the repository split. Sorry, but I'm still very busy. What is deadline, and can I submit changes after repo-split? - Denis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review42069 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: kdelibs Description --- It is wrapper to access KColorScheme's methods from QML code. Also added Q_GADGET to KColorScheme to enable Q_ENUMS using, to make them accessible from QML code. As it will be accepted, QML-clone of KgPopupItem will be posted for review to libkdegames, as it uses it to access KDE's color theme. More info: * search for KDE theme colors API for QML thread at kdelibs and kdegames mailinglists * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION kdeui/colors/kcolorscheme.h 17570fd kdeui/colors/kcolorscheme.cpp a6650ac kdeui/colors/kcolorschemetoken.h PRE-CREATION kdeui/colors/kcolorschemetoken.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/112880/diff/ Testing --- I've tested it with KReversi's deniskup/gsoc2013/newdesign branch. Thanks, Denis Kuplyakov
Re: Review Request 111342: make kinfocenter compile on non x11 systems and Windows
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111342/#review44724 --- Hi, I maintain KInfoCenter :) Also agree on the shipping it, somehow I missed this when it was submitted :( - David Stephen Hubner On July 1, 2013, 5:17 p.m., Patrick von Reth wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111342/ --- (Updated July 1, 2013, 5:17 p.m.) Review request for kde-workspace, KInfoCenter and kwin. Repository: kde-workspace Description --- make kinfocenter compile on non x11 systems and Windows Kinfocenter is quite useful to test a solid backend Diffs - CMakeLists.txt 57cd82c56539b93fafe7866a259c155eebcc86a0 kinfocenter/Modules/CMakeLists.txt 0a87eb48d97df2e0224819225ba0af6bf0d93f39 kinfocenter/Modules/base/os_base.h f09202d9d0c592238735dc1b2d5041a921358adb kinfocenter/Modules/devinfo/soldevicetypes.cpp d3387d972b14368e9fa2b5ad1f97d5210d2beb01 Diff: http://git.reviewboard.kde.org/r/111342/diff/ Testing --- windows Thanks, Patrick von Reth
Re: Review Request 111851: Split KCModules into different Categories in KHelpcenter's navigation tree
On Nov. 22, 2013, 7:05 a.m., Ben Cooksley wrote: khelpcenter/plugintraverser.cpp, line 95 http://git.reviewboard.kde.org/r/111851/diff/1/?file=176031#file176031line95 Do you intend on modifying the control modules used by Dolphin and Konqueror here to fit this? That sounds like something which will cause unintended breakages. Perhaps it might be wise to check their respective code to determine how they look up the modules they want to load? As far as I understand the code, neither Dolphin nor Konqueror use X-KDE-ParentApp. In dolphinsettingsdialog.cpp the pages are manually added to the configuration dialog In konqmainwindow.cpp the pages are added e.g. via addModule(KCModuleInfo(QString(fmModules[i])+.desktop),fileManagementGroup); where fmModules is a hardcoded list of desktop file names On Nov. 22, 2013, 7:05 a.m., Ben Cooksley wrote: kcmshell/main.cpp, line 63 http://git.reviewboard.kde.org/r/111851/diff/1/?file=176023#file176023line63 System Settings doesn't look for control modules via X-KDE-ParentApp. It uses the following trader query instead: [X-KDE-System-Settings-Parent-Category] != '' The list of modules you get from kcmshell4 --list is different from the list of modules in systemsettings. Wirh or without the patch you get the same output from kcmshell4 --list - Burkhard --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111851/#review44184 --- On Nov. 21, 2013, 11:50 a.m., Burkhard Lück wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111851/ --- (Updated Nov. 21, 2013, 11:50 a.m.) Review request for Documentation, KDE Runtime, Albert Astals Cid, Ben Cooksley, and David Faure. Bugs: 262935 http://bugs.kde.org/show_bug.cgi?id=262935 Repository: kde-runtime Description --- The KHelpcenter navigation tree has a top level item Control Center Modules, an unsorted list of all KCModules (80 for a full kde main modules install from stable). This makes this item hardly usable, see https://bugs.kde.org/show_bug.cgi?id=262935 This patch implements: A) Alphabetical sorting for Control Center Modules/Foo Settings Modules + KInfoCenter items B) New / changed top level categories in the navigation tree: 1) System Settings Modules - replaces old Control Center Modules items: see http://docs.kde.org/stable/en/kde-workspace/systemsettings/general.html 2) Konqueror Settings Modules (see Konqueror settings dialog) items: General, Performance, Bookmarks 3) Filemanager Settings Modules (see Konqueror/Dolphin settings dialog) items: File Management, View Modes, Navigation, Services, General, Trash No File Associations, because it is already in System Settings Modules 4) Browser Settings Modules (see Konqueror settings dialog) items: Web Browsing, Proxy, Appearance, AdBlocK Filters, Web Shortcuts, Cache, History, Cookies, Browser Identification, Java JavaScript, Plugins 5) Other Settings Modules all other items like e.g. CGI Scripts from kde-runtime To make full use of these new/changed categories some kcm desktop files in other modules than kde-runtime need a change of X-KDE-ParentApp from kcontrol to konquerorcontrol, browsercontrol, filemanagercontrol or othercontrol, but that is not part of this review. As long as not all desktop files are fixed according to this patch or a necessary change in a desktop files is overlooked that KCM will be in System Settings Modules like now, but in sorted order. C) Change wording from Control Center Modules to System Settings Modules and using Foo Settings Modules for the new categories. Control Center is from KDE 3, we use System Settings nearly all over GUI and in the whole documentation. Diffs - kcmshell/main.cpp dab8fc6 khelpcenter/navigator.cpp 7460cc8 khelpcenter/plugins/CMakeLists.txt d09b869 khelpcenter/plugins/browsercontrolmodules.desktop e69de29 khelpcenter/plugins/filemanagercontrolmodules.desktop e69de29 khelpcenter/plugins/kcontrolmodules.desktop 1813df3 khelpcenter/plugins/konquerorcontrolmodules.desktop e69de29 khelpcenter/plugins/othercontrolmodules.desktop e69de29 khelpcenter/plugintraverser.cpp b0b0e78 kioslave/cgi/kcmcgi/kcmcgi.desktop f49eeb9 Diff: http://git.reviewboard.kde.org/r/111851/diff/ Testing --- Checked with one example each for konquerorcontrol, browsercontrol, filemanagercontrol and othercontrol, see attached screenshot. File Attachments
Re: Review Request 111851: Split KCModules into different Categories in KHelpcenter's navigation tree
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111851/ --- (Updated Nov. 28, 2013, 8:36 p.m.) Review request for Documentation, KDE Runtime, Albert Astals Cid, Ben Cooksley, David Faure, and Frank Reininghaus. Bugs: 262935 http://bugs.kde.org/show_bug.cgi?id=262935 Repository: kde-runtime Description --- The KHelpcenter navigation tree has a top level item Control Center Modules, an unsorted list of all KCModules (80 for a full kde main modules install from stable). This makes this item hardly usable, see https://bugs.kde.org/show_bug.cgi?id=262935 This patch implements: A) Alphabetical sorting for Control Center Modules/Foo Settings Modules + KInfoCenter items B) New / changed top level categories in the navigation tree: 1) System Settings Modules - replaces old Control Center Modules items: see http://docs.kde.org/stable/en/kde-workspace/systemsettings/general.html 2) Konqueror Settings Modules (see Konqueror settings dialog) items: General, Performance, Bookmarks 3) Filemanager Settings Modules (see Konqueror/Dolphin settings dialog) items: File Management, View Modes, Navigation, Services, General, Trash No File Associations, because it is already in System Settings Modules 4) Browser Settings Modules (see Konqueror settings dialog) items: Web Browsing, Proxy, Appearance, AdBlocK Filters, Web Shortcuts, Cache, History, Cookies, Browser Identification, Java JavaScript, Plugins 5) Other Settings Modules all other items like e.g. CGI Scripts from kde-runtime To make full use of these new/changed categories some kcm desktop files in other modules than kde-runtime need a change of X-KDE-ParentApp from kcontrol to konquerorcontrol, browsercontrol, filemanagercontrol or othercontrol, but that is not part of this review. As long as not all desktop files are fixed according to this patch or a necessary change in a desktop files is overlooked that KCM will be in System Settings Modules like now, but in sorted order. C) Change wording from Control Center Modules to System Settings Modules and using Foo Settings Modules for the new categories. Control Center is from KDE 3, we use System Settings nearly all over GUI and in the whole documentation. Diffs - kcmshell/main.cpp dab8fc6 khelpcenter/navigator.cpp 7460cc8 khelpcenter/plugins/CMakeLists.txt d09b869 khelpcenter/plugins/browsercontrolmodules.desktop e69de29 khelpcenter/plugins/filemanagercontrolmodules.desktop e69de29 khelpcenter/plugins/kcontrolmodules.desktop 1813df3 khelpcenter/plugins/konquerorcontrolmodules.desktop e69de29 khelpcenter/plugins/othercontrolmodules.desktop e69de29 khelpcenter/plugintraverser.cpp b0b0e78 kioslave/cgi/kcmcgi/kcmcgi.desktop f49eeb9 Diff: http://git.reviewboard.kde.org/r/111851/diff/ Testing --- Checked with one example each for konquerorcontrol, browsercontrol, filemanagercontrol and othercontrol, see attached screenshot. File Attachments New Categories in KHelpcenter navigation tree http://git.reviewboard.kde.org/media/uploaded/files/2013/08/03/khelpcenter1.png Thanks, Burkhard Lück
Re: Review Request 111851: Split KCModules into different Categories in KHelpcenter's navigation tree
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111851/#review44734 --- In theory I have a possible solution using the existing X-KDE-PluginKeyword in the desktop files, so no changes outside khelpcenter would be required. But in contrary to the api documentation this does not work or I do wrong: $ ktraderclient --servicetype KCModule --constraint [X-KDE-PluginKeyword] == 'dolphinviewmodes' servicetype is : KCModule constraint is : [X-KDE-PluginKeyword] == 'dolphinviewmodes' got 0 offers. This should return *one* offer kcmdolphinviewmodes.desktop which has X-KDE-ServiceTypes=KCModule + X-KDE-PluginKeyword=dolphinviewmodes What is wrong here? - Burkhard Lück On Nov. 28, 2013, 8:36 p.m., Burkhard Lück wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111851/ --- (Updated Nov. 28, 2013, 8:36 p.m.) Review request for Documentation, KDE Runtime, Albert Astals Cid, Ben Cooksley, David Faure, and Frank Reininghaus. Bugs: 262935 http://bugs.kde.org/show_bug.cgi?id=262935 Repository: kde-runtime Description --- The KHelpcenter navigation tree has a top level item Control Center Modules, an unsorted list of all KCModules (80 for a full kde main modules install from stable). This makes this item hardly usable, see https://bugs.kde.org/show_bug.cgi?id=262935 This patch implements: A) Alphabetical sorting for Control Center Modules/Foo Settings Modules + KInfoCenter items B) New / changed top level categories in the navigation tree: 1) System Settings Modules - replaces old Control Center Modules items: see http://docs.kde.org/stable/en/kde-workspace/systemsettings/general.html 2) Konqueror Settings Modules (see Konqueror settings dialog) items: General, Performance, Bookmarks 3) Filemanager Settings Modules (see Konqueror/Dolphin settings dialog) items: File Management, View Modes, Navigation, Services, General, Trash No File Associations, because it is already in System Settings Modules 4) Browser Settings Modules (see Konqueror settings dialog) items: Web Browsing, Proxy, Appearance, AdBlocK Filters, Web Shortcuts, Cache, History, Cookies, Browser Identification, Java JavaScript, Plugins 5) Other Settings Modules all other items like e.g. CGI Scripts from kde-runtime To make full use of these new/changed categories some kcm desktop files in other modules than kde-runtime need a change of X-KDE-ParentApp from kcontrol to konquerorcontrol, browsercontrol, filemanagercontrol or othercontrol, but that is not part of this review. As long as not all desktop files are fixed according to this patch or a necessary change in a desktop files is overlooked that KCM will be in System Settings Modules like now, but in sorted order. C) Change wording from Control Center Modules to System Settings Modules and using Foo Settings Modules for the new categories. Control Center is from KDE 3, we use System Settings nearly all over GUI and in the whole documentation. Diffs - kcmshell/main.cpp dab8fc6 khelpcenter/navigator.cpp 7460cc8 khelpcenter/plugins/CMakeLists.txt d09b869 khelpcenter/plugins/browsercontrolmodules.desktop e69de29 khelpcenter/plugins/filemanagercontrolmodules.desktop e69de29 khelpcenter/plugins/kcontrolmodules.desktop 1813df3 khelpcenter/plugins/konquerorcontrolmodules.desktop e69de29 khelpcenter/plugins/othercontrolmodules.desktop e69de29 khelpcenter/plugintraverser.cpp b0b0e78 kioslave/cgi/kcmcgi/kcmcgi.desktop f49eeb9 Diff: http://git.reviewboard.kde.org/r/111851/diff/ Testing --- Checked with one example each for konquerorcontrol, browsercontrol, filemanagercontrol and othercontrol, see attached screenshot. File Attachments New Categories in KHelpcenter navigation tree http://git.reviewboard.kde.org/media/uploaded/files/2013/08/03/khelpcenter1.png Thanks, Burkhard Lück
Re: Review Request 111851: Split KCModules into different Categories in KHelpcenter's navigation tree
On Nov. 28, 2013, 8:42 p.m., Burkhard Lück wrote: In theory I have a possible solution using the existing X-KDE-PluginKeyword in the desktop files, so no changes outside khelpcenter would be required. But in contrary to the api documentation this does not work or I do wrong: $ ktraderclient --servicetype KCModule --constraint [X-KDE-PluginKeyword] == 'dolphinviewmodes' servicetype is : KCModule constraint is : [X-KDE-PluginKeyword] == 'dolphinviewmodes' got 0 offers. This should return *one* offer kcmdolphinviewmodes.desktop which has X-KDE-ServiceTypes=KCModule + X-KDE-PluginKeyword=dolphinviewmodes What is wrong here? This is what's wrong: KServicePrivate::property: Request for unknown property ' X-KDE-PluginKeyword ' Fixed by defining this property in kdelibs/kio/kcmodule.desktop: --- a/kio/kcmodule.desktop +++ b/kio/kcmodule.desktop @@ -114,3 +114,8 @@ Type=QString # sets the order of the modules in the TreeList/IconList [PropertyDef::X-KDE-Weight] Type=int + +# TODO: document +[PropertyDef::X-KDE-PluginKeyword] +Type=QString - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111851/#review44734 --- On Nov. 28, 2013, 8:36 p.m., Burkhard Lück wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111851/ --- (Updated Nov. 28, 2013, 8:36 p.m.) Review request for Documentation, KDE Runtime, Albert Astals Cid, Ben Cooksley, David Faure, and Frank Reininghaus. Bugs: 262935 http://bugs.kde.org/show_bug.cgi?id=262935 Repository: kde-runtime Description --- The KHelpcenter navigation tree has a top level item Control Center Modules, an unsorted list of all KCModules (80 for a full kde main modules install from stable). This makes this item hardly usable, see https://bugs.kde.org/show_bug.cgi?id=262935 This patch implements: A) Alphabetical sorting for Control Center Modules/Foo Settings Modules + KInfoCenter items B) New / changed top level categories in the navigation tree: 1) System Settings Modules - replaces old Control Center Modules items: see http://docs.kde.org/stable/en/kde-workspace/systemsettings/general.html 2) Konqueror Settings Modules (see Konqueror settings dialog) items: General, Performance, Bookmarks 3) Filemanager Settings Modules (see Konqueror/Dolphin settings dialog) items: File Management, View Modes, Navigation, Services, General, Trash No File Associations, because it is already in System Settings Modules 4) Browser Settings Modules (see Konqueror settings dialog) items: Web Browsing, Proxy, Appearance, AdBlocK Filters, Web Shortcuts, Cache, History, Cookies, Browser Identification, Java JavaScript, Plugins 5) Other Settings Modules all other items like e.g. CGI Scripts from kde-runtime To make full use of these new/changed categories some kcm desktop files in other modules than kde-runtime need a change of X-KDE-ParentApp from kcontrol to konquerorcontrol, browsercontrol, filemanagercontrol or othercontrol, but that is not part of this review. As long as not all desktop files are fixed according to this patch or a necessary change in a desktop files is overlooked that KCM will be in System Settings Modules like now, but in sorted order. C) Change wording from Control Center Modules to System Settings Modules and using Foo Settings Modules for the new categories. Control Center is from KDE 3, we use System Settings nearly all over GUI and in the whole documentation. Diffs - kcmshell/main.cpp dab8fc6 khelpcenter/navigator.cpp 7460cc8 khelpcenter/plugins/CMakeLists.txt d09b869 khelpcenter/plugins/browsercontrolmodules.desktop e69de29 khelpcenter/plugins/filemanagercontrolmodules.desktop e69de29 khelpcenter/plugins/kcontrolmodules.desktop 1813df3 khelpcenter/plugins/konquerorcontrolmodules.desktop e69de29 khelpcenter/plugins/othercontrolmodules.desktop e69de29 khelpcenter/plugintraverser.cpp b0b0e78 kioslave/cgi/kcmcgi/kcmcgi.desktop f49eeb9 Diff: http://git.reviewboard.kde.org/r/111851/diff/ Testing --- Checked with one example each for konquerorcontrol, browsercontrol, filemanagercontrol and othercontrol, see attached screenshot. File Attachments New Categories in KHelpcenter navigation tree http://git.reviewboard.kde.org/media/uploaded/files/2013/08/03/khelpcenter1.png Thanks, Burkhard Lück
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 21, 2013, 11:22 a.m., Kevin Ottens wrote: To get in this patch would benefit from being based on the frameworks branch and go into kdeclarative. Kevin Ottens wrote: Any chance for an update? Denis Kuplyakov wrote: Yes I will finish it, when have time. There are many pre-exams in university. Kevin Ottens wrote: Any news? we need to get in or discard all the old patches now. Kevin Ottens wrote: Last warning before getting discarded. Patches will soon not be accepted in kdelibs/frameworks in preparation of the repository split. Denis Kuplyakov wrote: Sorry, but I'm still very busy. What is deadline, and can I submit changes after repo-split? Re-submitting *after* the repository is perfectly fine. It will have to be a new review though. If you're fine with that we can just discard that one, and you can resubmit at your convenience post-split. - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review42069 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: kdelibs Description --- It is wrapper to access KColorScheme's methods from QML code. Also added Q_GADGET to KColorScheme to enable Q_ENUMS using, to make them accessible from QML code. As it will be accepted, QML-clone of KgPopupItem will be posted for review to libkdegames, as it uses it to access KDE's color theme. More info: * search for KDE theme colors API for QML thread at kdelibs and kdegames mailinglists * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION kdeui/colors/kcolorscheme.h 17570fd kdeui/colors/kcolorscheme.cpp a6650ac kdeui/colors/kcolorschemetoken.h PRE-CREATION kdeui/colors/kcolorschemetoken.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/112880/diff/ Testing --- I've tested it with KReversi's deniskup/gsoc2013/newdesign branch. Thanks, Denis Kuplyakov
