Re: Review Request 129281: [Konsole] Render text at primary font's baseline
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129281/ --- (Updated Dec. 7, 2016, 7:18 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Konsole. Changes --- Submitted with commit 7e2f9d839f5af447a0fc7fd178dd5d8f58e489bb by Christoph Feck to branch master. Bugs: 371687 http://bugs.kde.org/show_bug.cgi?id=371687 Repository: konsole Description --- When Konsole draws a line of text, it first computes the rectangle of the line that the text covers (taking into account cells width and height), then passes this rectangle to the drawText(QRect, flags, text) call. Qt detects if the selected font does not offer all characters in the text, and substitutes individual characters with a different font. Due to designer choices, the same font point size does not lead to same pixel height (or ascent size) in all fonts, so the substituted characters might be larger than the characters from the primary font. Using a rectangle causes Qt to position glyphs relative to the bounding box of the text, instead of anchored to the baseline. This patch uses a pixel position instead of a rectangle to draw the text, taking into account only the baseline of the primary font. I have added all "frameworks" developers to increase possible test coverage. Diffs - src/TerminalDisplay.cpp 39a8b84 Diff: https://git.reviewboard.kde.org/r/129281/diff/ Testing --- On my system, lines with substituted Unicode characters are no longer shifted away from the baseline, and therefore do not appear cropped. Further testing is needed, as there are many (equivalent, similar, or different) bug reports about font rendering on different systems, see https://bugs.kde.org/buglist.cgi?bug_status=UNCONFIRMED_status=CONFIRMED=font=konsole Thanks, Christoph Feck
Review Request 129623: klauncher: remove unnecessary newline from error message
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129623/ --- Review request for KDE Frameworks. Repository: kinit Description --- I don't see a reason why this error message should end with a line break. This just looks bad, see for example http://i.imgur.com/26Y6Xix.png (note that the message in this screenshot has also other problems, see https://git.reviewboard.kde.org/r/129625/) Diffs - src/klauncher/klauncher.cpp f1727df17d29882d7631f8e89e672d67d1d47add Diff: https://git.reviewboard.kde.org/r/129623/diff/ Testing --- Thanks, Elvis Angelaccio
Review Request 129625: Fix errorString() messages from Slave::createSlave()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129625/ --- Review request for KDE Frameworks and David Faure. Repository: kio Description --- This patch is an attempt to fix this ugly error message: http://i.imgur.com/26Y6Xix.png The problem is that in KIO::buildErrorString() the ERR_CANNOT_LAUNCH_PROCESS case assumes that the `errorText` starts with the name of a process (example usage in [kio_smb](https://lxr.kde.org/source/kde/kdenetwork/kio-extras/smb/kio_smb_mount.cpp#0110)). However, Slave::createSlave() also uses ERR_CANNOT_LAUNCH_PROCESS but with "self-contained" error messages, which results in the unfortunate screenshot above. There is no easy way to fix this without breaking the error messages from the client slaves, so I propose to add a new error code ERR_CANNOT_CREATE_SLAVE which is used only by Slave::createSlave() (at least for now). Diffs - src/core/global.h 3d38436fbe28f4236b6ca34a651c1805295c02f2 src/core/job_error.cpp a67954cc44be6994992167700315321761ce0528 src/core/slave.cpp 4690ed50d3f5d28bdaeee84f85107ab75cfab555 Diff: https://git.reviewboard.kde.org/r/129625/diff/ Testing --- See screenshots before/after. File Attachments Before https://git.reviewboard.kde.org/media/uploaded/files/2016/12/07/a4a6468a-0376-4979-9af0-8a11229acf69__26Y6Xix.png After https://git.reviewboard.kde.org/media/uploaded/files/2016/12/07/b534602e-39eb-44bf-915d-046110ed0e78__Screenshot_20161207_194933.png Thanks, Elvis Angelaccio
Re: Review Request 129588: http slave: add newlines to long error message
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129588/ --- (Updated Dec. 7, 2016, 11:11 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Changes --- Submitted with commit e5a4a221e312b9ba72f8cf889172a13af7b06c76 by Elvis Angelaccio to branch master. Repository: kio Description --- Otherwise Dolphin shows an error dialog with too much width. Diffs - src/ioslaves/http/http.cpp 62eb09d2882097c3dea99ea73fd1933cea4376a7 Diff: https://git.reviewboard.kde.org/r/129588/diff/ Testing --- See screenshots. File Attachments Before https://git.reviewboard.kde.org/media/uploaded/files/2016/11/29/bb368736-5987-4800-996f-2e05c4971d7f__Spectacle.a29637.png After https://git.reviewboard.kde.org/media/uploaded/files/2016/11/29/456f0afe-d192-4e09-8c9a-1fa0b27f5bb4__6wJEi10.png Thanks, Elvis Angelaccio
Re: Review Request 129607: Display Application version in About dialog header
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129607/ --- (Updated Dec. 7, 2016, 11:31 a.m.) Review request for KDE Frameworks, KDE Usability and Sebastian Kügler. Repository: kxmlgui Description --- Last year, a change in kaboutapplicationdialog (review 124133) moved the application version in a separate tab. However for applications, it is really annoying (several Kdenlive users complained and I fully agree) to not have this information clearly displayed as soon as the About dialog is displayed. I understand the interest of having a separate version tab displaying the various underlying libraries used, but I propose to display again the application version in the About dialog header (see screenshot). Diffs - src/kaboutapplicationdialog.cpp 156410e Diff: https://git.reviewboard.kde.org/r/129607/diff/ Testing --- Reverts a recent change, tested and works. File Attachments (updated) About dialog with version https://git.reviewboard.kde.org/media/uploaded/files/2016/12/03/c02f4767-2a74-4c25-8599-de410678867e__about.png Updated patch https://git.reviewboard.kde.org/media/uploaded/files/2016/12/03/ee13cb47-2162-4be7-8255-3dce7f470c2b__about2.png renamed to libraries https://git.reviewboard.kde.org/media/uploaded/files/2016/12/07/f01ed0c3-1391-48cf-a153-a177d9a7b54a__about3.png Thanks, Jean-Baptiste Mardelle
Re: Review Request 129607: Display Application version in About dialog header
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129607/ --- (Updated Dec. 7, 2016, 11:30 a.m.) Review request for KDE Frameworks, KDE Usability and Sebastian Kügler. Changes --- Renamed tab to "Libraries", removed "Using" in tab text Repository: kxmlgui Description --- Last year, a change in kaboutapplicationdialog (review 124133) moved the application version in a separate tab. However for applications, it is really annoying (several Kdenlive users complained and I fully agree) to not have this information clearly displayed as soon as the About dialog is displayed. I understand the interest of having a separate version tab displaying the various underlying libraries used, but I propose to display again the application version in the About dialog header (see screenshot). Diffs (updated) - src/kaboutapplicationdialog.cpp 156410e Diff: https://git.reviewboard.kde.org/r/129607/diff/ Testing --- Reverts a recent change, tested and works. File Attachments About dialog with version https://git.reviewboard.kde.org/media/uploaded/files/2016/12/03/c02f4767-2a74-4c25-8599-de410678867e__about.png Updated patch https://git.reviewboard.kde.org/media/uploaded/files/2016/12/03/ee13cb47-2162-4be7-8255-3dce7f470c2b__about2.png Thanks, Jean-Baptiste Mardelle
