Re: [PATCH 2/2] rebase -i: Preserve whitespace at beginning of commit header in $GIT_EDITOR

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

 



Nazri Ramliy <ayiehere@xxxxxxxxx> writes:

> I guess it boils down to your answer to this:
>
> When you want to reword the badly formatted commit header, would you
> prefer the see the output like the one shown in exhibit 1 over that of
> exhibit 2 in your $GIT_EDITOR?

What I was getting at was if you want to add a special case _only_ for
commits that have indented subject line, and do not care about ones that
are missing the subject line, excess whitespace in the middle or at the
end, or no commit log message.  That is why I asked the "What happens"
questions.  For example, if neither the current nor the updated code
removes excess whitespace in the middle or at the end, then that becomes a
non-issue.

I just checked.  You can create "funny commits" of various shape by
misusing hash-object (and presumably importing from foreign SCMs with
custom importers).  Since the topic here is about _fixing_ such mistakes,
seeing how we handle commits that have different but similar breakages is
very relevant before going forward.

 - We do keep excess whitespaces in the middle, so they are easy to spot.

 - A commit without any message is shown without _any_ message in the
   list, so presumably we can identify them (if there are many, you would
   want to fix them all anyway).

 - We drop trailing whitespaces, so it is impossible to spot a commit with
   such a subject line.  The user who wants to fix these breakages in the
   commit log messages presumably is intelligent enough to check with "git
   cat-file commit $that_commit | cat -e" to find such a commit, make a
   mental note of what the log message of the broken commit said, and then
   identify the one with the same message when editing the instruction
   sheet for the rebase-i, so it is not the end of the world, though.

 - A commit with an extra blank line is not shown any differently, so it
   is impossible to identify.  The "remember the message to identify"
   trick would work very well, though.

 - A commit with a space at the begining gets that excess space stripped,
   so it is impossible to identify.  Again, the "remember the message to
   identify" trick would work very well.

So the bigger picture laid out here shows that there are five cases,
including your "SP at the beginning" which is the last one.

Among these five, two are non-issues, and the remaining three share the
same "impossible to spot if you _only_ look at the insn sheet, but it is
easy to remember which ones to edit" characteristics.  Is the change that
only keeps the excess whitespaces at the beginning such an improvement
worth the code churn?

To put it differently, if we really do perceive this "impossible to spot
if you _only_ look at the insn sheet, but it is easy to remember which
ones to edit" as an issue, shouldn't the patch at least mention that it
attempts to solve only one and punts on the other two that are still to be
fixed later (so that other people can come back to help)?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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]