Re: 2.6.0: Comment in rebase instruction has become too rigid

[Ralf Thielow <ralf.thielow@gmail.com>]

2015-09-29 20:17 GMT+02:00 Junio C Hamano <gitster@pobox.com>:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>>> Confirmed: Git 2.1.4 accepts this, 2.6 doesn't:
>>>
>>> Warning: the command isn't recognized in the following line:
>>>  - # pick dbafac11052a0075233bdcf0b71f54d1503aa82d test
>>>
>>> You can fix this with 'git rebase --edit-todo'.
>>> Or you can abort the rebase with 'git rebase --abort'.
>>>
>>> I didn't bisect, but I guess this was introduced in the series
>>> introducing this check on the todolist before starting the bisection.
>>
>> Indeed:
>>
>> 804098bb30a5339cccb0be981a3e876245aa0ae5 is the first bad commit
>
> Yup, before that series, expand_todo_ids -> transfom_todo_ids ended
> up reading each line with "while read -r command rest" loop and the
> we did not honor the usual "# at the beginning line is the comment"
> convention, which I think was a bug.  With that commit, a separate
> step in check_bad_cmd_and_sha1 uses a similar looking "while read"
> loop but forgets to take '#' into account.
>
> I know you alluded to preprocess what is fed to stripspace, but I
> wonder if we can remove the misguided call to stripspace in the
> first place and do something like the attached instead.
>
>  git-rebase--interactive.sh | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index f01637b..a64f77a 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -886,7 +886,6 @@ check_commit_sha () {
>  # from the todolist in stdin
>  check_bad_cmd_and_sha () {
>         retval=0
> -       git stripspace --strip-comments |
>         (
>                 while read -r line
>                 do
> @@ -896,7 +895,7 @@ check_bad_cmd_and_sha () {
>                         sha1=$2
>
>                         case $command in
> -                       ''|noop|x|"exec")
> +                       '#'*|''|noop|x|"exec")

If so, I think we should use "$comment_char"* here.

>                                 # Doesn't expect a SHA-1
>                                 ;;
>                         pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html