Re: [PATCH] hooks: replace irrelevant hook sample

[Kaartic Sivaraam]

On Fri, 2017-07-07 at 08:05 -0700, Junio C Hamano wrote:
> That is because I wear multiple hats, because I try to help in
> different ways, and because open source is not a battle to see whose
> idea is more right, but is a cooperative process to find a better
> solution together.
> 
Thanks for helping and being kind!

> As a fellow contributor, I do not think that removing the hint that
> is commented out, which is meant to be helpful to users while in
> their editor and which will be removed after the editor finishes
> anyway, is a useful enough example to keep the now otherwise useless
> sample hook.  But as the maintainer, I can see that you are still
> making sincere efforts to come up with a useful example and improve
> the end-user experience, and more importantly, I haven't heard from
> other people what they think---the only thing I have are different
> opinions from two people.  That is why I am not deciding and telling
> you to go find another area to hack in.
> 
> At the same time, I found that your implementation of the idea, i.e.
> removal of the commented-out hint, can be improved.  With an
> improved implementation of the proposed solution, it may have a
> better chance to be supported by others on the list, and equally
> importantly, if it turns out that other people do support what this
> patch tries to do, i.e. keep the sample hook alive by replacing the
> now useless examples with this one, we would have a better
> implementation of it.  And that is something I can help with, while
> I, the maintainer, is waiting.
> 
So, I'll improve it and of course wait for any replies. BTW, thanks for
clearing off the little confusion I had. I'm not used to these
multiple-hat replies from single person. Thanks for exposing me to it.

> Oh, by the way, what the maintainer is waiting for is not just "me
> too"s; this is not exactly a "having more people wins" democracy.
Got it.

-- 
Quote: “The most valuable person on any team is the person who makes
everyone else on the team more valuable”

Regards,
Kaartic

Re: [PATCH] hooks: replace irrelevant hook sample

[Junio C Hamano]

Kaartic Sivaraam  writes:

> That said, in case my interpretation that "'prepare-commit-msg' hook is
> not to be shipped due to it's uselessness" is correct, the reply of
> this mail as a whole seems to contradict it.

That is because I wear multiple hats, because I try to help in
different ways, and because open source is not a battle to see whose
idea is more right, but is a cooperative process to find a better
solution together.

As a fellow contributor, I do not think that removing the hint that
is commented out, which is meant to be helpful to users while in
their editor and which will be removed after the editor finishes
anyway, is a useful enough example to keep the now otherwise useless
sample hook.  But as the maintainer, I can see that you are still
making sincere efforts to come up with a useful example and improve
the end-user experience, and more importantly, I haven't heard from
other people what they think---the only thing I have are different
opinions from two people.  That is why I am not deciding and telling
you to go find another area to hack in.

At the same time, I found that your implementation of the idea, i.e.
removal of the commented-out hint, can be improved.  With an
improved implementation of the proposed solution, it may have a
better chance to be supported by others on the list, and equally
importantly, if it turns out that other people do support what this
patch tries to do, i.e. keep the sample hook alive by replacing the
now useless examples with this one, we would have a better
implementation of it.  And that is something I can help with, while
I, the maintainer, is waiting.

Oh, by the way, what the maintainer is waiting for is not just "me
too"s; this is not exactly a "having more people wins" democracy.

Re: [PATCH] hooks: replace irrelevant hook sample

[Kaartic Sivaraam]

On Wed, 2017-07-05 at 12:50 -0700, Junio C Hamano wrote:
> Three things that caught my eyes:
> 
>  - Between "git commit --cleanup=strip" and "git commit --
> cleanup=verbatim",
>    lines that make up this initial instruction section are different.
> 
>  - "git grep 'Please enter the '" finds that this string is subject
>    to translation, so the pattern may not match (in which case it
>    will be a no-op without doing any harm, which is OK).
> 
>  - core.commentChar can be set to something other than '#', so the
>    pattern may not match (I do not offhand know if that may cause a
>    wrong line to match, causing harm, or not).
> 
> As merely an example, it probably is OK to say "this won't work if
> you are not using the C locale, and/or you are using custom
> core.commentChar".  So if we disregard the latter two, I would think
> 
> sed -e '/^# Please enter the commit message /,/^#$/d'
> 
> may be simpler to reason about to achieve the same goal.  
> 
Thanks for enlightening me about this. I thought sed was greedy with
address spaces the same way it's greedy with regex.

sed -e '/^# Please enter the commit message /,/^#$/d'


This command does seem to work regardless of the cleanup mode used.

That said, in case my interpretation that "'prepare-commit-msg' hook is
not to be shipped due to it's uselessness" is correct, the reply of
this mail as a whole seems to contradict it.

Should I work on this patch and another related one (he one that
modifies the signature part of the hook) or
should I drop it ?

IOW, would this patch likely make the hook useful again?

-- 
Kaartic

Re: [PATCH] hooks: replace irrelevant hook sample

[Junio C Hamano]

Kaartic Sivaraam  writes:

> +sed -e '/^# Please enter the .*/ {
> +  N
> +  N
> +  d
> +}' "$1" >'.sed-output.temp' && mv '.sed-output.temp' "$1"

Three things that caught my eyes:

 - Between "git commit --cleanup=strip" and "git commit --cleanup=verbatim",
   lines that make up this initial instruction section are different.

 - "git grep 'Please enter the '" finds that this string is subject
   to translation, so the pattern may not match (in which case it
   will be a no-op without doing any harm, which is OK).

 - core.commentChar can be set to something other than '#', so the
   pattern may not match (I do not offhand know if that may cause a
   wrong line to match, causing harm, or not).

As merely an example, it probably is OK to say "this won't work if
you are not using the C locale, and/or you are using custom
core.commentChar".  So if we disregard the latter two, I would think

sed -e '/^# Please enter the commit message /,/^#$/d'

may be simpler to reason about to achieve the same goal.  

Thanks.