D5139: VCS commit message width feedback

Milian Wolff noreply at phabricator.kde.org
Mon Mar 27 20:20:06 UTC 2017


mwolff requested changes to this revision.
mwolff added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> gitmessagehighlighter.cpp:38
>      QTextCharFormat format;
> -    format.setFontUnderline(true);
> -    format.setUnderlineStyle(QTextCharFormat::SpellCheckUnderline);
> -    format.setUnderlineColor(warning ? Qt::yellow : Qt::red);
> +    format.setFontLetterSpacing(125);
> +    if (!warning) {

I don't get these changes, why do you change the spacing? what has this to do with width? please expand your commit message and add screenshots that show the effect of this before/after

from my POV, the previous code looks better, adding underlines in different color is much better than messing around with letter spacing and italics

> vcsdiffpatchsources.cpp:47
>  
> +class VCSCommitMessageEditor : public KTextEdit {
> +    Q_OBJECT

move { to its own line

> vcsdiffpatchsources.cpp:79
> +    // Given the widget margins that requires 74 actual characters.
> +    editor->setMinWidth(editor->fontMetrics().width(QString(74, 'm')));
> +    m_vcs->setupCommitMessageEditor(updater->url(), editor);

use `editor->fontMetrics().maxWidth() * 74`

REPOSITORY
  R33 KDevPlatform

REVISION DETAIL
  https://phabricator.kde.org/D5139

To: rjvbb, #kdevelop, mwolff
Cc: mwolff, kdevelop-devel, #kdevelop, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170327/9280d1ac/attachment.html>


More information about the KDevelop-devel mailing list