Re: [PATCH/RFC] commit-template: improve readability of commit template

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2017-06-27 at 10:56 -0700, Junio C Hamano wrote:
> Kaartic Sivaraam <kaarticsivaraam91196@xxxxxxxxx> writes:
> > I thought it's not good to trade-off readability for vertical space
> > as
> > the ultimate aim of the commit template (at least to me) is to
> > convey
> > information to the user about the commit that he's going to make.
> > For
> > which, I thought it made more sense to improve it's readability by
> > adding new lines between different sections rather than constrain
> > the
> > output within a few lines.
> 
> You have to be careful when making a trade-off argument.  It depends
> on how familiar you already are with the presentation.  Those who
> are/got used to the order of things that come, they will know there
> is extra information when the block of lines are longer than usual
> without reading every character and then their eyes are guided to
> read what is extra, without having to waste precious screen real
> estate.  Nobody will _stay_ a new user who is not yet familiar with
> the everyday output.
> 
You're right. I didn't consider the fact that experienced users would
be affected as a result of this change, sorry about that. I thought,
making this change would help the new users who would possibly find the
commit template to be congested and let experienced users to get
accustomed to this new output format. I thought this change would be a
win-win (at least after people get accustomed to the new formatting). 

In case screen real estate is considered more important here, no
issues. I'll drop that part of the change, happily.

> > I actually didn't think of modifying that in order to keep it in
> > line
> > with the output of `git status`.
> 
> I was (and still am) assuming that if we make this change to "git
> commit", we should make matching change to "git status" as a given.
I get it now. In that case, I don't think making the change would be a
good choice for the following reasons,

    * I think vertical spacing matters more in the output printed to a
    console.
    * I myself find it odd to add a new line below the branch
    information possibly because I'm too accustomed to it's current
    output.

I tried adding the new line, it seemed to be too spacious. It might be
just me in this case.

> > Further, to me, adding *this* new line
> > before the "Changes not staged for commit" (or something in it's
> > place)
> > seems to be wasting some vertical space ...
> 
> I think it is in line with your original reasoning why you wanted
> these extra blank lines to separate blocks of different kinds of
> information:
> 
>  - "Please do this" instruction at the beginning
>  - Make sure you know the default is --only, not --include
>  - By the way you are committing for that person, not you
>  - This change is being committed on that branch
>  - Here are the changes that are already in the index
>  - Here are the changes that are not in the index
>  - Here are untracked files
> 
> Lack of a blank between the fourth block and the fifth block [*1*]
> makes it somewhat inconsistent, doesn't it?
> 
It does, for the given set of blocks. I didn't find it inconsistent as
I thought the separate blocks as follows,

 - "Please do this" instruction at the beginning
 - Make sure you know the default is --only, not --include
 - By the way you are committing for that person, not you
 - Status of repository (git status)

> [Footnote]
> 
> *1* Yes, we should think about removing the optional second block,
>     as I think that it outlived its usefulness; if we are to do so,
>     these become the third and the fourth blocks.
If I interpreted your previous email correctly, I thought we were doing
it!

I'll send a "typical" patch as a follow-up of this mail.

-- 
Regards,
Kaartic Sivaraam <kaarticsivaraam91196@xxxxxxxxx>



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux