Review Request 129324: VCSCommitDiffPatchSource point changes : set width to 72 characters and use deleteLater()

[René J.V. Bertin]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129324/
-----------------------------------------------------------

(Updated Nov. 4, 2016, 6:07 p.m.)


Review request for KDE Software on Mac OS X and KDevelop.


Changes
-------

Implements most of the suggestions on the original patch, except a migration to a full Kate view.

I've also included some proposed changes to the git highlighter:
- it sets a tooltip summarising the commit message guidelines
- it avoids creating confusion with spell checker indications by no longer using underlining. Instead:
• summary text exceeding the soft limit is set in italic font (and this applies no longer to the full line). IIRC this is a rather classical use of italic to signal an issue.
• text exceeding the hard limit is ~~overlined~~.

I considered using bold face for hard limit exceeding, but bold is already used (for the summary) and apparently it depends on the typeface used whether the hint is respected at all (it's not for the fonts I've tried).


Repository: kdevplatform


Description
-------

This patch proposes 2 point changes to `VCSCommitDiffPatchSource`. They're unrelated but both so small and limited to a single file that I hope you won't mind I put them in a single RR.

In order of appearance:

1- KDE insists on a 72 character limit for commit messages, and that is probably a very reasonable standard. In the current implementation there is no easy way to judge how far from that threshold we are. The easiest way to do this is by setting the commit message editor to a width of 72 characters (which for some reason requires taking the width of a string of 74 characters). I think there is room enough for this, but a more complete implementation would probably introduce a resize handle between the 2 main widgets. I haven't yet tried to look into that.

2- The commit message has been a very likely source of the crashes I've seen in the past (on Mac) when closing patch reviews, and which have been addressed by using `QObject::deleteLater()` for select widgets. That change was somehow never submitted for the commit message editor.


Diffs (updated)
-----

  plugins/git/gitmessagehighlighter.cpp da7660d 
  vcs/widgets/vcsdiffpatchsources.cpp 42cf1d8 

Diff: https://git.reviewboard.kde.org/r/129324/diff/


Testing
-------

On OS X with 2 different fixed font typefaces (sizes). The deleteLater() change has been in use since about 4.7.1 .


File Attachments
----------------

preview: 72-char wide commit message editor
  https://git.reviewboard.kde.org/media/uploaded/files/2016/11/03/2ffdd73a-db19-4bf2-9545-7a850a4b7fd1__Screen_Shot_2016-11-03_at_18.24.54.png


Thanks,

René J.V. Bertin

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20161104/cd877c56/attachment.html>