Re: Review Request 119572: Part: use correct slots for QStatusbar
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119572/#review63769 --- Ship it! Ship It! - Martin Tobias Holmedahl Sandsmark On Aug. 2, 2014, 10:19 a.m., Heiko Becker wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119572/ --- (Updated Aug. 2, 2014, 10:19 a.m.) Review request for kdelibs and Martin Tobias Holmedahl Sandsmark. Repository: filelight Description --- Part: use correct slots for QStatusbar The correct names are showMessage(..) and clearMessage(). Diffs - src/part/part.cpp e63c852151c54018dc5dee448b0bc1367d6ee149 Diff: https://git.reviewboard.kde.org/r/119572/diff/ Testing --- Warning message is gone when running and file names start appearing in the status bar. Thanks, Heiko Becker
Re: Review Request 118811: Fix compile with giflib-5.1.0 and upwards.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118811/#review60379 --- Ship it! Ship It! - Martin Tobias Holmedahl Sandsmark On June 18, 2014, 11:31 a.m., Milian Wolff wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118811/ --- (Updated June 18, 2014, 11:31 a.m.) Review request for kdelibs, Aleix Pol Gonzalez, Andreas Schwab, David Faure, and Raymond Wooninck. Repository: kdelibs Description --- Fix compile with giflib-5.1.0 and upwards. See news about the giflib-5.1.0 release about the API break here: http://fossies.org/linux/giflib/NEWS Diffs - khtml/imload/decoders/gifloader.cpp 6c61ff502b7891b2973847c198f0aaf55e5c9143 Diff: https://git.reviewboard.kde.org/r/118811/diff/ Testing --- Thanks, Milian Wolff
Re: Review Request 114623: kjs: Implement es6 Math.fround
On Dec. 24, 2013, 1:13 p.m., Martin Tobias Holmedahl Sandsmark wrote: what a silly function, but looks okay to me. Aleix Pol Gonzalez wrote: Shouldn't this go to KF5? Also kdelibs is feature frozen... yes, it should go into kf5 as well, but it was decided some time ago that missing implementations in kjs and khtml counted as bugs. - Martin Tobias Holmedahl --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114623/#review46132 --- On Jan. 16, 2014, 5:56 p.m., Bernd Buschinski wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114623/ --- (Updated Jan. 16, 2014, 5:56 p.m.) Review request for kdelibs. Repository: kdelibs Description --- kjs: Implement es6 Math.fround Diffs - kjs/math_object.h 3d193dd kjs/math_object.cpp 89835e5 Diff: https://git.reviewboard.kde.org/r/114623/diff/ Testing --- Basic Mozilla tests: Math.fround(0) // 0 Math.fround(1) // 1 Math.fround(1.337) // 1.337123977661 Math.fround(1.5) // 1.5 Math.fround(NaN) // NaN Math.fround(Infinity) // Inf Math.fround(-Infinity) // -Inf Thanks, Bernd Buschinski
Re: Review Request 114717: Language detection in Sonnet
e9fc573 data/CMakeLists.txt PRE-CREATION Diff: https://git.reviewboard.kde.org/r/114717/diff/ Testing --- mostly using test_highlighter. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 114717: Language detection in Sonnet
On Jan. 2, 2014, 2:47 p.m., Vishesh Handa wrote: src/plugins/CMakeLists.txt, line 29 https://git.reviewboard.kde.org/r/114717/diff/1/?file=227733#file227733line29 We don't care about Enchant any more? it's seems to be unmaintained, is just an abstraction library, and it spammed my stdout, so I dropped it. On Jan. 2, 2014, 2:47 p.m., Vishesh Handa wrote: src/core/settings.cpp, line 229 https://git.reviewboard.kde.org/r/114717/diff/1/?file=227725#file227725line229 Does the case matter over here? eh, it's just an internal, static function. I thought it looked prettier this way. :-) - Martin Tobias Holmedahl --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114717/#review46626 --- On Dec. 29, 2013, 4:49 a.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114717/ --- (Updated Dec. 29, 2013, 4:49 a.m.) Review request for kdelibs and KDEPIM. Repository: sonnet Description --- I started by merging in the old language detection branch from SVN, while improving it as I went along. One improvement was to use QChar's unicode information instead of shipping our own unicode code point information tables. The old filter class also got replaced with a new tokenizer, which I rewrote most of to simplify. I added kdepim to the reviewers because I remember talking with someone working on PIM stuff on IRC, and he was interested in this (a long time ago, though). Diffs - data/trigrams/ja PRE-CREATION data/trigrams/kk PRE-CREATION data/trigrams/ko PRE-CREATION data/trigrams/ky PRE-CREATION data/trigrams/la PRE-CREATION data/trigrams/lt PRE-CREATION data/trigrams/lv PRE-CREATION data/trigrams/mk PRE-CREATION data/trigrams/mn PRE-CREATION data/trigrams/nb PRE-CREATION data/trigrams/ne PRE-CREATION data/trigrams/nl PRE-CREATION data/trigrams/nr PRE-CREATION data/trigrams/pl PRE-CREATION data/trigrams/ps PRE-CREATION data/trigrams/pt PRE-CREATION data/trigrams/pt_BR PRE-CREATION data/trigrams/pt_PT PRE-CREATION data/trigrams/ro PRE-CREATION data/trigrams/ru PRE-CREATION data/trigrams/sk PRE-CREATION data/trigrams/sl PRE-CREATION data/trigrams/so PRE-CREATION data/trigrams/sq PRE-CREATION data/trigrams/sr PRE-CREATION data/trigrams/ss PRE-CREATION data/trigrams/st PRE-CREATION data/trigrams/sv PRE-CREATION data/trigrams/sw PRE-CREATION data/trigrams/th PRE-CREATION data/trigrams/tl PRE-CREATION data/trigrams/tn PRE-CREATION data/trigrams/tr PRE-CREATION data/trigrams/ts PRE-CREATION data/trigrams/uk PRE-CREATION data/trigrams/ur PRE-CREATION data/trigrams/uz PRE-CREATION data/trigrams/ve PRE-CREATION data/trigrams/vi PRE-CREATION data/trigrams/xh PRE-CREATION data/trigrams/zu PRE-CREATION sonnet.yaml c54f87b src/CMakeLists.txt e79492f src/core/CMakeLists.txt 2f8a184 src/core/backgroundchecker.cpp 8b9e983 src/core/backgroundchecker_p.h PRE-CREATION src/core/backgroundengine.cpp 3a14d34 src/core/backgroundengine_p.h 10f6a27 src/core/client_p.h bd3e416 src/core/filter.cpp e99d332 src/core/filter_p.h 6c7d8c9 src/core/globals.h 0c54c96 src/core/globals.cpp e57450f src/core/guesslanguage.h PRE-CREATION src/core/guesslanguage.cpp PRE-CREATION src/core/languagefilter.cpp PRE-CREATION src/core/languagefilter_p.h PRE-CREATION src/core/loader.cpp ee8db0e src/core/settings.cpp 095eddb src/core/settings_p.h ee2d22c src/core/speller.h 7428339 src/core/speller.cpp 8cc2a1e src/core/textbreaks.cpp PRE-CREATION src/core/textbreaks_p.h PRE-CREATION src/core/tokenizer.cpp PRE-CREATION src/core/tokenizer_p.h PRE-CREATION src/plugins/CMakeLists.txt fc33a97 src/plugins/aspell/kspell_aspellclient.h eadb52a src/plugins/enchant/CMakeLists.txt 817db0c src/plugins/enchant/enchantclient.h 25f62eb src/plugins/hspell/CMakeLists.txt e128cb3 src/plugins/hspell/kspell_hspellclient.h 966303f src/plugins/hunspell/CMakeLists.txt ccae7f7 src/plugins/hunspell/kspell_hunspellclient.h 79638bb src/ui/configui.ui 6532552 src/ui/configwidget.cpp 7a5cc99 src/ui/dialog.cpp 13ad39d src/ui/highlighter.h 46418b9 src/ui/highlighter.cpp 9f31268 src/unicode/CMakeLists.txt 1be0a54 src/unicode/README f9b8030 src/unicode/data/GraphemeBreakProperty.txt 8805f36 src/unicode/data/SentenceBreakProperty.txt fc58820 src/unicode/data/WordBreakProperty.txt 78c531c src/unicode/parseucd/parseucd.cpp a050140
Re: Review Request 114717: Language detection in Sonnet
On Sun, Dec 29, 2013 at 05:46:33PM -, Christoph Feck wrote: It probably has better detection (uses quadgraphs instead of trigraphs), and covers more languages, but it hardly looks compact, with the cld2_generated_quad0720.cc file being over 20 megabytes large :) Exactly. I could probably have gotten better detection rate from the ngrams alone if I was willing to increase the datasize, but we've limited to 300 trigrams per language to keep the datasize tiny (remember that this is loaded into every process). Currently the entire trigram map for all languages is 264KB, and this could be improved further with for example a trie. Instead we use the hybrid algorithm which increases the detection ability drastically, and is much better for our usecase (detecting the language to use for spellchecking). That said, it would be trivial to enhance the sonnet implementation to use quadgrams in the future, I just don't think the tradeoff is worth it at the moment. -- Martin Sandsmark
Review Request 114717: Language detection in Sonnet
/ Testing --- mostly using test_highlighter. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 114455: kjs: Implement ES6 Math.imul
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114455/#review46131 --- Ship it! Ship It! - Martin Tobias Holmedahl Sandsmark On Dec. 22, 2013, 7:34 p.m., Bernd Buschinski wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114455/ --- (Updated Dec. 22, 2013, 7:34 p.m.) Review request for kdelibs. Repository: kdelibs Description --- kjs: Implement ES6 Math.imul I admit I am totally lost here, I need help, the specs just confuse me. The code doesn't look like it does anything useful. step 1 to 4 is just getting the values. a and b must be a uint32_t. 5. Let product be (a * b) modulo 2^32. 6. If product ? 2^31, return product - 2^32, otherwise return product. (Taken from ES6 draft 08.11.2013) This is the part I don't understand at all. I am not even sure about the product datatype. Also NOTE: the JSC and firefox implementation look totally different and both return different values in highvalue cases, so I rather not trust them. Diffs - kjs/math_object.h 3d193dd kjs/math_object.cpp 89835e5 Diff: https://git.reviewboard.kde.org/r/114455/diff/ Testing --- Thanks, Bernd Buschinski
Re: Review Request 114623: kjs: Implement es6 Math.fround
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114623/#review46132 --- Ship it! what a silly function, but looks okay to me. - Martin Tobias Holmedahl Sandsmark On Dec. 22, 2013, 7:34 p.m., Bernd Buschinski wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114623/ --- (Updated Dec. 22, 2013, 7:34 p.m.) Review request for kdelibs. Repository: kdelibs Description --- kjs: Implement es6 Math.fround Diffs - kjs/math_object.h 3d193dd kjs/math_object.cpp 89835e5 Diff: https://git.reviewboard.kde.org/r/114623/diff/ Testing --- Basic Mozilla tests: Math.fround(0) // 0 Math.fround(1) // 1 Math.fround(1.337) // 1.337123977661 Math.fround(1.5) // 1.5 Math.fround(NaN) // NaN Math.fround(Infinity) // Inf Math.fround(-Infinity) // -Inf Thanks, Bernd Buschinski
Re: Review Request 114472: kjs: Implement ES6 Number parseInt, parseFloat
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114472/#review45855 --- Ship it! Ship It! - Martin Tobias Holmedahl Sandsmark On Dec. 15, 2013, 10:07 a.m., Bernd Buschinski wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114472/ --- (Updated Dec. 15, 2013, 10:07 a.m.) Review request for kdelibs. Repository: kdelibs Description --- kjs: Implement ES6 Number parseInt, parseFloat NOTE: unlike the other new ES6 Number function these two MUST behave the same as the global parseInt/parseFloat. The explicit number type check is only valid for the other functions, not for parseInt/parseFloat and therefor was moved to the functions. Diffs - kjs/function.h 17958ab kjs/function.cpp 1102208 kjs/number_object.h 634c642 kjs/number_object.cpp c284746 Diff: http://git.reviewboard.kde.org/r/114472/diff/ Testing --- Thanks, Bernd Buschinski
Re: Review Request 114448: kjs: reogranize basic math functions function checking
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114448/#review45689 --- Ship it! Ship It! - Martin Tobias Holmedahl Sandsmark On Dec. 14, 2013, 4:30 p.m., Bernd Buschinski wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114448/ --- (Updated Dec. 14, 2013, 4:30 p.m.) Review request for kdelibs. Repository: kdelibs Description --- kjs: reorganize basic math functions function checking. Most noticeable change is that kjs will no longer quietly fail at runtime if a important function is missing, but fail at compiletime. This will only happen on a very strange platform, or a typo in my checks. Also note, I added more checks, so theoretically more platforms are now supported. Ah yes and the math functions are now always inlined, not that it makes any huge performance difference... Diffs - khtml/ecma/kjs_arraybuffer.cpp c4d2e51 khtml/ecma/kjs_arraybufferview.h 4b6992b khtml/ecma/kjs_context2d.cpp bbdba8f kjs/CMakeLists.txt 48f5231 kjs/config-kjs.h.cmake f644528 kjs/date_object.cpp c8d776c kjs/function.cpp 1102208 kjs/internal.cpp 49c2ce2 kjs/jsonstringify.cpp e07a789 kjs/math_object.cpp 89835e5 kjs/number_object.cpp c284746 kjs/operations.h 1112c2b kjs/operations.cpp 9e2fc71 kjs/value.cpp f02aeea kjs/wtf/MathExtras.h 58be75b Diff: http://git.reviewboard.kde.org/r/114448/diff/ Testing --- Thanks, Bernd Buschinski
Re: Review Request 114454: kjs: Implement ES6 Math.hypot
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114454/#review45690 --- how about just a single loop, but store if we've found a nan? then exit early if you find an inf, or return nan after the loop is over if a nan was found. - Martin Tobias Holmedahl Sandsmark On Dec. 14, 2013, 4:30 p.m., Bernd Buschinski wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114454/ --- (Updated Dec. 14, 2013, 4:30 p.m.) Review request for kdelibs. Repository: kdelibs Description --- kjs: Implement ES6 Math.hypot I agree that the loop, that is just checking for Inf looks weird, but thats what the spec says. We MUST check everything first for Inf, NaN checks must come later. So a (1, NaN, Inf) must return Inf. Diffs - kjs/math_object.h 3d193dd kjs/math_object.cpp 89835e5 Diff: http://git.reviewboard.kde.org/r/114454/diff/ Testing --- Thanks, Bernd Buschinski
Re: Review Request 114449: kjs: Implement ES6 acosh, asinh, cbrt, cosh, exmp1, log1p, log10, sign, sinh, tanh, trunc Math Functions
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114449/#review45691 --- Ship it! Ship It! - Martin Tobias Holmedahl Sandsmark On Dec. 14, 2013, 4:30 p.m., Bernd Buschinski wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114449/ --- (Updated Dec. 14, 2013, 4:30 p.m.) Review request for kdelibs. Repository: kdelibs Description --- kjs: Implement ES6 acosh, asinh, cbrt, cosh, exmp1, log1p, log10, sign, sinh, tanh, trunc Math Functions sign, is a bit different from the c++ signbit function, but everything else is just straight forward. Diffs - kjs/math_object.h 3d193dd kjs/math_object.cpp 89835e5 Diff: http://git.reviewboard.kde.org/r/114449/diff/ Testing --- Thanks, Bernd Buschinski
Re: Review Request 114454: kjs: Implement ES6 Math.hypot
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114454/#review45696 --- Ship it! Ship It! - Martin Tobias Holmedahl Sandsmark On Dec. 14, 2013, 6:53 p.m., Bernd Buschinski wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114454/ --- (Updated Dec. 14, 2013, 6:53 p.m.) Review request for kdelibs. Repository: kdelibs Description --- kjs: Implement ES6 Math.hypot I agree that the loop, that is just checking for Inf looks weird, but thats what the spec says. We MUST check everything first for Inf, NaN checks must come later. So a (1, NaN, Inf) must return Inf. Diffs - kjs/math_object.h 3d193dd kjs/math_object.cpp 89835e5 Diff: http://git.reviewboard.kde.org/r/114454/diff/ Testing --- Thanks, Bernd Buschinski
Re: Review Request 114464: kjs: Implement ES6 Number isFinite, isInteger, isSafeInteger, isNaN and add MAX_SAFE_INTEGER, MIN_SAFE_INTEGER constants
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114464/#review45706 --- Ship it! Ship It! - Martin Tobias Holmedahl Sandsmark On Dec. 14, 2013, 8:01 p.m., Bernd Buschinski wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114464/ --- (Updated Dec. 14, 2013, 8:01 p.m.) Review request for kdelibs. Repository: kdelibs Description --- kjs: Implement ES6 Number isFinite, isInteger, isSafeInteger, isNaN and add MAX_SAFE_INTEGER, MIN_SAFE_INTEGER constants Diffs - kjs/number_object.h 634c642 kjs/number_object.cpp c284746 Diff: http://git.reviewboard.kde.org/r/114464/diff/ Testing --- Thanks, Bernd Buschinski
Re: Review Request 113342: Add support for retrying to KIO (frameworks branch)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113342/ --- (Updated Nov. 2, 2013, 6:48 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, kdelibs and David Faure. Repository: kdelibs Description --- Add an explicit retry button to the kio skipdialog. Diffs - staging/kio/src/core/chmodjob.cpp 236386d staging/kio/src/core/copyjob.cpp 794efa1 staging/kio/src/core/jobuidelegateextension.h 689647f staging/kio/src/widgets/skipdialog.h a45f654 staging/kio/src/widgets/skipdialog.cpp d463ba1 Diff: http://git.reviewboard.kde.org/r/113342/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 113518: KDM/KFrontend: Avoid potentially exploitable privilege dropping
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113518/#review42880 --- looks good to me, but please add Oswald Buddenhagen (ossi) to the reviewers list, he's the KDM maintainer. - Martin Tobias Holmedahl Sandsmark On Oct. 31, 2013, 9:57 a.m., Martin Bříza wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113518/ --- (Updated Oct. 31, 2013, 9:57 a.m.) Review request for kde-workspace. Repository: kde-workspace Description --- Initialize the user's groups in between calling setegid and seteuid to have the correct supplemental groups in place. Diffs - kdm/kfrontend/kgreeter.cpp 1bddab5 Diff: http://git.reviewboard.kde.org/r/113518/diff/ Testing --- Thanks, Martin Bříza
Re: Review Request 113342: Add support for retrying to KIO (frameworks branch)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113342/ --- (Updated Nov. 1, 2013, 11:31 p.m.) Review request for KDE Frameworks, kdelibs and David Faure. Repository: kdelibs Description --- Add an explicit retry button to the kio skipdialog. Diffs - staging/kio/src/core/chmodjob.cpp 236386d staging/kio/src/core/copyjob.cpp 794efa1 staging/kio/src/core/jobuidelegateextension.h 689647f staging/kio/src/widgets/skipdialog.h a45f654 staging/kio/src/widgets/skipdialog.cpp d463ba1 Diff: http://git.reviewboard.kde.org/r/113342/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 113342: Add support for retrying to KIO (frameworks branch)
On Oct. 21, 2013, 8:03 a.m., David Faure wrote: Nice. This is simpler than I thought it would be. Please close bug 61, and verify that you can close bugs 44563 and 29526 too. kind of hard to verify when kioslavetest crashes on me. I'll try to figure out why that is happening. :) - Martin Tobias Holmedahl --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113342/#review42054 --- On Oct. 18, 2013, 8:55 p.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113342/ --- (Updated Oct. 18, 2013, 8:55 p.m.) Review request for kdelibs and David Faure. Repository: kdelibs Description --- Add an explicit retry button to the kio skipdialog. Diffs - staging/kio/src/core/chmodjob.cpp 236386d staging/kio/src/core/copyjob.cpp 794efa1 staging/kio/src/core/jobuidelegateextension.h 689647f staging/kio/src/widgets/skipdialog.h a45f654 staging/kio/src/widgets/skipdialog.cpp d463ba1 Diff: http://git.reviewboard.kde.org/r/113342/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Review Request 113342: Add support for retrying to KIO (frameworks branch)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113342/ --- Review request for kdelibs and David Faure. Repository: kdelibs Description --- Add an explicit retry button to the kio skipdialog. Diffs - staging/kio/src/core/chmodjob.cpp 236386d staging/kio/src/core/copyjob.cpp 794efa1 staging/kio/src/core/jobuidelegateextension.h 689647f staging/kio/src/widgets/skipdialog.h a45f654 staging/kio/src/widgets/skipdialog.cpp d463ba1 Diff: http://git.reviewboard.kde.org/r/113342/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 112294: Implement multi-seat support in KDM
On Sept. 3, 2013, 10:20 p.m., Oswald Buddenhagen wrote: given that there is no intention to make further feature releases of the kde workspace which will include kdm, i wonder why we'd go through the (potentially tedious) process of upstreaming this now? Stefan Brüns wrote: The reason for sending this was to have one canonical implementation for multiseat support which is upstream. Otherwise, any patches/bugreports must be coordinated downstream, which I really dislike. Reason for pushing this into KDM is that: a) KDM is here today and will stay for some time b) this patch has been tested thoroughly c) alternative DMs are not up to the job yet (SDDM) or introduce additional dependencies (GDM) d) I want multiseat support in the DM now, not in a distant future Oswald Buddenhagen wrote: that's besides the point. whatever gets merged now will never be released. i'm not quite sure why the responsible persons didn't rm -rf the directories yet. Well, at least it gives distros somewhere to pick the patch from. - Martin Tobias Holmedahl --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/#review39302 --- On Sept. 2, 2013, 11:34 p.m., Stefan Brüns wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/ --- (Updated Sept. 2, 2013, 11:34 p.m.) Review request for kde-workspace and Oswald Buddenhagen. Description --- This patch implements dynamic multiseat in KDM. It follows the description in: http://www.freedesktop.org/wiki/Software/systemd/writing-display-managers/ In case systemd is no found at compile time, nothing changes. If logind is not running, nothing changes. If no additional seats have been configured (some Plugable USB-GPUs are automatically added as additional seats), nothing changes. In case there are additional seats beyond seat0, a reserved display is promoted to a local static one (and demoted if the seat is removed) and a new X-Server/greeter is spawned. The code has been tested extensively, with a combination of [Radeon dedicated GPU|Intel iGPU], [Intel iGPU|Displaylink USB GPU] and others. For history of this patch, see https://bugzilla.redhat.com/show_bug.cgi?id=884271 and https://bugzilla.redhat.com/show_bug.cgi?id=975079 Diffs - CMakeLists.txt a3bdbb3 cmake/modules/CMakeLists.txt 117b3a5 cmake/modules/FindSystemd.cmake PRE-CREATION kdm/backend/CMakeLists.txt 25f383f kdm/backend/client.c 26bb0b4 kdm/backend/dm.h 64e106b kdm/backend/dm.c e0f1366 kdm/backend/server.c d8dd6f3 kdm/backend/session.c 0e7901c Diff: http://git.reviewboard.kde.org/r/112294/diff/ Testing --- Single seat system, several multiseat systems Thanks, Stefan Brüns
Re: Review Request 112294: Implement multi-seat support in KDM
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/#review39221 --- I don't see anything obviously wrong with it, but you should probably explicitly add Oswald Buddenhagen as a reviewer. - Martin Tobias Holmedahl Sandsmark On Sept. 2, 2013, 7:57 p.m., Stefan Brüns wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/ --- (Updated Sept. 2, 2013, 7:57 p.m.) Review request for kde-workspace. Description --- This patch implements dynamic multiseat in KDM. It follows the description in: http://www.freedesktop.org/wiki/Software/systemd/writing-display-managers/ In case systemd is no found at compile time, nothing changes. If logind is not running, nothing changes. If no additional seats have been configured (some Plugable USB-GPUs are automatically added as additional seats), nothing changes. In case there are additional seats beyond seat0, a reserved display is promoted to a local static one (and demoted if the seat is removed) and a new X-Server/greeter is spawned. The code has been tested extensively, with a combination of [Radeon dedicated GPU|Intel iGPU], [Intel iGPU|Displaylink USB GPU] and others. For history of this patch, see https://bugzilla.redhat.com/show_bug.cgi?id=884271 and https://bugzilla.redhat.com/show_bug.cgi?id=975079 Diffs - CMakeLists.txt a3bdbb3 cmake/modules/CMakeLists.txt 117b3a5 cmake/modules/FindSystemd.cmake PRE-CREATION kdm/backend/CMakeLists.txt 25f383f kdm/backend/client.c 26bb0b4 kdm/backend/dm.h 64e106b kdm/backend/dm.c e0f1366 kdm/backend/server.c d8dd6f3 kdm/backend/session.c 0e7901c Diff: http://git.reviewboard.kde.org/r/112294/diff/ Testing --- Single seat system, several multiseat systems Thanks, Stefan Brüns
Re: Review Request 112294: Implement multi-seat support in KDM
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/#review38743 --- kdm/backend/dm.h http://git.reviewboard.kde.org/r/112294/#comment28606 minor nitpick, but it is preferred to use camelcasing (Qt coding style and all that). looks fine to me, just that tiny minor nitpick on variable naming. - Martin Tobias Holmedahl Sandsmark On Aug. 26, 2013, 3:49 p.m., Stefan Brüns wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/ --- (Updated Aug. 26, 2013, 3:49 p.m.) Review request for kde-workspace. Description --- This patch implements dynamic multiseat in KDM. It follows the description in: http://www.freedesktop.org/wiki/Software/systemd/writing-display-managers/ In case systemd is no found at compile time, nothing changes. If logind is not running, nothing changes. If no additional seats have been configured (some Plugable USB-GPUs are automatically added as additional seats), nothing changes. In case there are additional seats beyond seat0, a reserved display is promoted to a local static one (and demoted if the seat is removed) and a new X-Server/greeter is spawned. The code has been tested extensively, with a combination of [Radeon dedicated GPU|Intel iGPU], [Intel iGPU|Displaylink USB GPU] and others. For history of this patch, see https://bugzilla.redhat.com/show_bug.cgi?id=884271 and https://bugzilla.redhat.com/show_bug.cgi?id=975079 Diffs - CMakeLists.txt a3bdbb3 cmake/modules/CMakeLists.txt 117b3a5 cmake/modules/FindSystemd.cmake PRE-CREATION kdm/backend/CMakeLists.txt 25f383f kdm/backend/client.c 26bb0b4 kdm/backend/dm.h 64e106b kdm/backend/dm.c e0f1366 kdm/backend/server.c d8dd6f3 kdm/backend/session.c 0e7901c Diff: http://git.reviewboard.kde.org/r/112294/diff/ Testing --- Single seat system, several multiseat systems Thanks, Stefan Brüns
Re: Review Request 109551: port KPtyProcess to QProcess
On March 18, 2013, 7:25 a.m., Oswald Buddenhagen wrote: kpty/tests/kptyprocesstest.cpp, line 180 http://git.reviewboard.kde.org/r/109551/diff/1/?file=120230#file120230line180 why should they? you already have the solution in the previous hunk. Doh, I'm just a silly moose. Thanks for reminding me. :-) - Martin Tobias Holmedahl --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109551/#review29421 --- On March 17, 2013, 4:44 p.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109551/ --- (Updated March 17, 2013, 4:44 p.m.) Review request for KDE Frameworks, kdelibs and David Faure. Description --- Just a simple port of KPtyProcess away from using KProcess. Diffs - kpty/kptyprocess.h 5e0df96 kpty/kptyprocess.cpp 015a58c kpty/tests/kptyprocesstest.h 64bded0 kpty/tests/kptyprocesstest.cpp 04990a0 Diff: http://git.reviewboard.kde.org/r/109551/diff/ Testing --- builds and tests pass. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 109549: port KRun away from KProcess
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109549/ --- (Updated March 18, 2013, 7:51 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, kdelibs and David Faure. Description --- Port KRun to use QProcess instead of KProcess. Instead of passing around KProcess instances, we simply pass the command with arguments and the working directory. Diffs - kio/kio/krun.cpp 76b7385 kio/kio/krun_p.h 01abb69 Diff: http://git.reviewboard.kde.org/r/109549/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 109551: port KPtyProcess to QProcess
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109551/ --- (Updated March 18, 2013, 7:54 p.m.) Review request for KDE Frameworks, kdelibs, David Faure, and Oswald Buddenhagen. Changes --- try to fix the outcommented tests. still don't know why the second one fails. Description --- Just a simple port of KPtyProcess away from using KProcess. Diffs (updated) - kpty/kptyprocess.h 5e0df96 kpty/kptyprocess.cpp 015a58c kpty/tests/kptyprocesstest.cpp 04990a0 Diff: http://git.reviewboard.kde.org/r/109551/diff/ Testing --- builds and tests pass. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 109551: port KPtyProcess to QProcess
On March 18, 2013, 10:04 p.m., Oswald Buddenhagen wrote: kpty/tests/kptyprocesstest.cpp, line 193 http://git.reviewboard.kde.org/r/109551/diff/2/?file=120310#file120310line193 i don't think eating the sleep is a good idea. i'm sure i added it for a reason (in a previous life ^^). with the sleep the test fails, and even with high load I can't get the test to fail without it. On March 18, 2013, 10:04 p.m., Oswald Buddenhagen wrote: kpty/tests/kptyprocesstest.cpp, line 208 http://git.reviewboard.kde.org/r/109551/diff/2/?file=120310#file120310line208 because it's completely broken ^^ having looked at QProcess and KProcess and KPtyDevice I still don't understand what is broken... On March 18, 2013, 10:04 p.m., Oswald Buddenhagen wrote: kpty/tests/kptyprocesstest.cpp, line 210 http://git.reviewboard.kde.org/r/109551/diff/2/?file=120310#file120310line210 the -c needs to be a separate argument. the quotes, backslashes and attempt at a newline are all garbage. I first had -c as a separate argument, and it didn't matter. And I had to add an extra newline to get it to work with doing it manually. - Martin Tobias Holmedahl --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109551/#review29478 --- On March 18, 2013, 7:54 p.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109551/ --- (Updated March 18, 2013, 7:54 p.m.) Review request for KDE Frameworks, kdelibs, David Faure, and Oswald Buddenhagen. Description --- Just a simple port of KPtyProcess away from using KProcess. Diffs - kpty/kptyprocess.h 5e0df96 kpty/kptyprocess.cpp 015a58c kpty/tests/kptyprocesstest.cpp 04990a0 Diff: http://git.reviewboard.kde.org/r/109551/diff/ Testing --- builds and tests pass. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 109531: Port the Sonnet ASpell plugin to the new style APIs
On March 17, 2013, 11:51 a.m., David Faure wrote: staging/sonnet/src/plugins/aspell/kspell_aspellclient.cpp, line 82 http://git.reviewboard.kde.org/r/109531/diff/1/?file=120141#file120141line82 Doesn't the .json file need to exist? Alternatively I think the FILE argument can be empty, iirc. Ah, forgot to add the json file... On March 17, 2013, 11:51 a.m., David Faure wrote: staging/sonnet/src/core/client_p.h, line 78 http://git.reviewboard.kde.org/r/109531/diff/1/?file=120139#file120139line78 Is this used anywhere? Funnily enough, the qobject_castClient * in loadPlugin already compiles. Ah I see, that's because Client is a QObject, but it wouldn't need to be, with the interface stuff. AFAIK it needs to be a QObject for QPluginLoader to load it? - Martin Tobias Holmedahl --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109531/#review29362 --- On March 16, 2013, 11:16 p.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109531/ --- (Updated March 16, 2013, 11:16 p.m.) Review request for KDE Frameworks and kdelibs. Description --- Port the ASpell plugin to the new style of doing plugins in Qt5. I'll do the rest of the plugins afterwards if this looks okay. Diffs - staging/sonnet/src/core/client_p.h cd5c0f2 staging/sonnet/src/plugins/aspell/kspell_aspellclient.h 95173ff staging/sonnet/src/plugins/aspell/kspell_aspellclient.cpp b649d0d Diff: http://git.reviewboard.kde.org/r/109531/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 109531: Port the Sonnet ASpell plugin to the new style APIs
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109531/ --- (Updated March 17, 2013, noon) Status -- This change has been marked as submitted. Review request for KDE Frameworks and kdelibs. Description --- Port the ASpell plugin to the new style of doing plugins in Qt5. I'll do the rest of the plugins afterwards if this looks okay. Diffs - staging/sonnet/src/core/client_p.h cd5c0f2 staging/sonnet/src/plugins/aspell/kspell_aspellclient.h 95173ff staging/sonnet/src/plugins/aspell/kspell_aspellclient.cpp b649d0d Diff: http://git.reviewboard.kde.org/r/109531/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Review Request 109538: port KFileMetaDataReader to QProcess
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109538/ --- Review request for KDE Frameworks, kdelibs, David Faure, and Vishesh Handa. Description --- KFileMetaDataReader currently uses KProcess, this ports it to use QProcess instead. Diffs - kio/kfile/kfilemetadatareader.cpp 88cadaa Diff: http://git.reviewboard.kde.org/r/109538/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 109538: port KFileMetaDataReader to QProcess
On March 17, 2013, 2:05 p.m., Vishesh Handa wrote: But why? KFileMetadataReader and the other KFileMetadataStuff should just be marked as deprecated. Why are we porting them? We already have better alternatives in the nepomuk-widgets repository. Because it was a simple user of KProcess. But if we can just deprecate the whole class (and move it into kde4support, I guess?) that's better. :-) - Martin Tobias Holmedahl --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109538/#review29377 --- On March 17, 2013, 1:26 p.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109538/ --- (Updated March 17, 2013, 1:26 p.m.) Review request for KDE Frameworks, kdelibs, David Faure, and Vishesh Handa. Description --- KFileMetaDataReader currently uses KProcess, this ports it to use QProcess instead. Diffs - kio/kfile/kfilemetadatareader.cpp 88cadaa Diff: http://git.reviewboard.kde.org/r/109538/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Review Request 109549: port KRun away from KProcess
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109549/ --- Review request for KDE Frameworks, kdelibs and David Faure. Description --- Port KRun to use QProcess instead of KProcess. Instead of passing around KProcess instances, we simply pass the command with arguments and the working directory. Diffs - kio/kio/krun.cpp 76b7385 kio/kio/krun_p.h 01abb69 Diff: http://git.reviewboard.kde.org/r/109549/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Review Request 109551: port KPtyProcess to QProcess
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109551/ --- Review request for KDE Frameworks, kdelibs and David Faure. Description --- Just a simple port of KPtyProcess away from using KProcess. Diffs - kpty/kptyprocess.h 5e0df96 kpty/kptyprocess.cpp 015a58c kpty/tests/kptyprocesstest.h 64bded0 kpty/tests/kptyprocesstest.cpp 04990a0 Diff: http://git.reviewboard.kde.org/r/109551/diff/ Testing --- builds and tests pass. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 109549: port KRun away from KProcess
On March 17, 2013, 4:41 p.m., Rolf Eike Beer wrote: kio/kio/krun.cpp, line 1780 http://git.reviewboard.kde.org/r/109549/diff/1/?file=120223#file120223line1780 Whitespacing around braces is inconsistent. it's consistent with the rest of the code around it? On March 17, 2013, 4:41 p.m., Rolf Eike Beer wrote: kio/kio/krun.cpp, line 1781 http://git.reviewboard.kde.org/r/109549/diff/1/?file=120223#file120223line1781 Trailing whitespace fixed locally, but a bother to upload a new patch... - Martin Tobias Holmedahl --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109549/#review29391 --- On March 17, 2013, 4:19 p.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109549/ --- (Updated March 17, 2013, 4:19 p.m.) Review request for KDE Frameworks, kdelibs and David Faure. Description --- Port KRun to use QProcess instead of KProcess. Instead of passing around KProcess instances, we simply pass the command with arguments and the working directory. Diffs - kio/kio/krun.cpp 76b7385 kio/kio/krun_p.h 01abb69 Diff: http://git.reviewboard.kde.org/r/109549/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 107791: move Sonnet out of KDEUI, port away from KDE classes.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107791/ --- (Updated March 16, 2013, 11:13 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, kdelibs and David Faure. Description --- Ported everything away from KConfig to QSettings. I couldn't really find any users of the ::save function, so I think the source incompatible change would be worth it. Alternatively we could add a deprecated dummy function that takes in a KConfig object and just ignores it, and uses QSettings. Diffs - kdeui/CMakeLists.txt 08faa20 kdeui/sonnet/configdialog.h 4c05f0d kdeui/sonnet/configdialog.cpp af3f9b4 kdeui/sonnet/configui.ui 11dffc0 kdeui/sonnet/configwidget.h 023b659 kdeui/sonnet/configwidget.cpp 549d5af kdeui/sonnet/dialog.h 241bd02 kdeui/sonnet/dialog.cpp 3c6f99f kdeui/sonnet/dictionarycombobox.h 82a807c kdeui/sonnet/dictionarycombobox.cpp 32e0970 kdeui/sonnet/highlighter.h f3bc4af kdeui/sonnet/highlighter.cpp d72b9e0 kdeui/sonnet/sonnetui.ui 03af35b kdeui/sonnet/tests/test_configdialog.cpp 4c4fd21 kdeui/widgets/ktextedit.h d0c1c4d kdeui/widgets/ktextedit.cpp 5bf4741 staging/sonnet/src/CMakeLists.txt 157dfa9 staging/sonnet/src/core/CMakeLists.txt 2f11dc0 staging/sonnet/src/core/backgroundchecker.h f0da3a3 staging/sonnet/src/core/backgroundchecker.cpp dc05b94 staging/sonnet/src/core/globals.cpp bf4f504 staging/sonnet/src/core/loader.cpp bfa56a9 staging/sonnet/src/core/settings.cpp e405c30 staging/sonnet/src/core/settings_p.h e14bad7 staging/sonnet/src/core/speller.h 37dd82f staging/sonnet/src/core/speller.cpp 024d7f7 staging/sonnet/src/ui/configdialog.h PRE-CREATION staging/sonnet/src/ui/configdialog.cpp PRE-CREATION staging/sonnet/src/ui/configui.ui PRE-CREATION staging/sonnet/src/ui/configwidget.h PRE-CREATION staging/sonnet/src/ui/configwidget.cpp PRE-CREATION staging/sonnet/src/ui/dialog.h PRE-CREATION staging/sonnet/src/ui/dialog.cpp PRE-CREATION staging/sonnet/src/ui/dictionarycombobox.h PRE-CREATION staging/sonnet/src/ui/dictionarycombobox.cpp PRE-CREATION staging/sonnet/src/ui/highlighter.h PRE-CREATION staging/sonnet/src/ui/highlighter.cpp PRE-CREATION staging/sonnet/src/ui/sonnetui.ui PRE-CREATION staging/sonnet/tests/CMakeLists.txt 11d32e6 staging/sonnet/tests/test_dialog.h 583b27b Diff: http://git.reviewboard.kde.org/r/107791/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Review Request 109531: Port the Sonnet ASpell plugin to the new style APIs
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109531/ --- Review request for KDE Frameworks and kdelibs. Description --- Port the ASpell plugin to the new style of doing plugins in Qt5. I'll do the rest of the plugins afterwards if this looks okay. Diffs - staging/sonnet/src/core/client_p.h cd5c0f2 staging/sonnet/src/plugins/aspell/kspell_aspellclient.h 95173ff staging/sonnet/src/plugins/aspell/kspell_aspellclient.cpp b649d0d Diff: http://git.reviewboard.kde.org/r/109531/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 107791: move Sonnet out of KDEUI, port away from KDE classes.
Hi! As noone raised any issues for a week now, and to try to move things along a bit, I'll just push it now. -- Martin Sandsmark
Re: Review Request 107791: move Sonnet out of KDEUI, port away from KDE classes.
On Feb. 11, 2013, 12:57 p.m., Aleix Pol Gonzalez wrote: Why are there so many removed files? Shouldn't they be renamed? Maybe you forgot some git mv? (just replying here as well, for posterity or something); I did use git mv, it's just the reviewboard interface not handling it properly, it seems. - Martin Tobias Holmedahl --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107791/#review27196 --- On Feb. 10, 2013, 8:40 p.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107791/ --- (Updated Feb. 10, 2013, 8:40 p.m.) Review request for KDE Frameworks, kdelibs and David Faure. Description --- Ported everything away from KConfig to QSettings. I couldn't really find any users of the ::save function, so I think the source incompatible change would be worth it. Alternatively we could add a deprecated dummy function that takes in a KConfig object and just ignores it, and uses QSettings. Diffs - kdeui/CMakeLists.txt 08faa20 kdeui/sonnet/configdialog.h 4c05f0d kdeui/sonnet/configdialog.cpp af3f9b4 kdeui/sonnet/configui.ui 11dffc0 kdeui/sonnet/configwidget.h 023b659 kdeui/sonnet/configwidget.cpp 549d5af kdeui/sonnet/dialog.h 241bd02 kdeui/sonnet/dialog.cpp 3c6f99f kdeui/sonnet/dictionarycombobox.h 82a807c kdeui/sonnet/dictionarycombobox.cpp 32e0970 kdeui/sonnet/highlighter.h f3bc4af kdeui/sonnet/highlighter.cpp d72b9e0 kdeui/sonnet/sonnetui.ui 03af35b kdeui/sonnet/tests/test_configdialog.cpp 4c4fd21 kdeui/widgets/ktextedit.h d0c1c4d kdeui/widgets/ktextedit.cpp 5bf4741 staging/sonnet/src/CMakeLists.txt 157dfa9 staging/sonnet/src/core/CMakeLists.txt 2f11dc0 staging/sonnet/src/core/backgroundchecker.h f0da3a3 staging/sonnet/src/core/backgroundchecker.cpp dc05b94 staging/sonnet/src/core/globals.cpp bf4f504 staging/sonnet/src/core/loader.cpp bfa56a9 staging/sonnet/src/core/settings.cpp e405c30 staging/sonnet/src/core/settings_p.h e14bad7 staging/sonnet/src/core/speller.h 37dd82f staging/sonnet/src/core/speller.cpp 024d7f7 staging/sonnet/src/ui/configdialog.h PRE-CREATION staging/sonnet/src/ui/configdialog.cpp PRE-CREATION staging/sonnet/src/ui/configui.ui PRE-CREATION staging/sonnet/src/ui/configwidget.h PRE-CREATION staging/sonnet/src/ui/configwidget.cpp PRE-CREATION staging/sonnet/src/ui/dialog.h PRE-CREATION staging/sonnet/src/ui/dialog.cpp PRE-CREATION staging/sonnet/src/ui/dictionarycombobox.h PRE-CREATION staging/sonnet/src/ui/dictionarycombobox.cpp PRE-CREATION staging/sonnet/src/ui/highlighter.h PRE-CREATION staging/sonnet/src/ui/highlighter.cpp PRE-CREATION staging/sonnet/src/ui/sonnetui.ui PRE-CREATION staging/sonnet/tests/CMakeLists.txt 11d32e6 staging/sonnet/tests/test_dialog.h 583b27b Diff: http://git.reviewboard.kde.org/r/107791/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 107791: move Sonnet out of KDEUI, port away from KDE classes.
On Mon, Feb 11, 2013 at 12:57:46PM -, Aleix Pol Gonzalez wrote: Why are there so many removed files? Shouldn't they be renamed? Maybe you forgot some git mv? No, I used git mv, and the new files show up, it's just reviewboard that doesn't show the moves properly. -- Martin Sandsmark
Re: Review Request 107791: move Sonnet out of KDEUI, port away from KDE classes.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107791/ --- (Updated Feb. 10, 2013, 8:40 p.m.) Review request for KDE Frameworks, kdelibs and David Faure. Changes --- fix issues raised so far, and also move UI parts out of kdeui. Summary (updated) - move Sonnet out of KDEUI, port away from KDE classes. Description --- Ported everything away from KConfig to QSettings. I couldn't really find any users of the ::save function, so I think the source incompatible change would be worth it. Alternatively we could add a deprecated dummy function that takes in a KConfig object and just ignores it, and uses QSettings. Diffs (updated) - kdeui/CMakeLists.txt 08faa20 kdeui/sonnet/configdialog.h 4c05f0d kdeui/sonnet/configdialog.cpp af3f9b4 kdeui/sonnet/configui.ui 11dffc0 kdeui/sonnet/configwidget.h 023b659 kdeui/sonnet/configwidget.cpp 549d5af kdeui/sonnet/dialog.h 241bd02 kdeui/sonnet/dialog.cpp 3c6f99f kdeui/sonnet/dictionarycombobox.h 82a807c kdeui/sonnet/dictionarycombobox.cpp 32e0970 kdeui/sonnet/highlighter.h f3bc4af kdeui/sonnet/highlighter.cpp d72b9e0 kdeui/sonnet/sonnetui.ui 03af35b kdeui/sonnet/tests/test_configdialog.cpp 4c4fd21 kdeui/widgets/ktextedit.h d0c1c4d kdeui/widgets/ktextedit.cpp 5bf4741 staging/sonnet/src/CMakeLists.txt 157dfa9 staging/sonnet/src/core/CMakeLists.txt 2f11dc0 staging/sonnet/src/core/backgroundchecker.h f0da3a3 staging/sonnet/src/core/backgroundchecker.cpp dc05b94 staging/sonnet/src/core/globals.cpp bf4f504 staging/sonnet/src/core/loader.cpp bfa56a9 staging/sonnet/src/core/settings.cpp e405c30 staging/sonnet/src/core/settings_p.h e14bad7 staging/sonnet/src/core/speller.h 37dd82f staging/sonnet/src/core/speller.cpp 024d7f7 staging/sonnet/src/ui/configdialog.h PRE-CREATION staging/sonnet/src/ui/configdialog.cpp PRE-CREATION staging/sonnet/src/ui/configui.ui PRE-CREATION staging/sonnet/src/ui/configwidget.h PRE-CREATION staging/sonnet/src/ui/configwidget.cpp PRE-CREATION staging/sonnet/src/ui/dialog.h PRE-CREATION staging/sonnet/src/ui/dialog.cpp PRE-CREATION staging/sonnet/src/ui/dictionarycombobox.h PRE-CREATION staging/sonnet/src/ui/dictionarycombobox.cpp PRE-CREATION staging/sonnet/src/ui/highlighter.h PRE-CREATION staging/sonnet/src/ui/highlighter.cpp PRE-CREATION staging/sonnet/src/ui/sonnetui.ui PRE-CREATION staging/sonnet/tests/CMakeLists.txt 11d32e6 staging/sonnet/tests/test_dialog.h 583b27b Diff: http://git.reviewboard.kde.org/r/107791/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Review Request 108875: Port Sonnet to QPluginLoader
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108875/ --- Review request for KDE Frameworks and kdelibs. Description --- Port Sonnet to use QPluginLoader instead of KServiceTypeTrader/KPluginLoader. Diffs - staging/sonnet/src/core/CMakeLists.txt fcbbbe1 staging/sonnet/src/core/loader.cpp bd64e2a staging/sonnet/src/core/loader_p.h 896a8f8 staging/sonnet/src/plugins/aspell/CMakeLists.txt 2f7c4bd staging/sonnet/src/plugins/enchant/CMakeLists.txt e3365cf staging/sonnet/src/plugins/hspell/CMakeLists.txt 9e7bf13 staging/sonnet/src/plugins/hunspell/CMakeLists.txt 3f8a30c Diff: http://git.reviewboard.kde.org/r/108875/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 108875: Port Sonnet to QPluginLoader
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108875/ --- (Updated Feb. 9, 2013, 4:48 p.m.) Review request for KDE Frameworks and kdelibs. Changes --- remove unused clients list. Description --- Port Sonnet to use QPluginLoader instead of KServiceTypeTrader/KPluginLoader. Diffs (updated) - staging/sonnet/src/core/CMakeLists.txt fcbbbe1 staging/sonnet/src/core/loader.cpp bd64e2a staging/sonnet/src/core/loader_p.h 896a8f8 staging/sonnet/src/plugins/aspell/CMakeLists.txt 2f7c4bd staging/sonnet/src/plugins/enchant/CMakeLists.txt e3365cf staging/sonnet/src/plugins/hspell/CMakeLists.txt 9e7bf13 staging/sonnet/src/plugins/hunspell/CMakeLists.txt 3f8a30c Diff: http://git.reviewboard.kde.org/r/108875/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 108875: Port Sonnet to QPluginLoader
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108875/ --- (Updated Feb. 9, 2013, 5:02 p.m.) Review request for KDE Frameworks and kdelibs. Changes --- remove unused stuff from CMakeLists.txt Description --- Port Sonnet to use QPluginLoader instead of KServiceTypeTrader/KPluginLoader. Diffs (updated) - staging/sonnet/src/core/CMakeLists.txt fcbbbe1 staging/sonnet/src/core/loader.cpp bd64e2a staging/sonnet/src/core/loader_p.h 896a8f8 staging/sonnet/src/plugins/aspell/CMakeLists.txt 2f7c4bd staging/sonnet/src/plugins/enchant/CMakeLists.txt e3365cf staging/sonnet/src/plugins/hspell/CMakeLists.txt 9e7bf13 staging/sonnet/src/plugins/hunspell/CMakeLists.txt 3f8a30c Diff: http://git.reviewboard.kde.org/r/108875/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request 108875: Port Sonnet to QPluginLoader
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108875/ --- (Updated Feb. 9, 2013, 6:30 p.m.) Review request for KDE Frameworks and kdelibs. Changes --- Fixed the issues raised. Description --- Port Sonnet to use QPluginLoader instead of KServiceTypeTrader/KPluginLoader. Diffs (updated) - staging/sonnet/src/core/CMakeLists.txt fcbbbe1 staging/sonnet/src/core/loader.cpp bd64e2a staging/sonnet/src/core/loader_p.h 896a8f8 staging/sonnet/src/plugins/aspell/CMakeLists.txt 2f7c4bd staging/sonnet/src/plugins/aspell/kspell_aspell.desktop aa22c1b staging/sonnet/src/plugins/enchant/CMakeLists.txt e3365cf staging/sonnet/src/plugins/enchant/kspell_enchant.desktop 4cee638 staging/sonnet/src/plugins/hspell/CMakeLists.txt 9e7bf13 staging/sonnet/src/plugins/hspell/kspell_hspell.desktop 929e843 staging/sonnet/src/plugins/hunspell/CMakeLists.txt 3f8a30c staging/sonnet/src/plugins/hunspell/kspell_hunspell.desktop 94ea5ed Diff: http://git.reviewboard.kde.org/r/108875/diff/ Testing --- Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request: KHTML, imload: Fix wrong version of last line of scaled tile
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108246/#review24938 --- Ship it! hmm, I wonder why valgrind didn't pick this up for me, but looks good to me. - Martin Tobias Holmedahl Sandsmark On Jan. 7, 2013, 6:16 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108246/ --- (Updated Jan. 7, 2013, 6:16 p.m.) Review request for kdelibs, Martin Tobias Holmedahl Sandsmark and Maks Orlovich. Description --- Code in scaledLoop32bit() has an off-by-one error in computation of the versions index, causing some images to contain garbage on their last line. This is visible in Konqueror and Akregator intro pages. Diffs - khtml/imload/scaledimageplane.cpp e8ab684 Diff: http://git.reviewboard.kde.org/r/108246/diff/ Testing --- Tested Akregator and Konqueror about pages. Browsed a few websites with Konqueror + KHTML. Screenshots --- Konqueror, before - after http://git.reviewboard.kde.org/r/108246/s/988/ Thanks, Aurélien Gâteau
Re: Review Request: port Sonnet to QSettings
On Thu, Dec 27, 2012 at 12:18:21PM -, Kevin Krammer wrote: Two levels are most likley the most common case because most systems do not have any system administrator. Once you have admin customization you have at least three (package, admin, user). I don't have any evidence for nor against usage of Kiosk features in regard to spell checking, I am just pointing out that the solution proposed here does have none of those. Well, the library in question here does spell checking, so that's what we should focus on. IMHO spell checking doesn't need any of the extra features offered by KConfig. The reuse of the filename sonnetrc suggest to me that the intention is to use the same file. A KConfig based application and a QSettings based application will behave inconsitently if using the same file. Or is this a per-application stored file? Do I use sonnetrc anywhere? In that case I missed something. Do we assume that KDE application developers who's programs are being used in an Enterprise setup will fork the library and reimplement the config with KConfig or do we want them to use the KF5 version? The later will either require that the library does not handle config itself or at least allows applications to intercept config access or provide config that takes precendence over stored config. If they care about that they can just load using whatever config system they want, and set the loaded options manually. I also don't think this is a very likely scenario, tbh. IMHO the most sensible case is to let the application handle config I/O, allowing it to store the config in a way that is most approriate for its usage. If that is a KConfig INI file, a simple QSetting INI file or a JSON file shouldn't really matter to an engine which only is interested in the values. I think the most sensible is to not let the application developers bother their pretty little heads with storing the config at all if they don't want to. With what I have proposed here they can reimplement the config storage if they want to, but by default It Just Works™. -- Martin Sandsmark
Re: Review Request: port Sonnet to QSettings
On Thu, Dec 27, 2012 at 03:13:52PM +0100, Kevin Krammer wrote: Indeed, it should be doing any form of config IO. It can't know whether any of its options need to be persisted, or are supplied hardcoded by the using code or offered to the user but volatile or offered the users and stored persistently or confgured by the system administrator or configured by the system vendor. Due to the old code we can deduce that the option values were potentially stored persistently, allowing user, administrator and system vendor to provide, override and lock-down each of them. But as you said yourself: it does spell checking, it does not have to care about settings persistence at all. So, we could just remove all usage of KConfig/QSettings? We're able to get the default locale for the application (and automatically detect the language once I merge that work), with the changeLanguage() function I think we're pretty well-covered. No idea, maybe they all just hard-code values. The old code's usage of KConfig and the config access in ktextedit.cpp suggests that some applications wanted to settings to stay configurable. In either case nothing the spell checker needs to care about. It uses the values, it has no interest what so ever in how it got them in the first place. Yeah, I couldn't really find any usage of the config storage stuff. My problem with understanding this is: why does the spell checker need the capability to secretly access persistent config? Removing its internal config access will still have it Just Works by default (again assuming that those variables are at least initialized properly). Well, it abused it for one thing in particular; words to ignore. But this should really be handled by whatever spellchecking backend we use (aspell at least has this functionality built in, with proper system/package/user separation etc.). So I looked at Sonnet::Loader which seemed to be referred to a lot regarding settings. Finally, a way to access the settings! Weird thing is that it is declared in a header with _p.h suffix, usually indicating that this is not public API. Even weirder, the same seems to be true for Sonnet::Settings! Yes, Sonnet::Settings isn't supposed to be accessed by applications. From looking at it, it doesn't really contain stuff that should be application-specific (skipping run-together words etc.). Sonnet::BackgroundChecker however, for example, already exposes a method to change language, which is something I think applications might want to change themselves. We could just remove the code that persist this, though? -- Martin Sandsmark
Re: Review Request: port sonnet away from i18nc
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107412/ --- (Updated Dec. 26, 2012, 11:02 p.m.) Review request for kdelibs and David Faure. Changes --- set the right context. Description --- port sonnet away from i18nc. Diffs (updated) - staging/sonnet/src/core/loader.cpp 887aee5 staging/sonnet/src/core/settings.cpp 59cb593 staging/sonnet/src/core/speller.cpp f831f55 Diff: http://git.reviewboard.kde.org/r/107412/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request: port Sonnet to QSettings
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107791/ --- (Updated Dec. 27, 2012, 12:52 a.m.) Review request for KDE Frameworks, kdelibs and David Faure. Changes --- Don't expose QSettings at all (now just in the internal, non-public Settings class). Description --- Ported everything away from KConfig to QSettings. I couldn't really find any users of the ::save function, so I think the source incompatible change would be worth it. Alternatively we could add a deprecated dummy function that takes in a KConfig object and just ignores it, and uses QSettings. Diffs (updated) - kdeui/sonnet/configdialog.h 7c4993b kdeui/sonnet/configdialog.cpp 625441b kdeui/sonnet/configwidget.h 023b659 kdeui/sonnet/configwidget.cpp 549d5af kdeui/sonnet/highlighter.cpp 6cbb14c kdeui/sonnet/tests/test_configdialog.cpp 4c4fd21 kdeui/widgets/ktextedit.h d0c1c4d kdeui/widgets/ktextedit.cpp 71d2a9f staging/sonnet/src/core/backgroundchecker.h f0da3a3 staging/sonnet/src/core/backgroundchecker.cpp dc05b94 staging/sonnet/src/core/globals.cpp bf4f504 staging/sonnet/src/core/loader.cpp 887aee5 staging/sonnet/src/core/settings.cpp 59cb593 staging/sonnet/src/core/settings_p.h e14bad7 staging/sonnet/src/core/speller.h 37dd82f staging/sonnet/src/core/speller.cpp f831f55 Diff: http://git.reviewboard.kde.org/r/107791/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request: port sonnet away from i18nc
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107412/ --- (Updated Dec. 24, 2012, 10:03 p.m.) Review request for kdelibs and David Faure. Changes --- add context, fix whitespace. Description --- port sonnet away from i18nc. Diffs (updated) - staging/sonnet/src/core/loader.cpp 887aee5 staging/sonnet/src/core/settings.cpp 59cb593 staging/sonnet/src/core/speller.cpp f831f55 Diff: http://git.reviewboard.kde.org/r/107412/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request: port sonnet away from i18nc
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107412/ --- (Updated Dec. 23, 2012, 5:32 p.m.) Review request for kdelibs and David Faure. Changes --- use tr() instead of QCoreApplication::translate(). Description --- port sonnet away from i18nc. Diffs (updated) - staging/sonnet/src/core/loader.cpp 887aee5 staging/sonnet/src/core/settings.cpp 59cb593 staging/sonnet/src/core/speller.cpp f831f55 Diff: http://git.reviewboard.kde.org/r/107412/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request: port Sonnet to QSettings
On Dec. 17, 2012, 10:38 p.m., Kevin Krammer wrote: IMHO this is wrong. Not code wise but conceptual. As far as I understand QSettings is basically deprecated, it is just not official marked as such because there is no replacement. This would be porting away from a fully functional, way more advanced and maintained settings. If the sole goal of this endavor is to remove the KConfig dependency than this ought to be done by either having an interface that can be implemented through KConfig or by passing values as QVariant maps or hashes. Oswald Buddenhagen wrote: and where exactly do you see that kconfig maintainer? ;) it's unfortunate that the chosen config class is part of the API. judging by uses, would it be reasonable to just disable that part of the API indefinitely? less drastically, would it be acceptable to pass a file name instead of a config object? that would of course incur some overhead (assuming we are passing the application's main config file). Kevin Krammer wrote: it's unfortunate that the chosen config class is part of the API. Indeed. Most likely out of convenience. Hence the idea to just replace it with a generic key/value object that doesn't do any specific form of I/O and can allowing the using application to decide on persistant storage. But if I understand you correctly you would rather go for the full solution and make those properties directly read-/writable, right? Oswald Buddenhagen wrote: the idea with the file name has the advantage that it is most natural, but sort of slow, and it may actually not work - if the app uses kconfig, but sonnet uses qsettings on the same file, you may actually get garbage out of it. i'd strongly recommend not using a variant map, etc., as using it would require lots of boilerplate code. if you make it so that instantiating is nothing else than new Sonnet::ConfigDialog(new KConfigWrapper(new KConfigGroup(KGlobal::config(), Spellchecking))); it may be ok. still a bit ... weird. you could make kconfiggroup directly implement the interface, but then you'd get an asymmetry with qsettings. also, where would KMapInterface be defined? where would the qsettings wrapper live? or maybe upstream QMapInterface and make QSettings implement it, too? would it even fit the API? What about not exposing any of the config storage details at all? We have the application name, that should be enough to store application specific settings. - Martin Tobias Holmedahl --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107791/#review23643 --- On Dec. 17, 2012, 9:15 p.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107791/ --- (Updated Dec. 17, 2012, 9:15 p.m.) Review request for KDE Frameworks, kdelibs and David Faure. Description --- Ported everything away from KConfig to QSettings. I couldn't really find any users of the ::save function, so I think the source incompatible change would be worth it. Alternatively we could add a deprecated dummy function that takes in a KConfig object and just ignores it, and uses QSettings. Diffs - kdeui/sonnet/configdialog.h 7c4993b kdeui/sonnet/configdialog.cpp 625441b kdeui/sonnet/configwidget.h 023b659 kdeui/sonnet/configwidget.cpp 549d5af kdeui/sonnet/highlighter.cpp 6cbb14c kdeui/widgets/ktextedit.cpp 71d2a9f staging/sonnet/src/core/backgroundchecker.h f0da3a3 staging/sonnet/src/core/backgroundchecker.cpp dc05b94 staging/sonnet/src/core/globals.cpp bf4f504 staging/sonnet/src/core/loader.cpp 887aee5 staging/sonnet/src/core/settings.cpp 59cb593 staging/sonnet/src/core/settings_p.h e14bad7 staging/sonnet/src/core/speller.h 37dd82f staging/sonnet/src/core/speller.cpp f831f55 Diff: http://git.reviewboard.kde.org/r/107791/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request: port sonnet away from i18nc
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107412/ --- (Updated Dec. 17, 2012, 8:05 p.m.) Review request for kdelibs and David Faure. Changes --- I found out that QLocale actually provides everything we needed, and it also parses the locale codes for us so we can simplify a bit. Description --- port sonnet away from i18nc. Diffs (updated) - staging/sonnet/src/core/loader.cpp 887aee5 staging/sonnet/src/core/settings.cpp 59cb593 staging/sonnet/src/core/speller.cpp f831f55 Diff: http://git.reviewboard.kde.org/r/107412/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Review Request: port Sonnet to QSettings
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107791/ --- Review request for kdelibs and David Faure. Description --- Ported everything away from KConfig to QSettings. I couldn't really find any users of the ::save function, so I think the source incompatible change would be worth it. Alternatively we could add a deprecated dummy function that takes in a KConfig object and just ignores it, and uses QSettings. Diffs - kdeui/sonnet/configdialog.h 7c4993b kdeui/sonnet/configdialog.cpp 625441b kdeui/sonnet/configwidget.h 023b659 kdeui/sonnet/configwidget.cpp 549d5af kdeui/sonnet/highlighter.cpp 6cbb14c kdeui/widgets/ktextedit.cpp 71d2a9f staging/sonnet/src/core/backgroundchecker.h f0da3a3 staging/sonnet/src/core/backgroundchecker.cpp dc05b94 staging/sonnet/src/core/globals.cpp bf4f504 staging/sonnet/src/core/loader.cpp 887aee5 staging/sonnet/src/core/settings.cpp 59cb593 staging/sonnet/src/core/settings_p.h e14bad7 staging/sonnet/src/core/speller.h 37dd82f staging/sonnet/src/core/speller.cpp f831f55 Diff: http://git.reviewboard.kde.org/r/107791/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request: port Sonnet to QSettings
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107791/ --- (Updated Dec. 17, 2012, 9:15 p.m.) Review request for KDE Frameworks, kdelibs and David Faure. Description --- Ported everything away from KConfig to QSettings. I couldn't really find any users of the ::save function, so I think the source incompatible change would be worth it. Alternatively we could add a deprecated dummy function that takes in a KConfig object and just ignores it, and uses QSettings. Diffs - kdeui/sonnet/configdialog.h 7c4993b kdeui/sonnet/configdialog.cpp 625441b kdeui/sonnet/configwidget.h 023b659 kdeui/sonnet/configwidget.cpp 549d5af kdeui/sonnet/highlighter.cpp 6cbb14c kdeui/widgets/ktextedit.cpp 71d2a9f staging/sonnet/src/core/backgroundchecker.h f0da3a3 staging/sonnet/src/core/backgroundchecker.cpp dc05b94 staging/sonnet/src/core/globals.cpp bf4f504 staging/sonnet/src/core/loader.cpp 887aee5 staging/sonnet/src/core/settings.cpp 59cb593 staging/sonnet/src/core/settings_p.h e14bad7 staging/sonnet/src/core/speller.h 37dd82f staging/sonnet/src/core/speller.cpp f831f55 Diff: http://git.reviewboard.kde.org/r/107791/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Review Request: port sonnet away from i18nc
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107412/ --- Review request for kdelibs and David Faure. Description --- port sonnet away from i18nc. Diffs - staging/sonnet/src/core/loader.cpp 887aee5 Diff: http://git.reviewboard.kde.org/r/107412/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request: port sonnet away from i18nc
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107412/ --- (Updated Nov. 21, 2012, 7:47 p.m.) Review request for kdelibs and David Faure. Description --- port sonnet away from i18nc. Diffs - staging/sonnet/src/core/loader.cpp 887aee5 Diff: http://git.reviewboard.kde.org/r/107412/diff/ Testing --- it builds. Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request: implement support for window.onhashchange
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104946/ --- (Updated May 15, 2012, 12:56 p.m.) Review request for kdelibs. Changes --- fix issues raised so far. Description --- Implement a custom hashchange event, and make the khtml kpart emit it when we are asked to navigate to another anchor, and the ref is different. Diffs (updated) - khtml/ecma/kjs_events.h 3727b94 khtml/ecma/kjs_events.cpp e7c7e5b khtml/ecma/kjs_html.h 089b550 khtml/ecma/kjs_html.cpp d64e07c khtml/ecma/kjs_window.h 416b045 khtml/ecma/kjs_window.cpp e75e6e7 khtml/html/html_baseimpl.cpp baa13b5 khtml/khtml_part.cpp 24589e4 khtml/misc/htmlattrs.in 21a2363b khtml/misc/htmlnames.h e3adbe7 khtml/misc/htmlnames.cpp 3b22b6d khtml/xml/dom2_eventsimpl.h 5b452d2 khtml/xml/dom2_eventsimpl.cpp f01a533 Diff: http://git.reviewboard.kde.org/r/104946/diff/ Testing --- tested extensively against http://lolcats.iskrembilen.com/ Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
On April 26, 2012, 5:09 p.m., Albert Astals Cid wrote:
If i understand you correctly you are suggesting to create a bug (option
that does nothing)?
Doesn't make much sense.
Dawit Alemayehu wrote:
Huh ? I do not follow. By option that does nothing you mean this change
by itself does nothing that is correct. But that is true of every option in
that dialog as well. Moreover, it is a well known fact that you cannot post a
patch for different components in reviewboard. Anyhow, the option will most
definitely be used by kwebkitpart. Whether or not some body implements
support for it in khtml is a question I cannot answer. That is what I meant.
David Faure wrote:
Do you have the kwebkitpart patch ready?
I suppose it'll be easy to apply to khtml as well.
Albert Astals Cid wrote:
You are suggesting to add an option that does nothing in our default html
rendering engine. That is adding a bug. The fact that you know it and don't
care about it makes me sad.
Dawit Alemayehu wrote:
@David: Yes I have a patch for kwebkitpart, but unlike in khtml adding
support for this in kwebkitpart required a very small change in one place in
addition to adding code to read the config option itself in
webkitettings.{h|cpp}. That does not seem to be the case in khtml. It is a
little bit more invovled.
@Albert: Really ?? So how exactly is another browser engine supposed to
provide configuration option to the user if it is supposed to be embedded
into Konqueror ? Why would a developer working on a separate browsing engine
be forced to implement any functionality into khtml ? Would that requirement
apply the other way around ? The last I checked, this is a konqueror setting.
Whether a specific browser engine supports it or not is up to that browser
engine.Just for the record I almost always go out of my way to implement
things in khtml ; especially when I factor out features that are common to
both engines. In this case, I neither have the time nor a complete grasp of
how the KWallet integration works in khtml. As I stated above the change is
not in a single isolated location like it is in kwebkitpart. Feel free to do
a grep in khtml and see for yourself. I can always add the set/get functions
to access the option in KHTMLSettings, but what use would that be if it is
not implemented.
Anyhow, I digress. If there are objections, I will simply move this
feature into the webkit config module I will have to create down the road.
Albert Astals Cid wrote:
So how exactly is another browser engine supposed to provide
configuration option to the user if it is supposed to be embedded into
Konqueror ?
Having it's own engine-only kcm for it's engine-specific options?
Why would a developer working on a separate browsing engine be forced to
implement any functionality into khtml ?
When did i say that?
Would that requirement apply the other way around ?
If you want to use the general apply to all engines options page, of
course.
You probably don't have any bit of user mentallity left in your head,
because it's pretty obvious that adding a configuration option to web
rendering configuration that won't work with our default rendering engine
it's bad usability.
But hey, since David promised to implement this for khtml, go ahead,
don't let me block progress.
I just briefly looked at how complex it would be to implement in KHTML, and it
seems to be enough with this three-line patch?
diff --git a/khtml/ui/passwordbar/storepassbar.cpp
b/khtml/ui/passwordbar/storepassbar.cpp
index 3f5c392..08d79a9 100644
--- a/khtml/ui/passwordbar/storepassbar.cpp
+++ b/khtml/ui/passwordbar/storepassbar.cpp
@@ -80,6 +80,10 @@ StorePass::~StorePass()
void StorePass::saveLoginInformation(const QString host, const QString key,
const QMapQString, QString walletMap)
{
+ KConfigGroup config( KGlobal::config(), HTML Settings );
+ if (!config.readEntrybool(OfferToSaveWebsitePassword, true))
+return;
+
m_host = host;
m_key = key;
m_walletMap = walletMap;
- Martin Tobias Holmedahl
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104631/#review12934
---
On April 26, 2012, 3:48 a.m., Dawit Alemayehu wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104631/
---
(Updated April 26, 2012, 3:48 a.m.)
Review request for KDE Base Apps, kdelibs and David Faure.
Description
---
A patch to make that provides an option to disable the offer to save website
passwords. Note that for this patch to take effect this option
Review Request: implement support for window.onhashchange
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104946/ --- Review request for kdelibs. Description --- Implement a custom hashchange event, and make the khtml kpart emit it when we are asked to navigate to another anchor, and the ref is different. Diffs - khtml/ecma/kjs_dom.h 38fae65 khtml/ecma/kjs_dom.cpp ab7f02a khtml/ecma/kjs_events.h 3727b94 khtml/ecma/kjs_events.cpp e7c7e5b khtml/ecma/kjs_window.h 416b045 khtml/ecma/kjs_window.cpp e75e6e7 khtml/html/html_baseimpl.cpp baa13b5 khtml/khtml_part.cpp 24589e4 khtml/misc/htmlnames.h e3adbe7 khtml/misc/htmlnames.cpp 3b22b6d khtml/xml/dom2_eventsimpl.h 5b452d2 khtml/xml/dom2_eventsimpl.cpp f01a533 Diff: http://git.reviewboard.kde.org/r/104946/diff/ Testing --- tested extensively against http://lolcats.iskrembilen.com/ Thanks, Martin Tobias Holmedahl Sandsmark
Review Request: bilinear scaling for khtml/imload
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104743/ --- Review request for kdelibs. Description --- Uses bilinear scaling for images. It's a bit prettier, but not much, but should be a reasonable tradeoff for speed. Diffs - khtml/imload/scaledimageplane.h 35fec21 khtml/imload/scaledimageplane.cpp 4977201 Diff: http://git.reviewboard.kde.org/r/104743/diff/ Testing --- ran it against a couple of different images of various sizes. Screenshots --- Before http://git.reviewboard.kde.org/r/104743/s/547/ After http://git.reviewboard.kde.org/r/104743/s/548/ Thanks, Martin Tobias Holmedahl Sandsmark
Re: Review Request: bilinear scaling for khtml/imload
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104743/ --- (Updated April 26, 2012, 6:54 p.m.) Review request for kdelibs. Changes --- merged in the patch that moves the implementation into a separate cpp file. Description --- Uses bilinear scaling for images. It's a bit prettier, but not much, but should be a reasonable tradeoff for speed. Diffs (updated) - khtml/imload/scaledimageplane.h 35fec21 khtml/imload/scaledimageplane.cpp 4977201 Diff: http://git.reviewboard.kde.org/r/104743/diff/ Testing --- ran it against a couple of different images of various sizes. Screenshots --- Before http://git.reviewboard.kde.org/r/104743/s/547/ After http://git.reviewboard.kde.org/r/104743/s/548/ Thanks, Martin Tobias Holmedahl Sandsmark
