Re: [PATCH] git-rebase--interactive.sh: add config option for custom instruction format
Hi Mike,
On 2015-06-11 16:02, Mike Rappazzo wrote:
> On Thu, Jun 11, 2015 at 9:40 AM, Johannes Schindelin
> 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
--
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] git-rebase--interactive.sh: add config option for custom instruction format
On Thu, Jun 11, 2015 at 9:40 AM, Johannes Schindelin
wrote:
> Hi Michael,
>
> 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.
>
>> diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
>> + git rebase --autosquash -i HEAD^^^ &&
>
> We usually write HEAD~3 instead of HEAD^^^...
>
Sure, I'll adjust it. I personally usually use up to 3 '^' and then
switch to '~' for > 3
>
> [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.
--
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] git-rebase--interactive.sh: add config option for custom instruction format
Hi Michael,
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?
> diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
> index 41370ab..1ef96eb 100755
> --- a/t/t3415-rebase-autosquash.sh
> +++ b/t/t3415-rebase-autosquash.sh
> @@ -250,4 +250,37 @@ test_expect_success 'squash! fixup!' '
> test_auto_fixup_fixup squash fixup
> '
>
> +test_expect_success 'autosquash with custom inst format matching on sha1' '
> + git reset --hard base &&
> + git config --add rebase.instructionFormat "[%an @ %ar] %s" &&
> + echo 1 >file1 &&
> + git add -u &&
> + test_tick &&
> + git commit -m "squash! $(git rev-parse --short HEAD^)" &&
> + git tag final-shasquash-instFmt &&
> + test_tick &&
> + git rebase --autosquash -i HEAD^^^ &&
We usually write HEAD~3 instead of HEAD^^^...
> + git log --oneline >actual &&
> + test_line_count = 3 actual &&
> + git diff --exit-code final-shasquash-instFmt &&
> + test 1 = "$(git cat-file blob HEAD^:file1)" &&
> + test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)
> +'
> +
> +test_expect_success 'autosquash with custom inst format matching on comment'
> '
> + git reset --hard base &&
> + git config --add rebase.instructionFormat "[%an @ %ar] %s" &&
> + echo 1 >file1 &&
> + git add -u &&
> + test_tick &&
> + git commit -m "squash! $(git log -n 1 --format=%s HEAD^)" &&
> + git tag final-comment-squash-instFmt &&
> + test_tick &&
> + git rebase --autosquash -i HEAD^^^ &&
> + git log --oneline >actual &&
> + test_line_count = 3 actual &&
> + git diff --exit-code final-comment-squash-instFmt &&
> + test 1 = "$(git cat-file blob HEAD^:file1)" &&
> + test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)
> +'
That is 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.
Ciao,
Johannes
--
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] git-rebase--interactive.sh: add config option for custom
Mike Rappazzo writes:
> On Mon, Jun 8, 2015 at 11:28 AM, Junio C Hamano wrote:
>
>> This is optional, but I still wonder why the command line cannot be
>> more like this, though:
>>
>> format=$(git config --get rebase.insnFormat)
>> git log --format="%H ${format-%s}" --reverse --right-only
>> --topo-order \
>> $revisions ${restrict_revision+^$restrict_revision} |
>> while read -r sha1 junk
>> do
>> ...
>>
>> That way we can optimize one "sed" process away.
>>
>> If this is a good idea, it needs to be a separate follow-up patch
>> that changes "%m filtered by sed" to "use --right-only". I do not
>> think such a change breaks anything, but I do not deal with complex
>> histories myself, so...
>
> As far as I can tell, the rev-list will return multiple lines when not
> using 'oneline'. The 'sed -n' will join the lines back together.
There is no joining going on.
To "rev-list", a custom --pretty/--format is a signal to trigger its
"verbose" mode, and it shows a "commit " line and then
the line in the format specified, e.g.
$ git rev-list --pretty='%m%H %<(35,trunc)%s' --right-only --reverse ...2024d3
commit 1e9676ec0a771de06abca3009eb4bdc5a4ae3312
>1e9676ec0a771de06abca3009eb4bdc5a4ae3312 lockfile: replace random() by ran..
commit 2024d3176536fd437b4c0a744161e96bc150a24e
>2024d3176536fd437b4c0a744161e96bc150a24e help.c: wrap wait-only poll() inv..
...
Because of that, "format=%m | sed -n s/>//p" would be one way to
make sure that all lines we care about are prefixed by '>' so that
we can pick them while discarding anything else. So you do need
filtering unless switch to "log", even if you used --right-only.
That is why I didn't use "rev-list" in the message you are
responding to.
--
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] git-rebase--interactive.sh: add config option for custom
On Mon, Jun 8, 2015 at 11:28 AM, 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
>> ---
>
> Thanks. Roberto's gizmo seems to be working OK ;-)
Will see if the pull request -> email contraption will allow me to put
[patch v2] in there. I also need to see if it can make a [patch 0/1]
>
>> git-rebase--interactive.sh | 9 -
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index dc3133f..cc79b81 100644
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -977,7 +977,14 @@ else
>> revisions=$onto...$orig_head
>> shortrevisions=$shorthead
>> fi
>> -git rev-list $merges_option --pretty=oneline --reverse --left-right
>> --topo-order \
>> +format=$(git config --get rebase.instructionFormat)
>> +if test -z "$format"
>> +then
>> + format="%s"
>
> Style. One indent level in our shell scripts is one HT, not a few spaces.
>
>> +fi
>> +# the 'rev-list .. | sed' requires %m to parse; the instruction requires %h
>> to parse
>> +format="%m%h ${format}"
>
> I think you want %H not %h here. If you check how the default
> "--pretty=online" is shown, you would see something like this:
>
> >1e9676ec0a771de06abca3009eb4bdc5a4ae3312 lockfile: replace ...
> >2024d3176536fd437b4c0a744161e96bc150a24e help.c: wrap wait-...
>
>> +git rev-list $merges_option --pretty="${format}" --reverse --left-right
>> --topo-order \
>> $revisions ${restrict_revision+^$restrict_revision} | \
>> sed -n "s/^>//p" |
I will make the changes from above, and resubmit a patch.
>
> This is optional, but I still wonder why the command line cannot be
> more like this, though:
>
> format=$(git config --get rebase.insnFormat)
> git log --format="%H ${format-%s}" --reverse --right-only
> --topo-order \
> $revisions ${restrict_revision+^$restrict_revision} |
> while read -r sha1 junk
> do
> ...
>
> That way we can optimize one "sed" process away.
>
> If this is a good idea, it needs to be a separate follow-up patch
> that changes "%m filtered by sed" to "use --right-only". I do not
> think such a change breaks anything, but I do not deal with complex
> histories myself, so...
>
As far as I can tell, the rev-list will return multiple lines when not
using 'oneline'. The 'sed -n' will join the lines back together. I
will take a look at moving it to 'git log' for a future change. I
have a huge codebase with tons of branches to experiment with.
--
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] git-rebase--interactive.sh: add config option for custom
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
> ---
Thanks. Roberto's gizmo seems to be working OK ;-)
> git-rebase--interactive.sh | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index dc3133f..cc79b81 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -977,7 +977,14 @@ else
> revisions=$onto...$orig_head
> shortrevisions=$shorthead
> fi
> -git rev-list $merges_option --pretty=oneline --reverse --left-right
> --topo-order \
> +format=$(git config --get rebase.instructionFormat)
> +if test -z "$format"
> +then
> + format="%s"
Style. One indent level in our shell scripts is one HT, not a few spaces.
> +fi
> +# the 'rev-list .. | sed' requires %m to parse; the instruction requires %h
> to parse
> +format="%m%h ${format}"
I think you want %H not %h here. If you check how the default
"--pretty=online" is shown, you would see something like this:
>1e9676ec0a771de06abca3009eb4bdc5a4ae3312 lockfile: replace ...
>2024d3176536fd437b4c0a744161e96bc150a24e help.c: wrap wait-...
> +git rev-list $merges_option --pretty="${format}" --reverse --left-right
> --topo-order \
> $revisions ${restrict_revision+^$restrict_revision} | \
> sed -n "s/^>//p" |
This is optional, but I still wonder why the command line cannot be
more like this, though:
format=$(git config --get rebase.insnFormat)
git log --format="%H ${format-%s}" --reverse --right-only --topo-order \
$revisions ${restrict_revision+^$restrict_revision} |
while read -r sha1 junk
do
...
That way we can optimize one "sed" process away.
If this is a good idea, it needs to be a separate follow-up patch
that changes "%m filtered by sed" to "use --right-only". I do not
think such a change breaks anything, but I do not deal with complex
histories myself, so...
--
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
