threads / patch / 48783

patchrebase -i: Fix white space in comments

Subject: [PATCH] rebase -i: Fix white space in comments

## tl;dr

7 messages between Jun 26, 2018 and Jun 27, 2018. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

dana· Jun 26, 2018, 18:02 UTC · lore

Fix a trivial white-space issue introduced by commit d48f97aa8 ("rebase: reindent function git_rebase__interactive", 2018-03-23). This affected the instructional comments displayed in the editor during an interactive rebase.

Signed-off-by: dana <dana@dana.is>
---

Sorry if i've done any of this wrong; i've never used this work-flow before. In any case, if it's not immediately obvious, this is the issue i mean to fix:

BEFORE (2.17.1):

# If you remove a line here THAT COMMIT WILL BE LOST. # # However, if you remove everything, the rebase will be aborted. # # Note that empty commits are commented out

AFTER (2.18.0):

# If you remove a line here THAT COMMIT WILL BE LOST. # # However, if you remove everything, the rebase will be aborted. # # # Note that empty commits are commented out

The 2.18.0 version is particularly irritating because many editors highlight the trailing tab in the penultimate line as a white-space error.

Aside: It's not a new thing, but i've always felt like that last line
should end in a full stop. Maybe i'll send a patch for that too.

Cheers, dana

 git-rebase--interactive.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to git-rebase--interactive.sh +2 −2
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 299ded213..a31af6d4c 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -222,9 +222,9 @@ $comment_char $(eval_ngettext \
 EOF
 	append_todo_help
 	gettext "
-	However, if you remove everything, the rebase will be aborted.
+However, if you remove everything, the rebase will be aborted.
 
-	" | git stripspace --comment-lines >>"$todo"
+" | git stripspace --comment-lines >>"$todo"
 
 	if test -z "$keep_empty"
 	then
-- 
2.18.0
Johannes Schindelin· Jun 26, 2018, 21:30 UTC · re: dana · lore

Re: [PATCH] rebase -i: Fix white space in comments

Let's Cc: Wink, who authored the commit mentioned as culprit in the commit message.

On Tue, 26 Jun 2018, dana wrote:
Show 62 quoted lines
> Fix a trivial white-space issue introduced by commit d48f97aa8
> ("rebase: reindent function git_rebase__interactive", 2018-03-23). This
> affected the instructional comments displayed in the editor during an
> interactive rebase.
> 
> Signed-off-by: dana <dana@dana.is>
> ---
> 
> Sorry if i've done any of this wrong; i've never used this work-flow
> before. In any case, if it's not immediately obvious, this is the issue
> i mean to fix:
> 
> BEFORE (2.17.1):
> 
> # If you remove a line here THAT COMMIT WILL BE LOST.
> #
> # However, if you remove everything, the rebase will be aborted.
> #
> # Note that empty commits are commented out
> 
> AFTER (2.18.0):
> 
> # If you remove a line here THAT COMMIT WILL BE LOST.
> #
> #	However, if you remove everything, the rebase will be aborted.
> #
> #	
> # Note that empty commits are commented out
> 
> The 2.18.0 version is particularly irritating because many editors
> highlight the trailing tab in the penultimate line as a white-space
> error.
> 
> Aside: It's not a new thing, but i've always felt like that last line
> should end in a full stop. Maybe i'll send a patch for that too.
> 
> Cheers,
> dana
> 
>  git-rebase--interactive.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 299ded213..a31af6d4c 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -222,9 +222,9 @@ $comment_char $(eval_ngettext \
>  EOF
>  	append_todo_help
>  	gettext "
> -	However, if you remove everything, the rebase will be aborted.
> +However, if you remove everything, the rebase will be aborted.
>  
> -	" | git stripspace --comment-lines >>"$todo"
> +" | git stripspace --comment-lines >>"$todo"
>  
>  	if test -z "$keep_empty"
>  	then
> -- 
> 2.18.0
> 
> 
Johannes Schindelin· Jun 26, 2018, 21:35 UTC · re: Johannes Schindelin · lore

Re: [PATCH] rebase -i: Fix white space in comments

Hi,
and now for the review...
On Tue, 26 Jun 2018, Johannes Schindelin wrote:
Show 18 quoted lines
> On Tue, 26 Jun 2018, dana wrote:
> 
> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > index 299ded213..a31af6d4c 100644
> > --- a/git-rebase--interactive.sh
> > +++ b/git-rebase--interactive.sh
> > @@ -222,9 +222,9 @@ $comment_char $(eval_ngettext \
> >  EOF
> >  	append_todo_help
> >  	gettext "
> > -	However, if you remove everything, the rebase will be aborted.
> > +However, if you remove everything, the rebase will be aborted.
> >  
> > -	" | git stripspace --comment-lines >>"$todo"
> > +" | git stripspace --comment-lines >>"$todo"
> >  
> >  	if test -z "$keep_empty"
> >  	then

This does the job, and I am fine with this way of doing things, and there seems to be a lot of precedent doing it this way e.g. in git-bisect.sh.

If my ACK is welcome, you hereby have it.

Ciao, Johannes

Johannes Schindelin· Jun 26, 2018, 21:44 UTC · re: Johannes Schindelin · lore

Re: [PATCH] rebase -i: Fix white space in comments

Hi, me again,
On Tue, 26 Jun 2018, Johannes Schindelin wrote:
Show 23 quoted lines
> On Tue, 26 Jun 2018, Johannes Schindelin wrote:
> 
> > On Tue, 26 Jun 2018, dana wrote:
> > 
> > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > > index 299ded213..a31af6d4c 100644
> > > --- a/git-rebase--interactive.sh
> > > +++ b/git-rebase--interactive.sh
> > > @@ -222,9 +222,9 @@ $comment_char $(eval_ngettext \
> > >  EOF
> > >  	append_todo_help
> > >  	gettext "
> > > -	However, if you remove everything, the rebase will be aborted.
> > > +However, if you remove everything, the rebase will be aborted.
> > >  
> > > -	" | git stripspace --comment-lines >>"$todo"
> > > +" | git stripspace --comment-lines >>"$todo"
> > >  
> > >  	if test -z "$keep_empty"
> > >  	then
> 
> This does the job, and I am fine with this way of doing things, and there
> seems to be a lot of precedent doing it this way e.g. in git-bisect.sh.

There is of course one other way to fix this, and that is by rewriting this in C.

Which Alban has done here ;-)
http://public-inbox.org/git/20180626161643.31152-3-alban.gruin@gmail.com

Ciao, Johannes

dana· Jun 26, 2018, 21:54 UTC · re: Johannes Schindelin · lore

Re: [PATCH] rebase -i: Fix white space in comments

On 26 Jun 2018, at 16:44, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 6 quoted lines
>There is of course one other way to fix this, and that is by rewriting
>this in C.
>
>Which Alban has done here ;-)
>
>http://public-inbox.org/git/20180626161643.31152-3-alban.gruin@gmail.com

Oh, i'm sorry, i didn't see that. That change does appear to solve the same problem, so i'm happy to defer to it.

Thanks for looking!
dana
Johannes Schindelin· Jun 27, 2018, 10:52 UTC · re: dana · lore

Re: [PATCH] rebase -i: Fix white space in comments

Hi Dana,
On Tue, 26 Jun 2018, dana wrote:
Show 9 quoted lines
> On 26 Jun 2018, at 16:44, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >There is of course one other way to fix this, and that is by rewriting
> >this in C.
> >
> >Which Alban has done here ;-)
> >
> >http://public-inbox.org/git/20180626161643.31152-3-alban.gruin@gmail.com
> 
> Oh, i'm sorry, i didn't see that.

No need to be sorry, nobody expects you to read the "firehose" that is the Git mailing list in its entirety.

> That change does appear to solve the same problem, so i'm happy to defer
> to it.

Thank you for confirming that it also fixes your issue, that is very helpful!

Ciao, Johannes

← back to recent threads