Re: Review Request 110430: [Taskbar applet] Added actions to be perfomed on middle click
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110430/#review32545 --- I am not in favour of this patch for a couple of reasons. First, there is a port underway to QML which does not need to be delayed further by adding more features. More importantly, however, middle click is a special case of not the first two mouse buttons (should we support N button mice?) and it adds yet more configuration to an already complex and full configuration dialog. It also conflicts with the meaning of middle click to expand / collapse groups (a fairly hidden feature I also was not particularly happy with tbh). - Aaron J. Seigo On May 14, 2013, 10:35 p.m., Albert Vaca Cintora wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110430/ --- (Updated May 14, 2013, 10:35 p.m.) Review request for kde-workspace and Plasma. Description --- This patch adds a feature present in KDE3 and requested by some users (see open bugs), that is performing an action when a taskbar entry is middle-clicked. I have added three possible actions: Close the application, hide it or move it to the current desktop, where the first two were user requests. This addresses bugs 182689 and 190951. http://bugs.kde.org/show_bug.cgi?id=182689 http://bugs.kde.org/show_bug.cgi?id=190951 Diffs - plasma/desktop/applets/tasks/tasks.h fe79a12 plasma/desktop/applets/tasks/tasks.cpp 0a86dcf plasma/desktop/applets/tasks/tasksConfig.ui 6f3ff18 plasma/desktop/applets/tasks/windowtaskitem.cpp f840076 Diff: http://git.reviewboard.kde.org/r/110430/diff/ Testing --- Manual testing. Thanks, Albert Vaca Cintora
Re: Review Request 110043: Proposed fix/workaround for legacy encoded filename handling
On May 7, 2013, 10:11 a.m., Róbert Szókovács wrote: The solution is intentionally shy, I really don't want to fan the flames surrounding this issue. I just stumbled upon this location when it can be handled painlessly. Whether or not it should be turned on by default, in my opinion, can be left for distributors. Thomas Lübking wrote: Then it's worthless. When I encounter broken filenames on a rw device, i know it's time for a fix. When I encounter broken filenames in joliet or rockridge (latter usually caused by myself long ago - thank you, wodim...) i know it's time to mount norock/nojoliet. Whether i do that or set a (KDE only affecting) env makes hardly a difference. When my little sister™ encounters broken filenames anywhere, she knows that it's time to call her personal IT (me) with these files won't open! - if she could not call me, she had no access to those files. Period. She won't think to google for kde broken filenames, because she would not think it's a problem with the name - the files have weird names, yes, but essentially they won't open when she clicks them. That this could be due to some restrictions in UTF-8 and QString and other terms she does not know, cannot be an expected consideration. So either this is not a fixworthy issue at all, or it (as OPT-IN) only becomes a way for distro discrimination (works on distro X but not on distro Y) because fact is that the filenames are broken and if we want to assist in that situation, we assist the unskilled *only* and the unskilled simply dont set env vars. If they did, they were also skilled enough for convmv et al. to deal with that issue correctly. IOW *every* distro but Arch/Gentoo/LFS - ie. where you read a wiki for setup - likely would *have* to set this anyway and those have the users to turn it off at will. /2¢ Róbert Szókovács wrote: OK, I'm all for making this on by default, but that would be a change from the current situation, when the default is QFile's filename encoding, basen on locale. If this becomes the default, it disrupts those who use a non-UTF8 locale. The current code provides an enviroment variable to force KDE to threat the filenames UTF8, this patch piggybacks that mechanism. Should we check the locale the same way QFile does? Thomas Lübking wrote: There should be no regression in regular use on non broken FS names for no-one - not even those using non UTF-8 locales, so yes - testing the locale to dis/enable this sounds reasonable. Is the solution as simple as deactivating it if the tested env is set to anything but non_broken_names? No, I'm affraid we would need heuristics similar to the one in QT, see qtextcodec.cpp, setupLocaleMapper(): Get the first nonempty value from $LC_ALL, $LC_CTYPE, and $LANG environment variables., then check the CODESET part; if it's UTF8, enable this new functionality, otherwise do as before the patch. - Róbert --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110043/#review32184 --- On May 7, 2013, 4:14 p.m., Róbert Szókovács wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110043/ --- (Updated May 7, 2013, 4:14 p.m.) Review request for kdelibs and Thiago Macieira. Description --- This patch works around the problem of filenames that are not valid UTF8 strings: in KLocalePrivate::initFileNameEncoding() KDE sets the QFile's encoding/decoding function, to to/fromUTF8() in QString, which in turn calls QUtf8's converter function (QUtf8 is not exported to developers, so I had to use an inefficient method, I think it would be better if we could use the state parameter for error detection). I replaced this with the said functions' copy/pasted version and changed it, so when it encounters an invalid UTF8 string, it will encode it byte by byte, mapping the lower 128 their normal unicode place and the upper 128 to U+18000-U+1807F, and of course the decoder reverses it. To make this actually work you have to define the KDE_UTF8_FILENAMES enviroment variable to a specific value (broken_names). To test it, do the following: .kde/env/KDE_UTF8_FILENAMES.sh with this content: export KDE_UTF8_FILENAMES=broken_names logout, login, try dolphin on faulty files. (instead of the usual boxed ? you'll see just boxes) This addresses bug 165044. http://bugs.kde.org/show_bug.cgi?id=165044 Diffs - kdecore/localization/klocale_kde.cpp b010e74 kdecore/localization/klocale_p.h af4a768 Diff:
Re: Review Request 110430: [Taskbar applet] Added actions to be perfomed on middle click
On May 15, 2013, 6:06 a.m., Aaron J. Seigo wrote: I am not in favour of this patch for a couple of reasons. First, there is a port underway to QML which does not need to be delayed further by adding more features. More importantly, however, middle click is a special case of not the first two mouse buttons (should we support N button mice?) and it adds yet more configuration to an already complex and full configuration dialog. It also conflicts with the meaning of middle click to expand / collapse groups (a fairly hidden feature I also was not particularly happy with tbh). Hello Aaron and thank for your reply. Let me defend the inclusion of this patch from the problems you mention: - Difficulty to port to QML: This feature is implemented in a ~10 lines switch (not counting the GUI-related code), so porting it should not be a problem (I could do it, if needed). - Support for N button mice: The desktop should adapt to the current hardware, and nowadays most mice have 3 buttons (not N, but neither only 2). Moreover, lots of apps have adopted the behaviour of closing tabs with a middle click, so adding this feature for applications in the taskbar is consistent with them. - Complexity of the configuration dialog: I agree that we should try to keep all the configuration dialogs simple, but not adding new features because of that reminds me of Gnome3 ;) I think this is not solution for the long-discussed problem of the user-friendliness. Finally, and more important, it has 2 open bugs (+ 4 duplicates) so there are real users requesting it. If it's not going to be added, those bugs should be marked as wontfix. - Albert --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110430/#review32545 --- On May 14, 2013, 10:35 p.m., Albert Vaca Cintora wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110430/ --- (Updated May 14, 2013, 10:35 p.m.) Review request for kde-workspace and Plasma. Description --- This patch adds a feature present in KDE3 and requested by some users (see open bugs), that is performing an action when a taskbar entry is middle-clicked. I have added three possible actions: Close the application, hide it or move it to the current desktop, where the first two were user requests. This addresses bugs 182689 and 190951. http://bugs.kde.org/show_bug.cgi?id=182689 http://bugs.kde.org/show_bug.cgi?id=190951 Diffs - plasma/desktop/applets/tasks/tasks.h fe79a12 plasma/desktop/applets/tasks/tasks.cpp 0a86dcf plasma/desktop/applets/tasks/tasksConfig.ui 6f3ff18 plasma/desktop/applets/tasks/windowtaskitem.cpp f840076 Diff: http://git.reviewboard.kde.org/r/110430/diff/ Testing --- Manual testing. Thanks, Albert Vaca Cintora
Re: Review Request 110327: KMessageWidget: Remove decoration icon
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110327/#review32564 --- This review has been submitted with commit c8264896312e244e028be291dc390363d3aad843 by Aurélien Gâteau to branch master. - Commit Hook On May 15, 2013, 9:45 a.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110327/ --- (Updated May 15, 2013, 9:45 a.m.) Review request for Dolphin, Kate and kdelibs. Description --- This avoids confusion between the decoration icon and the close button, especially when type is KMessageWidget::Error. This happens for example with Dolphin when an error happens while trying to connect to an non available host. This change also has the nice side-effect of leaving more space for the widget text. Diffs - kdeui/widgets/kmessagewidget.h 80a9980e0af63185e032ca759800bb9fb32b1557 kdeui/widgets/kmessagewidget.cpp 724da0fa60a7b236d61685257e011fc49a30c1ff Diff: http://git.reviewboard.kde.org/r/110327/diff/ Testing --- Tested with kmessagewidgetdemo, Dolphin and Kate. Thanks, Aurélien Gâteau
Re: Review Request 110327: KMessageWidget: Remove decoration icon
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110327/ --- (Updated May 15, 2013, 1:54 p.m.) Status -- This change has been marked as submitted. Review request for Dolphin, Kate and kdelibs. Description --- This avoids confusion between the decoration icon and the close button, especially when type is KMessageWidget::Error. This happens for example with Dolphin when an error happens while trying to connect to an non available host. This change also has the nice side-effect of leaving more space for the widget text. Diffs - kdeui/widgets/kmessagewidget.h 80a9980e0af63185e032ca759800bb9fb32b1557 kdeui/widgets/kmessagewidget.cpp 724da0fa60a7b236d61685257e011fc49a30c1ff Diff: http://git.reviewboard.kde.org/r/110327/diff/ Testing --- Tested with kmessagewidgetdemo, Dolphin and Kate. Thanks, Aurélien Gâteau
Re: Review Request 110430: [Taskbar applet] Added actions to be perfomed on middle click
On May 15, 2013, 6:06 a.m., Aaron J. Seigo wrote: I am not in favour of this patch for a couple of reasons. First, there is a port underway to QML which does not need to be delayed further by adding more features. More importantly, however, middle click is a special case of not the first two mouse buttons (should we support N button mice?) and it adds yet more configuration to an already complex and full configuration dialog. It also conflicts with the meaning of middle click to expand / collapse groups (a fairly hidden feature I also was not particularly happy with tbh). Albert Vaca Cintora wrote: Hello Aaron and thank for your reply. Let me defend the inclusion of this patch from the problems you mention: - Difficulty to port to QML: This feature is implemented in a ~10 lines switch (not counting the GUI-related code), so porting it should not be a problem (I could do it, if needed). - Support for N button mice: The desktop should adapt to the current hardware, and nowadays most mice have 3 buttons (not N, but neither only 2). Moreover, lots of apps have adopted the behaviour of closing tabs with a middle click, so adding this feature for applications in the taskbar is consistent with them. - Complexity of the configuration dialog: I agree that we should try to keep all the configuration dialogs simple, but not adding new features because of that reminds me of Gnome3 ;) I think this is not solution for the long-discussed problem of the user-friendliness. Finally, and more important, it has 2 open bugs (+ 4 duplicates) so there are real users requesting it. If it's not going to be added, those bugs should be marked as wontfix. porting it should not be a problem (I could do it, if needed). that is definitely a point in your favor. assuming this feature addition is accepted: there's little point to putting it in the c++ version, however, only to put it later in the qml version (which is supposed to be in 4.11). so putting it directly in the QML version would make the most sense. Moreover, lots of apps have adopted the behaviour of closing tabs with a middle click to point out the obvious: windows are not tabs. this also implies that middle click to close is actually a *good* feature. i think it is for power users .. but i have seen too many instances where these kinds of magic click that button and magically something happens features lead to confusion, and confusion leads to distrust of the system as a whole. yes, the default is to do nothing in your patch (+1 for that), but if sitting down to someone else's system results in wildly unpredictable behaviour ... keep in mind this is not a random component, but a default part of every plasma desktop installation, so it has to work better than most. how much faster is middle click versus right-click-close? fast enough to warrant the risk of surprising behaviour for people who aren't expecting it? Complexity of the configuration dialog: I agree that we should try to keep all the configuration dialogs simple currently that page has 11 settings. ad-hoc testing i've done in the past hints that many of these settings are already not understandable to many users. i really don't want to make this worse. several of the plasmoids have room for more options. the taskbar, folderview, clock and system tray are four that really don't. adding feature over feature is leading to an increasing mess that nobody cares to clean up. if this patch introduced a re-think of the configuration presentation so that it is easier to understand and more approachable, i would consider that a fair trade for accepting a feature i'm not particularly in favour of :) but not adding new features because of that reminds me of Gnome3 for future reference: when i see this kind of statement made, i usually add -1 to my overall weighting. i do not agree with many design decisions in other projects, but design can not be done well if it is primarily directed by not being that other group. common sense and reasoning should be applied to each case without the justification of not like them, otherwise we just create the opposite kind of error. it has 2 open bugs (+ 4 duplicates) so there are real users requesting it. for any product with a large enough user base, given enough time and an open feature request tracker, any possible feature will be requested (usually multiple times). this means that any feature, regardless of intrinsic value, can be justified using this argument. (the least useful case of this i've seen is when people submit the feature request, then later a patch and then use the feature request as evidence it is wanted ;) - Aaron J. --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110430/#review32545
Re: Review Request 110430: [Taskbar applet] Added actions to be perfomed on middle click
On May 15, 2013, 8:06 a.m., Aaron J. Seigo wrote: I am not in favour of this patch for a couple of reasons. First, there is a port underway to QML which does not need to be delayed further by adding more features. More importantly, however, middle click is a special case of not the first two mouse buttons (should we support N button mice?) and it adds yet more configuration to an already complex and full configuration dialog. It also conflicts with the meaning of middle click to expand / collapse groups (a fairly hidden feature I also was not particularly happy with tbh). Albert Vaca Cintora wrote: Hello Aaron and thank for your reply. Let me defend the inclusion of this patch from the problems you mention: - Difficulty to port to QML: This feature is implemented in a ~10 lines switch (not counting the GUI-related code), so porting it should not be a problem (I could do it, if needed). - Support for N button mice: The desktop should adapt to the current hardware, and nowadays most mice have 3 buttons (not N, but neither only 2). Moreover, lots of apps have adopted the behaviour of closing tabs with a middle click, so adding this feature for applications in the taskbar is consistent with them. - Complexity of the configuration dialog: I agree that we should try to keep all the configuration dialogs simple, but not adding new features because of that reminds me of Gnome3 ;) I think this is not solution for the long-discussed problem of the user-friendliness. Finally, and more important, it has 2 open bugs (+ 4 duplicates) so there are real users requesting it. If it's not going to be added, those bugs should be marked as wontfix. Aaron J. Seigo wrote: porting it should not be a problem (I could do it, if needed). that is definitely a point in your favor. assuming this feature addition is accepted: there's little point to putting it in the c++ version, however, only to put it later in the qml version (which is supposed to be in 4.11). so putting it directly in the QML version would make the most sense. Moreover, lots of apps have adopted the behaviour of closing tabs with a middle click to point out the obvious: windows are not tabs. this also implies that middle click to close is actually a *good* feature. i think it is for power users .. but i have seen too many instances where these kinds of magic click that button and magically something happens features lead to confusion, and confusion leads to distrust of the system as a whole. yes, the default is to do nothing in your patch (+1 for that), but if sitting down to someone else's system results in wildly unpredictable behaviour ... keep in mind this is not a random component, but a default part of every plasma desktop installation, so it has to work better than most. how much faster is middle click versus right-click-close? fast enough to warrant the risk of surprising behaviour for people who aren't expecting it? Complexity of the configuration dialog: I agree that we should try to keep all the configuration dialogs simple currently that page has 11 settings. ad-hoc testing i've done in the past hints that many of these settings are already not understandable to many users. i really don't want to make this worse. several of the plasmoids have room for more options. the taskbar, folderview, clock and system tray are four that really don't. adding feature over feature is leading to an increasing mess that nobody cares to clean up. if this patch introduced a re-think of the configuration presentation so that it is easier to understand and more approachable, i would consider that a fair trade for accepting a feature i'm not particularly in favour of :) but not adding new features because of that reminds me of Gnome3 for future reference: when i see this kind of statement made, i usually add -1 to my overall weighting. i do not agree with many design decisions in other projects, but design can not be done well if it is primarily directed by not being that other group. common sense and reasoning should be applied to each case without the justification of not like them, otherwise we just create the opposite kind of error. it has 2 open bugs (+ 4 duplicates) so there are real users requesting it. for any product with a large enough user base, given enough time and an open feature request tracker, any possible feature will be requested (usually multiple times). this means that any feature, regardless of intrinsic value, can be justified using this argument. (the least useful case of this i've seen is when people submit the feature request, then later a patch and then use the feature request as evidence it is wanted ;) if this patch
Re: Review Request 110430: [Taskbar applet] Added actions to be perfomed on middle click
On May 15, 2013, 6:06 a.m., Aaron J. Seigo wrote: I am not in favour of this patch for a couple of reasons. First, there is a port underway to QML which does not need to be delayed further by adding more features. More importantly, however, middle click is a special case of not the first two mouse buttons (should we support N button mice?) and it adds yet more configuration to an already complex and full configuration dialog. It also conflicts with the meaning of middle click to expand / collapse groups (a fairly hidden feature I also was not particularly happy with tbh). Albert Vaca Cintora wrote: Hello Aaron and thank for your reply. Let me defend the inclusion of this patch from the problems you mention: - Difficulty to port to QML: This feature is implemented in a ~10 lines switch (not counting the GUI-related code), so porting it should not be a problem (I could do it, if needed). - Support for N button mice: The desktop should adapt to the current hardware, and nowadays most mice have 3 buttons (not N, but neither only 2). Moreover, lots of apps have adopted the behaviour of closing tabs with a middle click, so adding this feature for applications in the taskbar is consistent with them. - Complexity of the configuration dialog: I agree that we should try to keep all the configuration dialogs simple, but not adding new features because of that reminds me of Gnome3 ;) I think this is not solution for the long-discussed problem of the user-friendliness. Finally, and more important, it has 2 open bugs (+ 4 duplicates) so there are real users requesting it. If it's not going to be added, those bugs should be marked as wontfix. Aaron J. Seigo wrote: porting it should not be a problem (I could do it, if needed). that is definitely a point in your favor. assuming this feature addition is accepted: there's little point to putting it in the c++ version, however, only to put it later in the qml version (which is supposed to be in 4.11). so putting it directly in the QML version would make the most sense. Moreover, lots of apps have adopted the behaviour of closing tabs with a middle click to point out the obvious: windows are not tabs. this also implies that middle click to close is actually a *good* feature. i think it is for power users .. but i have seen too many instances where these kinds of magic click that button and magically something happens features lead to confusion, and confusion leads to distrust of the system as a whole. yes, the default is to do nothing in your patch (+1 for that), but if sitting down to someone else's system results in wildly unpredictable behaviour ... keep in mind this is not a random component, but a default part of every plasma desktop installation, so it has to work better than most. how much faster is middle click versus right-click-close? fast enough to warrant the risk of surprising behaviour for people who aren't expecting it? Complexity of the configuration dialog: I agree that we should try to keep all the configuration dialogs simple currently that page has 11 settings. ad-hoc testing i've done in the past hints that many of these settings are already not understandable to many users. i really don't want to make this worse. several of the plasmoids have room for more options. the taskbar, folderview, clock and system tray are four that really don't. adding feature over feature is leading to an increasing mess that nobody cares to clean up. if this patch introduced a re-think of the configuration presentation so that it is easier to understand and more approachable, i would consider that a fair trade for accepting a feature i'm not particularly in favour of :) but not adding new features because of that reminds me of Gnome3 for future reference: when i see this kind of statement made, i usually add -1 to my overall weighting. i do not agree with many design decisions in other projects, but design can not be done well if it is primarily directed by not being that other group. common sense and reasoning should be applied to each case without the justification of not like them, otherwise we just create the opposite kind of error. it has 2 open bugs (+ 4 duplicates) so there are real users requesting it. for any product with a large enough user base, given enough time and an open feature request tracker, any possible feature will be requested (usually multiple times). this means that any feature, regardless of intrinsic value, can be justified using this argument. (the least useful case of this i've seen is when people submit the feature request, then later a patch and then use the feature request as evidence it is wanted ;) Martin
Re: Review Request 110084: Make ftp error messages prettier
On April 19, 2013, 1:13 p.m., David Faure wrote: Well, yes, it looks nicer to my eyes because I'm not very good at german (and your before screenshot is in german) :-P More seriously I'm not sure this will always work... what if the error message is much longer? Shouldn't it start on its own line so we don't confuse it with the kio_ftp-provided sentence, in general? Thomas Lübking wrote: What esp. should not happen is to just print 421 but indicate that this is Error #421 or similar. More 2¢: For more visual appeal i'd suggest to emphasize the upper (main) line (h2 or maybe just b), get rid of the empty line(s? - trailing one) and ideally get the close button top aligned (esp. since it almost looks like the icon...) Kai Uwe Broulik wrote: I have no idea how long that message can actually get and if it will always start with the error number. I only really encounter the too many connections message, but there are reports about other messages on BKO. @Thomas: that's out of the scope of kio, it's Dolphin's way of presenting error messages, so Frank is the person you want for this ;) but yes, using a labeled Close button like Kate does it might help. Ping? @Thomas: The close button confusion is no longer present since the error icon got removed from the message widget ;) - Kai Uwe --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110084/#review31289 --- On May 7, 2013, 7:48 a.m., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110084/ --- (Updated May 7, 2013, 7:48 a.m.) Review request for kdelibs and David Faure. Description --- Remove linebreaks and trim the message so the ugly linebreak at the end disappears. This addresses bug 318079. http://bugs.kde.org/show_bug.cgi?id=318079 Diffs - kioslave/ftp/ftp.cpp b0868d8 Diff: http://git.reviewboard.kde.org/r/110084/diff/ Testing --- Works and looks much nicer :) File Attachments Before http://git.reviewboard.kde.org/media/uploaded/files/2013/04/19/ftpmessagebefore.png After http://git.reviewboard.kde.org/media/uploaded/files/2013/04/19/ftpmsgafter.png Thanks, Kai Uwe Broulik
Re: Review Request 110084: Make ftp error messages prettier
On April 19, 2013, 1:13 p.m., David Faure wrote: Well, yes, it looks nicer to my eyes because I'm not very good at german (and your before screenshot is in german) :-P More seriously I'm not sure this will always work... what if the error message is much longer? Shouldn't it start on its own line so we don't confuse it with the kio_ftp-provided sentence, in general? Thomas Lübking wrote: What esp. should not happen is to just print 421 but indicate that this is Error #421 or similar. More 2¢: For more visual appeal i'd suggest to emphasize the upper (main) line (h2 or maybe just b), get rid of the empty line(s? - trailing one) and ideally get the close button top aligned (esp. since it almost looks like the icon...) Kai Uwe Broulik wrote: I have no idea how long that message can actually get and if it will always start with the error number. I only really encounter the too many connections message, but there are reports about other messages on BKO. @Thomas: that's out of the scope of kio, it's Dolphin's way of presenting error messages, so Frank is the person you want for this ;) but yes, using a labeled Close button like Kate does it might help. Kai Uwe Broulik wrote: Ping? @Thomas: The close button confusion is no longer present since the error icon got removed from the message widget ;) I posted to that ReviewRequest :-P Maybe have the network icon instead? - Thomas --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110084/#review31289 --- On May 7, 2013, 7:48 a.m., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110084/ --- (Updated May 7, 2013, 7:48 a.m.) Review request for kdelibs and David Faure. Description --- Remove linebreaks and trim the message so the ugly linebreak at the end disappears. This addresses bug 318079. http://bugs.kde.org/show_bug.cgi?id=318079 Diffs - kioslave/ftp/ftp.cpp b0868d8 Diff: http://git.reviewboard.kde.org/r/110084/diff/ Testing --- Works and looks much nicer :) File Attachments Before http://git.reviewboard.kde.org/media/uploaded/files/2013/04/19/ftpmessagebefore.png After http://git.reviewboard.kde.org/media/uploaded/files/2013/04/19/ftpmsgafter.png Thanks, Kai Uwe Broulik
Re: KF5 Update Meeting 2013-w20
On Wednesday 15 May 2013, Kevin Ottens wrote:
Hello all,
So yesterday 4pm (Paris timezone :p) as planned we had the first update
is it planned to have those meetings sometimes also later in the day ?
At 4pm I'm at work, and so can't attend.
...
Action items:
* [ervin] Send meeting reminders on mondays
* [dfaure] Import QCommandLineArgs in qt5kdestaging
* [dfaure + ervin] Discuss the upcoming KIO split
* [steveire] Look into the SOVERSION issue, and cleanup the CMake files
I had a look at KConfigWidgets. It has
ecm_setup_version(5 0 0 VARIABLE_PREFIX ITEMVIEWS
VERSION_HEADER kconfigwidgets_version.h
PACKAGE_VERSION_FILE KConfigWidgetsConfigVersion.cmake)
i.e. the macro will setup the variables
ITEMVIEWS_VERSION_(MAJOR|MINOR|PATCH)
(as documented in ECMSetupVersion.cmake) which is most probably not wanted,
and then uses
set_target_properties(KConfigWidgets PROPERTIES
VERSION ${KCONFIGWIDGETS_VERSION_STRING}
SOVERSION ${KCONFIGWIDGETS_SOVERSION}
)
for setting the version on the library. Since those variables are empty, cmake
sees:
set_target_properties(KConfigWidgets PROPERTIES
VERSION SOVERSION
)
(I finally decided to git and build my qt completely from scratch again, so
soon I may be able to work again on frameworks)
For the cleanup, my plan would be to get rid of
* ECMVersion.cmake
* ECMQtFramework.cmake
* ECMQtFrameworkConfig.cmake.in
* ECMWriteVersionHeader.cmake
I'm also not too excited about ECMMarkAsTest.cmake and
ECMMarkNonGuiExecutable.cmake, since both basically only wrap a call to
set_target_properties().
In all the CMakeLists.txt in the test directories there is very similar code
for generating the test executables, refactoring that into a generically
useful macro would be nice.
Beside that, I would like if we could do a release of extra-cmake-modules as
soon as possible, so other projects, KDE and non-KDE can start to make use of
it and people can start to contribute.
* [steveire] Write a CMake for frameworks guideline in the wiki
* [ervin] Talk to Ben about having our own qt5.git with a kf5 branch
* [notmart] Look at cleaning up the CMake files in plasma-framework (if
time permits)
Last time I looked it didn't look too bad.
As long as kdelibs is only partly split some somewhat ugly things have to stay
around I think.
Do you have anything special in mind ?
Alex
Re: Review Request 110084: Make ftp error messages prettier
On April 19, 2013, 1:13 p.m., David Faure wrote: Well, yes, it looks nicer to my eyes because I'm not very good at german (and your before screenshot is in german) :-P More seriously I'm not sure this will always work... what if the error message is much longer? Shouldn't it start on its own line so we don't confuse it with the kio_ftp-provided sentence, in general? Thomas Lübking wrote: What esp. should not happen is to just print 421 but indicate that this is Error #421 or similar. More 2¢: For more visual appeal i'd suggest to emphasize the upper (main) line (h2 or maybe just b), get rid of the empty line(s? - trailing one) and ideally get the close button top aligned (esp. since it almost looks like the icon...) Kai Uwe Broulik wrote: I have no idea how long that message can actually get and if it will always start with the error number. I only really encounter the too many connections message, but there are reports about other messages on BKO. @Thomas: that's out of the scope of kio, it's Dolphin's way of presenting error messages, so Frank is the person you want for this ;) but yes, using a labeled Close button like Kate does it might help. Kai Uwe Broulik wrote: Ping? @Thomas: The close button confusion is no longer present since the error icon got removed from the message widget ;) Thomas Lübking wrote: I posted to that ReviewRequest :-P Maybe have the network icon instead? I cannot control this from KIO, and also Dolphin itself doesn't really allow setting a custom icon (doesn't even let you control the message type for the widget) that is that contextual (ie. network errors or so). And this is not subject of this review anyway. - Kai Uwe --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110084/#review31289 --- On May 7, 2013, 7:48 a.m., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110084/ --- (Updated May 7, 2013, 7:48 a.m.) Review request for kdelibs and David Faure. Description --- Remove linebreaks and trim the message so the ugly linebreak at the end disappears. This addresses bug 318079. http://bugs.kde.org/show_bug.cgi?id=318079 Diffs - kioslave/ftp/ftp.cpp b0868d8 Diff: http://git.reviewboard.kde.org/r/110084/diff/ Testing --- Works and looks much nicer :) File Attachments Before http://git.reviewboard.kde.org/media/uploaded/files/2013/04/19/ftpmessagebefore.png After http://git.reviewboard.kde.org/media/uploaded/files/2013/04/19/ftpmsgafter.png Thanks, Kai Uwe Broulik
Re: Review Request 108442: [High-dpi issues] Fix KIconButton initial icon size and its occurence in KPropertiesDialog
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108442/ --- (Updated May 15, 2013, 7:37 p.m.) Review request for kdelibs. Changes --- Use screen dpi to calculate the button size Description --- The KIconButton and the other occurences assume that the icon size for desktop icons is always 48x48. This assumption is wrong. This patch makes KPropertiesDialog use the proper IconSize. There are other places that need fixing too (eg. Dolphin's Place edit dialog or KMenuEdit) which I will fix later as well. So, with KDE Frameworks at the horizon and kdelibs frozen, does this mean, when I am re-writing the KIconDialog to be more userfriendly, use an UI file, introduce new strings, etc this cannot go into master but only frameworks branch? Diffs (updated) - kio/kfile/kicondialog.cpp 73a7449 kio/kfile/kpropertiesdialog.cpp b4cd8ee Diff: http://git.reviewboard.kde.org/r/108442/diff/ Testing --- Yup, you won't notice any difference with default settings but with higher icon size and font scales perfectly and looks good. See screenshot. File Attachments KPropertiesDialog with Retina settings http://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/iconbutton.png Thanks, Kai Uwe Broulik
