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

[Johannes Schindelin <johannes.schindelin@gmx.de>]

Hi Mike,

On 2015-06-11 16:02, Mike Rappazzo wrote:
> On Thu, Jun 11, 2015 at 9:40 AM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
>>
>> On 2015-06-11 03:30, Michael Rappazzo wrote:
>>
>>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>>> index dc3133f..6d14315 100644
>>> --- a/git-rebase--interactive.sh
>>> +++ b/git-rebase--interactive.sh
>>> @@ -740,10 +740,19 @@ collapse_todo_ids() {
>>>  # "pick sha1 fixup!/squash! msg" appears in it so that the latter
>>>  # comes immediately after the former, and change "pick" to
>>>  # "fixup"/"squash".
>>> +#
>>> +# Note that if the config has specified a custom instruction format
>>> +# each log message will be re-retrieved in order to normalize the
>>> +# autosquash arrangement
>>>  rearrange_squash () {
>>>       # extract fixup!/squash! lines and resolve any referenced sha1's
>>> -     while read -r pick sha1 message
>>> +     while read -r pick sha1 todo_message
>>>       do
>>> +             message=${todo_message}
>>
>> Why not just leave the `read -r pick sha1 message` as-is and simply write
>>
>>                 # For "autosquash":
>>                 test -z "$format" ||
>>                 message="$(git log -n 1 --format="%s" $sha1)"
>>
>> here?
> 
> I did notice that I am not using '$todo_message' in the first loop at
> all, so I will adjust it.  In the second loop, I do use both the
> original and the reformatted.  I will apply your suggestion there if
> applicable.

It might make sense to use

                 if test -z "$format"
                 then
                         oneline="$message"
                 else
                         oneline="$(git log -n 1 --format="%s" $sha1)"
                 fi

in the instances where you need to compare against the original oneline, and then only adjust the "message" variable name in places that require the original oneline.

>> [The two test functions are] copied almost verbatim, except for the commit message. The code would be easier to maintain if it did not repeat so much code e.g. by refactoring out a function that takes the commit message as a parameter.
> 
> Makes sense.  I'll implement that.

Thank you,
Johannes