Automatic word wrapping of long lines in commit text Gerrit Code Review?

507 views
Skip to first unread message

Niels Dekker

unread,
Oct 27, 2017, 10:07:21 AM10/27/17
to Repo and Gerrit Discussion
Yesterday I did a "git gerrit-push" to get a patch of mine reviewed for http://itk.org/ITK.git  Unfortunately it appeared rather inconvenient to read my commit text as it had a line of more than 200 characters, which was not wrapped: http://review.source.kitware.com/#/c/22749/1

Does Gerrit support automatic word wrapping of long lines in commit text?

The Gerrit version at http://review.source.kitware.com is "Gerrit Code Review (2.12-1-g6f7dc21)"

Kind regards, Niels


zivkov

unread,
Oct 27, 2017, 10:15:16 AM10/27/17
to Repo and Gerrit Discussion


On Friday, October 27, 2017 at 4:07:21 PM UTC+2, Niels Dekker wrote:
Yesterday I did a "git gerrit-push" to get a patch of mine reviewed for http://itk.org/ITK.git  Unfortunately it appeared rather inconvenient to read my commit text as it had a line of more than 200 characters

It is indeed inconvenient to read such long lines.
Please take care to not exceed 72-80 characters line width in your commit messages.
Don't expect every (Git) tool to word wrap long lines.

which was not wrapped: http://review.source.kitware.com/#/c/22749/1

Does Gerrit support automatic word wrapping of long lines in commit text?

No, I don't think so.

We give CR-1 on changes where lines in commit message are too long and the author has to
fix them and upload a new patch-set.

Niels Dekker

unread,
Oct 28, 2017, 4:41:49 AM10/28/17
to Repo and Gerrit Discussion
Thank you for your prompt reply, zivkov.


On Friday, October 27, 2017 at 4:15:16 PM UTC+2, zivkov wrote:
Please take care to not exceed 72-80 characters line width in your commit messages.
Don't expect every (Git) tool to word wrap long lines.

I think it's a pity that in general those tools still don't seem to be able to wrap long lines automatically.


Does Gerrit support automatic word wrapping of long lines in commit text?

No, I don't think so.

It would have been a nice feature...
 

We give CR-1 on changes where lines in commit message are too long and the author has to
fix them and upload a new patch-set.

What do you mean by CR-1? 

Kind regards, Niels

Magnus Bäck

unread,
Oct 30, 2017, 2:24:04 AM10/30/17
to Niels Dekker, Repo and Gerrit Discussion
On Friday, October 27, 2017 at 17:00 CEST,
Niels Dekker <niels.dekker...@gmail.com> wrote:

> On Friday, October 27, 2017 at 4:15:16 PM UTC+2, zivkov wrote:
>
> > Please take care to not exceed 72-80 characters line width in your
> > commit messages.
> > Don't expect every (Git) tool to word wrap long lines.
>
> I think it's a pity that in general those tools still don't seem to be
> able to wrap long lines automatically.

I believe the idea is that the text editor that you're using to compose
your commit message should do that wrapping.

> > > Does Gerrit support automatic word wrapping of long lines in
> > > commit text?
> >
> > No, I don't think so.
>
> It would have been a nice feature...

When would that line wrapping take place? Keep in mind that the commit
message affects the commit SHA-1, so line-wrapping commit messages on
the fly will have consequences.

> We give CR-1 on changes where lines in commit message are too long and
> the author has to fix them and upload a new patch-set.
>
> What do you mean by CR-1?

A Code-Review -1 vote.

--
Magnus Bäck | Software Engineer, Development Tools
magnu...@axis.com | Axis Communications

David Pursehouse

unread,
Oct 30, 2017, 4:10:01 AM10/30/17
to Niels Dekker, Repo and Gerrit Discussion
On Mon, Oct 30, 2017 at 3:24 PM Magnus Bäck <magnu...@axis.com> wrote:
On Friday, October 27, 2017 at 17:00 CEST,
     Niels Dekker <niels.dekker...@gmail.com> wrote:

> On Friday, October 27, 2017 at 4:15:16 PM UTC+2, zivkov wrote:
>
> > Please take care to not exceed 72-80 characters line width in your
> > commit messages.
> > Don't expect every (Git) tool to word wrap long lines.
>
> I think it's a pity that in general those tools still don't seem to be
> able to wrap long lines automatically.

I believe the idea is that the text editor that you're using to compose
your commit message should do that wrapping.

> > > Does Gerrit support automatic word wrapping of long lines in
> > > commit text?
> >
> > No, I don't think so.
>
> It would have been a nice feature...

When would that line wrapping take place? Keep in mind that the commit
message affects the commit SHA-1, so line-wrapping commit messages on
the fly will have consequences.

Wrapping the lines in the UI, which was the desired behaviour if I understood the original post correctly, would not have such consequences.  That would only be the case if Gerrit were to actually rewrite the commit message creating a new patch set on the change.

 

> We give CR-1 on changes where lines in commit message are too long and
> the author has to fix them and upload a new patch-set.
>
> What do you mean by CR-1?

A Code-Review -1 vote.

--
Magnus Bäck          | Software Engineer, Development Tools
magnu...@axis.com | Axis Communications

--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Niels Dekker

unread,
Oct 30, 2017, 5:29:11 AM10/30/17
to Repo and Gerrit Discussion
Thank you all for your replies so far.


David Pursehouse wrote:
Wrapping the lines in the UI, which was the desired behaviour if I understood the original post correctly, would not have such consequences.

Yes indeed, that's what I would like  :-)  I would like the Gerrit Code Review web page to display long lines in a word wrapped manner, basically as CSS style "overflow-wrap: normal", as described at https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-wrap


Kind regards, Niels

Edwin Kempin

unread,
Oct 30, 2017, 5:37:33 AM10/30/17
to Niels Dekker, Repo and Gerrit Discussion
On Mon, Oct 30, 2017 at 10:29 AM, Niels Dekker <niels.dekker...@gmail.com> wrote:
Thank you all for your replies so far.

David Pursehouse wrote:
Wrapping the lines in the UI, which was the desired behaviour if I understood the original post correctly, would not have such consequences.

Yes indeed, that's what I would like  :-)  I would like the Gerrit Code Review web page to display long lines in a word wrapped manner,
I don't think this is a good idea. If Gerrit would auto-wrap the commit message, it's easy for reviewers to miss that the commit message is badly formatted and then you have a mess when you do 'git log' later after this change was merged, but then it's too late to fix.
 
basically as CSS style "overflow-wrap: normal", as described at https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-wrap


Kind regards, Niels

--
--
To unsubscribe, email repo-discuss+unsubscribe@googlegroups.com

More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss+unsubscribe@googlegroups.com.

David Ostrovsky

unread,
Oct 30, 2017, 6:29:03 AM10/30/17
to Repo and Gerrit Discussion

Am Montag, 30. Oktober 2017 10:37:33 UTC+1 schrieb Edwin Kempin:


On Mon, Oct 30, 2017 at 10:29 AM, Niels Dekker <niels.dekker...@gmail.com> wrote:
Thank you all for your replies so far.

David Pursehouse wrote:
Wrapping the lines in the UI, which was the desired behaviour if I understood the original post correctly, would not have such consequences.

Yes indeed, that's what I would like  :-)  I would like the Gerrit Code Review web page to display long lines in a word wrapped manner,
I don't think this is a good idea. If Gerrit would auto-wrap the commit message, it's easy for reviewers to miss that the commit message is badly formatted and then you have a mess when you do 'git log' later after this change was merged, but then it's too late to fix.

But line warp in commit message is implemented in the PG UI, e.g.: [1], [2].
These CLs are discussed in these issues, correspondingly: [3], [4].
Switching to the GWT UI, would restore the original (long line) commit message.

Reply all
Reply to author
Forward
0 new messages