Re: [PATCH v4] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-12 Thread Junio C Hamano
Mike Rappazzo  writes:

> In the second loop, I changed it to recalculate the presented message
> when the re-ordered commit is added:
>
> +   if test -n "${format}"
> +   then
> +msg_content=$(git log -n 1 --format="${format}" ${squash})
>
> That is the "$rest".

Ahh, yeah, I missed that --format=${format} thing.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-12 Thread Mike Rappazzo
On Fri, Jun 12, 2015 at 6:05 PM, Junio C Hamano  wrote:
> But because you overwrite the $message variable you read from the
> original insn sheet (which uses the custom format) and compute $rest
> based on the default "%s" and store that in "$1.sq", lines in
> "$1.sq" do not know anything about the custom format, do they?
>
> And then they are injected to appropriate places in "$1.rearranged".
> Moved lines in the the rearranged result would end up written in the
> default "%s" format, no?
>
> That was the part that made me uneasy.
>
> I do not think that is a bug worth fixing, but I view it as a sign
> that fundamentally the autosquash and the idea of configurable
> format do not mesh well with each other.

My understanding of the rearrange_squash function is this:
There are two loops.  The first loop collects the commits which should
be moved (squashed).  The second loop re-constructs the instruction
list using the info from the first loop.

In the second loop, I changed it to recalculate the presented message
when the re-ordered commit is added:

+   if test -n "${format}"
+   then
+msg_content=$(git log -n 1 --format="${format}" ${squash})


That is the "$rest".

I have patched my locally installed `git-rebase--interactive` with
these changes, and I did see the proper rearrangement of commits with
the custom formatted message.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-12 Thread Junio C Hamano
Mike Rappazzo  writes:

> On Fri, Jun 12, 2015 at 4:56 PM, Junio C Hamano  wrote:
> ...
>> The autosquash part somehow makes me feel uneasy, though.  The
>> feature fundamentally has to have %s as the first thing in the
>> format to work, but by making the format overridable, you are
>> potentially breaking that feature, aren't you?
>
> It only needs the '%s' for the autosquash when the todo/instruction
> list order is determined.  For this, in the rearrange_squash function,
> it will re-calculate the message:
>
> +   test -z "${format}" || message=$(git log -n 1
> --format="%s" ${sha1})
>
> Additionally, it may also rerun the log command when preparing the final list.

Yeah, I noticed that when I took a diff between v3 and v4.  I didn't
mean by "break" that you may end up reorder lines incorrectly.

But because you overwrite the $message variable you read from the
original insn sheet (which uses the custom format) and compute $rest
based on the default "%s" and store that in "$1.sq", lines in
"$1.sq" do not know anything about the custom format, do they?

And then they are injected to appropriate places in "$1.rearranged".
Moved lines in the the rearranged result would end up written in the
default "%s" format, no?

That was the part that made me uneasy.

I do not think that is a bug worth fixing, but I view it as a sign
that fundamentally the autosquash and the idea of configurable
format do not mesh well with each other.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-12 Thread Mike Rappazzo
It only needs the '%s' for the autosquash when the todo/instruction
list order is determined.  For this, in the rearrange_squash function,
it will re-calculate the message:

+   test -z "${format}" || message=$(git log -n 1
--format="%s" ${sha1})

Additionally, it may also rerun the log command when preparing the final list.

It is possible that this could be made more efficient by separating
the list arrangement from the list presentation.  I can look into that
for a future patch.

I did add a test which uses the instructionFormat config, and then
interactively auto-squashes using both a 'squash! ' and a
'squash! '. in the commits.


On Fri, Jun 12, 2015 at 4:56 PM, Junio C Hamano  wrote:
> Michael Rappazzo  writes:
>
>> A config option 'rebase.instructionFormat' can override the
>> default 'oneline' format of the rebase instruction list.
>>
>> Since the list is parsed using the left, right or boundary mark plus
>> the sha1, they are prepended to the instruction format.
>>
>> Signed-off-by: Michael Rappazzo 
>> ---
>>  Documentation/git-rebase.txt |  7 +++
>>  git-rebase--interactive.sh   | 20 +---
>>  t/t3415-rebase-autosquash.sh | 21 +
>>  3 files changed, 45 insertions(+), 3 deletions(-)
>
> Thanks, will replace.
>
> The autosquash part somehow makes me feel uneasy, though.  The
> feature fundamentally has to have %s as the first thing in the
> format to work, but by making the format overridable, you are
> potentially breaking that feature, aren't you?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] git-rebase--interactive.sh: add config option for custom instruction format

2015-06-12 Thread Junio C Hamano
Michael Rappazzo  writes:

> A config option 'rebase.instructionFormat' can override the
> default 'oneline' format of the rebase instruction list.
>
> Since the list is parsed using the left, right or boundary mark plus
> the sha1, they are prepended to the instruction format.
>
> Signed-off-by: Michael Rappazzo 
> ---
>  Documentation/git-rebase.txt |  7 +++
>  git-rebase--interactive.sh   | 20 +---
>  t/t3415-rebase-autosquash.sh | 21 +
>  3 files changed, 45 insertions(+), 3 deletions(-)

Thanks, will replace.

The autosquash part somehow makes me feel uneasy, though.  The
feature fundamentally has to have %s as the first thing in the
format to work, but by making the format overridable, you are
potentially breaking that feature, aren't you?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html