* [PATCH] git-rebase.sh: Update USAGE string
@ 2008-02-03 20:19 Jari Aalto
2008-02-03 21:14 ` Jakub Narebski
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Jari Aalto @ 2008-02-03 20:19 UTC (permalink / raw)
To: git
Present the options in -s|--long (short, long) order.
Mention merge and new whitespace option.
Signed-off-by: Jari Aalto <jari.aalto AT cante.net>
---
git-rebase.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index bdcea0e..9c8cf90 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -3,7 +3,7 @@
# Copyright (c) 2005 Junio C Hamano.
#
-USAGE='[--interactive | -i] [-v] [--onto <newbase>] <upstream> [<branch>]'
+USAGE='[-i|--interactive] [-v|--verbose] [--whitespace={nowarn|warn|error|error-all|strip}] [-m|--merge] [--onto <newbase>] <upstream> [<branch>]'
LONG_USAGE='git-rebase replaces <branch> with a new branch of the
same name. When the --onto option is provided the new branch starts
out with a HEAD equal to <newbase>, otherwise it is equal to <upstream>
--
1.5.4-rc5.GIT-dirty
--
Welcome to FOSS revolution: we fix and modify until it shines
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] git-rebase.sh: Update USAGE string
2008-02-03 20:19 [PATCH] git-rebase.sh: Update USAGE string Jari Aalto
@ 2008-02-03 21:14 ` Jakub Narebski
2008-02-03 23:33 ` [PATCH] git-rebase.sh: Update USAGE string (No. 1) Jari Aalto
2008-02-04 0:16 ` [PATCH v2] git-rebase.sh: Update USAGE string Jari Aalto
2 siblings, 0 replies; 20+ messages in thread
From: Jakub Narebski @ 2008-02-03 21:14 UTC (permalink / raw)
To: Jari Aalto; +Cc: git
Jari Aalto <jari.aalto@cante.net> writes:
> Present the options in -s|--long (short, long) order.
This is a good change (but might not make sense without the following
one)...
> Mention merge and new whitespace option.
...and this is not; usage string is now too long.
Please either use placeholder ([<whitespace>]), or enumerate
only short version of commands ([-i] [-v] [-m]) in synopsis,
or break synopsis into lines 80-column lines max.
> -USAGE='[--interactive | -i] [-v] [--onto <newbase>] <upstream> [<branch>]'
> +USAGE='[-i|--interactive] [-v|--verbose] [--whitespace={nowarn|warn|error|error-all|strip}] [-m|--merge] [--onto <newbase>] <upstream> [<branch>]'
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-03 20:19 [PATCH] git-rebase.sh: Update USAGE string Jari Aalto
2008-02-03 21:14 ` Jakub Narebski
@ 2008-02-03 23:33 ` Jari Aalto
2008-02-03 23:41 ` Johannes Schindelin
` (2 more replies)
2008-02-04 0:16 ` [PATCH v2] git-rebase.sh: Update USAGE string Jari Aalto
2 siblings, 3 replies; 20+ messages in thread
From: Jari Aalto @ 2008-02-03 23:33 UTC (permalink / raw)
To: git
Present the options in -s|--long (short, long) order.
Mention merge and new whitespace option.
ed-off-by: Jari Aalto <jari.aalto AT cante.net>
---
***********************************************************************
Reworked patch (No. 1)
[Jakub Narebski]
> or break synopsis into lines 80-column lines max.
***********************************************************************
git-rebase.sh | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index bdcea0e..6805742 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -3,7 +3,10 @@
# Copyright (c) 2005 Junio C Hamano.
#
-USAGE='[--interactive | -i] [-v] [--onto <newbase>] <upstream> [<branch>]'
+USAGE='[-i|--interactive] [-v|--verbose]
+[--whitespace={nowarn|warn|error|error-all|strip}]
+[-m|--merge] [--onto <newbase>] <upstream> [<branch>]'
+
LONG_USAGE='git-rebase replaces <branch> with a new branch of the
same name. When the --onto option is provided the new branch starts
out with a HEAD equal to <newbase>, otherwise it is equal to <upstream>
--
1.5.4-rc5.GIT-dirty
--
Welcome to FOSS revolution: we fix and modify until it shines
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-03 23:33 ` [PATCH] git-rebase.sh: Update USAGE string (No. 1) Jari Aalto
@ 2008-02-03 23:41 ` Johannes Schindelin
2008-02-03 23:52 ` Jakub Narebski
2008-02-04 11:12 ` [PATCH] git-rebase.sh: Update USAGE string (No. 1) しらいしななこ
2 siblings, 0 replies; 20+ messages in thread
From: Johannes Schindelin @ 2008-02-03 23:41 UTC (permalink / raw)
To: Jari Aalto; +Cc: git
Hi,
what's this "(No. 1)" business?
On Mon, 4 Feb 2008, Jari Aalto wrote:
> ed-off-by: Jari Aalto <jari.aalto AT cante.net>
ed-by: me... ;-)
> Welcome to FOSS revolution: we fix and modify until it shines
/me grins
Ciao,
Dscho
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-03 23:33 ` [PATCH] git-rebase.sh: Update USAGE string (No. 1) Jari Aalto
2008-02-03 23:41 ` Johannes Schindelin
@ 2008-02-03 23:52 ` Jakub Narebski
2008-02-04 0:11 ` Jari Aalto
2008-02-04 0:24 ` Junio C Hamano
2008-02-04 11:12 ` [PATCH] git-rebase.sh: Update USAGE string (No. 1) しらいしななこ
2 siblings, 2 replies; 20+ messages in thread
From: Jakub Narebski @ 2008-02-03 23:52 UTC (permalink / raw)
To: Jari Aalto; +Cc: git
Jari Aalto <jari.aalto@cante.net> writes:
> Subject: Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
What's with the "(No. 1)" here? If you want to add some comment that
should *not* be part of commit message, put it in brackets beside
PATCH, for example "[PATCH/RFC]", "[PATCH (resend)]",
"[PATCH (amend)]", "[PATCH v2]" etc.
> Present the options in -s|--long (short, long) order.
> Mention merge and new whitespace option.
>
> ed-off-by: Jari Aalto <jari.aalto AT cante.net>
I guess that it is copy'n'paste error, and it should be
Signed-off-by: Jari Aalto <jari.aalto@cante.net>
>
> -USAGE='[--interactive | -i] [-v] [--onto <newbase>] <upstream> [<branch>]'
> +USAGE='[-i|--interactive] [-v|--verbose]
> +[--whitespace={nowarn|warn|error|error-all|strip}]
I would say "[--whitespace=<option>]" or "[--whitespace=<action>]"
instead of introducing yet not agreed upon notation (this has the
advantage of shortening synopisis, which should be short IMHO).
Besides here the mutually exclusive options are naturelly delimited,
so you can say just (I think):
[--whitespace=nowarn|warn|error|error-all|strip]
> +[-m|--merge] [--onto <newbase>] <upstream> [<branch>]'
> +
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-03 23:52 ` Jakub Narebski
@ 2008-02-04 0:11 ` Jari Aalto
2008-02-04 0:27 ` Johannes Schindelin
2008-02-04 0:39 ` Junio C Hamano
2008-02-04 0:24 ` Junio C Hamano
1 sibling, 2 replies; 20+ messages in thread
From: Jari Aalto @ 2008-02-04 0:11 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
* Sun 2008-02-03 Jakub Narebski <jnareb@gmail.com> INBOX
> I would say "[--whitespace=<option>]" or "[--whitespace=<action>]"
> instead of introducing yet not agreed upon notation (this has the
> advantage of shortening synopisis, which should be short IMHO).
The "{a|b|c}" is a used syntax. See cpio(1).
cpio {-o|--create} [-0acvABLV] ...
The information about values to --whitespace is buried under 2 manual
pages. A help that displays the values immediately is more helpful. The
--whitespace option is used very frequently.
Jari
--
Welcome to FOSS revolution: we fix and modify until it shines
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] git-rebase.sh: Update USAGE string
2008-02-03 20:19 [PATCH] git-rebase.sh: Update USAGE string Jari Aalto
2008-02-03 21:14 ` Jakub Narebski
2008-02-03 23:33 ` [PATCH] git-rebase.sh: Update USAGE string (No. 1) Jari Aalto
@ 2008-02-04 0:16 ` Jari Aalto
2 siblings, 0 replies; 20+ messages in thread
From: Jari Aalto @ 2008-02-04 0:16 UTC (permalink / raw)
To: git
Present the options in -s|--long (short, long) order.
Mention merge and new whitespace option.
Signed-off-by: Jari Aalto <jari.aalto AT cante.net>
---
***********************************************************************
Reworked patch (No. 2)
* [Jakub Narebski]
> break synopsis into lines 80-column lines max.
* Fix "ed-off-by"
***********************************************************************
git-rebase.sh | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index bdcea0e..6805742 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -3,7 +3,10 @@
# Copyright (c) 2005 Junio C Hamano.
#
-USAGE='[--interactive | -i] [-v] [--onto <newbase>] <upstream> [<branch>]'
+USAGE='[-i|--interactive] [-v|--verbose]
+[--whitespace={nowarn|warn|error|error-all|strip}]
+[-m|--merge] [--onto <newbase>] <upstream> [<branch>]'
+
LONG_USAGE='git-rebase replaces <branch> with a new branch of the
same name. When the --onto option is provided the new branch starts
out with a HEAD equal to <newbase>, otherwise it is equal to <upstream>
--
1.5.4-rc5.GIT-dirty
--
Welcome to FOSS revolution: we fix and modify until it shines
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-03 23:52 ` Jakub Narebski
2008-02-04 0:11 ` Jari Aalto
@ 2008-02-04 0:24 ` Junio C Hamano
2008-02-04 8:04 ` [PATCH v3] git-rebase.sh: Update USAGE string Jari Aalto
1 sibling, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2008-02-04 0:24 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Jari Aalto, git
Jakub Narebski <jnareb@gmail.com> writes:
> Besides here the mutually exclusive options are naturelly delimited,
> so you can say just (I think):
> [--whitespace=nowarn|warn|error|error-all|strip]
I think that is probably the right thing to do in this case. It
is clear where the alternatives end, without adding extra
grouping, so it keeps the description easy to read.
It may fail with a sick reader who would complain "but 'git
rebase --whitespace=nowarn' is correct while 'git rebase warn'
is not!", but personally I do not think we deeply care about
pleasing such a pedantic nitpicker.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-04 0:11 ` Jari Aalto
@ 2008-02-04 0:27 ` Johannes Schindelin
2008-02-04 0:39 ` Junio C Hamano
1 sibling, 0 replies; 20+ messages in thread
From: Johannes Schindelin @ 2008-02-04 0:27 UTC (permalink / raw)
To: Jari Aalto; +Cc: Jakub Narebski, git
Hi,
On Mon, 4 Feb 2008, Jari Aalto wrote:
> * Sun 2008-02-03 Jakub Narebski <jnareb@gmail.com> INBOX
> > I would say "[--whitespace=<option>]" or "[--whitespace=<action>]"
> > instead of introducing yet not agreed upon notation (this has the
> > advantage of shortening synopisis, which should be short IMHO).
>
> The "{a|b|c}" is a used syntax. See cpio(1).
By this reasoning, "(a|b|c)" is also a used syntax. See git-branch(1).
Ciao,
Dscho
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-04 0:11 ` Jari Aalto
2008-02-04 0:27 ` Johannes Schindelin
@ 2008-02-04 0:39 ` Junio C Hamano
2008-02-04 10:15 ` Jakub Narebski
1 sibling, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2008-02-04 0:39 UTC (permalink / raw)
To: Jari Aalto; +Cc: Jakub Narebski, git
Jari Aalto <jari.aalto@cante.net> writes:
> * Sun 2008-02-03 Jakub Narebski <jnareb@gmail.com> INBOX
>> I would say "[--whitespace=<option>]" or "[--whitespace=<action>]"
>> instead of introducing yet not agreed upon notation (this has the
>> advantage of shortening synopisis, which should be short IMHO).
>
> The "{a|b|c}" is a used syntax. See cpio(1).
>
> cpio {-o|--create} [-0acvABLV] ...
I do not think using {} for grouping is incorrect, and I think
there is at least a consensus on the list that it is Ok as long
as we consistently do so.
Unfortunately, the majority of, if not all of, our existing
documents use () instead for that purpose.
So pros-and-cons are that (1) changing all of them to use {} is
more politically correct (pro); (2) our use of (), as we
consistently use them, does not hurt readability (neutral); and
(3) it is a thankless makework to replace them all to {} _and
make sure the conversion is correct_ (large con). (4) also if
other people make changes to documentation at the same time,
that would add more work in conflict resolution (slight con).
As you seem to have volunteered to do the conversion and
validation, I do not see much problem to move from () to {} in
principle.
About the part your patch touches, [--whitespace={a|b|c}] is
more precise than [--whitespace=a|b|c] Jakub suggested, but I
suspect most sane people would not misinterpret the latter as
"this part can be omitted but you could write '--whitespace=a',
'b' or 'c' here", so...
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3] git-rebase.sh: Update USAGE string
2008-02-04 0:24 ` Junio C Hamano
@ 2008-02-04 8:04 ` Jari Aalto
0 siblings, 0 replies; 20+ messages in thread
From: Jari Aalto @ 2008-02-04 8:04 UTC (permalink / raw)
To: git
Present the options in -s|--long (short, long) order.
Mention merge and new whitespace option.
Signed-off-by: Jari Aalto <jari.aalto AT cante.net>
---
***********************************************************************
reworked patch (No. 3)
***********************************************************************
[Junio C Hamano}
>> [--whitespace=nowarn|warn|error|error-all|strip]
>
> I think that is probably the right thing to do in this case. It
> is clear where the alternatives end, without adding extra
> grouping, so it keeps the description easy to read.
git-rebase.sh | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index bdcea0e..6128455 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -3,7 +3,10 @@
# Copyright (c) 2005 Junio C Hamano.
#
-USAGE='[--interactive | -i] [-v] [--onto <newbase>] <upstream> [<branch>]'
+USAGE='[-i|--interactive] [-v|--verbose]
+[--whitespace=nowarn|warn|error|error-all|strip]
+[-m|--merge] [--onto <newbase>] <upstream> [<branch>]'
+
LONG_USAGE='git-rebase replaces <branch> with a new branch of the
same name. When the --onto option is provided the new branch starts
out with a HEAD equal to <newbase>, otherwise it is equal to <upstream>
--
1.5.4-rc5.GIT-dirty
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-04 0:39 ` Junio C Hamano
@ 2008-02-04 10:15 ` Jakub Narebski
2008-02-04 11:57 ` Mike Hommey
0 siblings, 1 reply; 20+ messages in thread
From: Jakub Narebski @ 2008-02-04 10:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jari Aalto, git
Junio C Hamano wrote:
> Jari Aalto <jari.aalto@cante.net> writes:
>
>> * Sun 2008-02-03 Jakub Narebski <jnareb@gmail.com> INBOX
>>> I would say "[--whitespace=<option>]" or "[--whitespace=<action>]"
>>> instead of introducing yet not agreed upon notation (this has the
>>> advantage of shortening synopisis, which should be short IMHO).
>>
>> The "{a|b|c}" is a used syntax. See cpio(1).
>>
>> cpio {-o|--create} [-0acvABLV] ...
BTW. it is not only cpio and rpm, but also combinediff (and friends from
patchutils), flex, gimp, jpegtopnm (and other from netpbm), ps2pdf,
run, tail, xmlto, ip, losetup, netstat, sudo and sudoedit,...
> I do not think using {} for grouping is incorrect, and I think
> there is at least a consensus on the list that it is Ok as long
> as we consistently do so.
>
> Unfortunately, the majority of, if not all of, our existing
> documents use () instead for that purpose.
>
> So pros-and-cons are that (1) changing all of them to use {} is
> more politically correct (pro); (2) our use of (), as we
> consistently use them, does not hurt readability (neutral); and
Actually we are not entirely consistent here. git-init(1) has
--shared[={false|true|umask|group|all|world|everybody}]
in the option description, git-rev-list(1) has
[ --date={local|relative|default|iso|rfc|short} ]
in its longish synopsis (although in second case we could omit
the curlies, it is in separate line so reducing line length does
not matter in this case).
> (3) it is a thankless makework to replace them all to {} _and
> make sure the conversion is correct_ (large con).
All true.
> (4) also if
> other people make changes to documentation at the same time,
> that would add more work in conflict resolution (slight con).
Well, we sould have to document this in CodingStyle, I think, then.
> About the part your patch touches, [--whitespace={a|b|c}] is
> more precise than [--whitespace=a|b|c] Jakub suggested, but I
> suspect most sane people would not misinterpret the latter as
> "this part can be omitted but you could write '--whitespace=a',
> 'b' or 'c' here", so...
It probably also depends if we deperately want to reduce line length as
not to split synopsis into yet another line (the shorter synopsis is,
usually the better).
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-03 23:33 ` [PATCH] git-rebase.sh: Update USAGE string (No. 1) Jari Aalto
2008-02-03 23:41 ` Johannes Schindelin
2008-02-03 23:52 ` Jakub Narebski
@ 2008-02-04 11:12 ` しらいしななこ
2008-02-04 15:06 ` rebase -i and --whitespace, was " Johannes Schindelin
2 siblings, 1 reply; 20+ messages in thread
From: しらいしななこ @ 2008-02-04 11:12 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Jari Aalto, git
Quoting Jakub Narebski <jnareb@gmail.com>:
> Jari Aalto <jari.aalto@cante.net> writes:
>>
>> -USAGE='[--interactive | -i] [-v] [--onto <newbase>] <upstream> [<branch>]'
>> +USAGE='[-i|--interactive] [-v|--verbose]
>> +[--whitespace={nowarn|warn|error|error-all|strip}]
>
> I would say "[--whitespace=<option>]" or "[--whitespace=<action>]"
> instead of introducing yet not agreed upon notation (this has the
> advantage of shortening synopisis, which should be short IMHO).
>
> Besides here the mutually exclusive options are naturelly delimited,
> so you can say just (I think):
> [--whitespace=nowarn|warn|error|error-all|strip]
>
>> +[-m|--merge] [--onto <newbase>] <upstream> [<branch>]'
>> +
I tried to run "git rebase --interactive --whitespace=strip" but it does not seem to strip blank characters at the end of my lines. Did I find a bug?
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
----------------------------------------------------------------------
Find out how you can get spam free email.
http://www.bluebottle.com/tag/3
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-04 10:15 ` Jakub Narebski
@ 2008-02-04 11:57 ` Mike Hommey
2008-02-04 12:53 ` Jakub Narebski
0 siblings, 1 reply; 20+ messages in thread
From: Mike Hommey @ 2008-02-04 11:57 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, Jari Aalto, git
On Mon, Feb 04, 2008 at 11:15:20AM +0100, Jakub Narebski <jnareb@gmail.com> wrote:
> Actually we are not entirely consistent here. git-init(1) has
> --shared[={false|true|umask|group|all|world|everybody}]
> in the option description, git-rev-list(1) has
> [ --date={local|relative|default|iso|rfc|short} ]
What is inconsistent here ? The first tells you you can use --shared without
an argument. And it's [--shared[=<permissions>]] in the synopsis, so you
may omit --shared, or use in alone, or specify permissions. The second tells
you can't use --date without an additional argument.
Mike
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-04 11:57 ` Mike Hommey
@ 2008-02-04 12:53 ` Jakub Narebski
0 siblings, 0 replies; 20+ messages in thread
From: Jakub Narebski @ 2008-02-04 12:53 UTC (permalink / raw)
To: Mike Hommey; +Cc: Junio C Hamano, Jari Aalto, git
On Mon, 4 Feb 2008, Mike Hommey wrote:
> On Mon, Feb 04, 2008 at 11:15:20AM +0100, Jakub Narebski <jnareb@gmail.com> wrote:
>> Junio C Hamano wrote:
>>> Unfortunately, the majority of, if not all of, our existing
>>> documents use () instead for that purpose.
>>>
>>> So pros-and-cons are that [...] (2) our use of (), as we
>>> consistently use them, does not hurt readability (neutral); and
The above was re-added by me.
>> Actually we are not entirely consistent here. git-init(1) has
>> --shared[={false|true|umask|group|all|world|everybody}]
>> in the option description, git-rev-list(1) has
>> [ --date={local|relative|default|iso|rfc|short} ]
>
> What is inconsistent here ? The first tells you you can use --shared without
> an argument. And it's [--shared[=<permissions>]] in the synopsis, so you
> may omit --shared, or use in alone, or specify permissions. The second tells
> you can't use --date without an additional argument.
You have cut a bit too much when quoting. The inconsistency is between
git-branch(1) using () to delimit mutually exclusive options:
git-branch (-m | -M) [<oldbranch>] <newbranch>
and git-init(1) (and git-rev-list(1)) using {} for that:
--shared[={false|true|umask|group|all|world|everybody}]
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 20+ messages in thread
* rebase -i and --whitespace, was Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-04 11:12 ` [PATCH] git-rebase.sh: Update USAGE string (No. 1) しらいしななこ
@ 2008-02-04 15:06 ` Johannes Schindelin
2008-02-04 15:42 ` Jakub Narebski
0 siblings, 1 reply; 20+ messages in thread
From: Johannes Schindelin @ 2008-02-04 15:06 UTC (permalink / raw)
To: しらいしななこ
Cc: Jakub Narebski, Jari Aalto, git
[-- Attachment #1: Type: TEXT/PLAIN, Size: 503 bytes --]
Hi,
On Mon, 4 Feb 2008, しらいしななこ wrote:
> I tried to run "git rebase --interactive --whitespace=strip" but it does
> not seem to strip blank characters at the end of my lines. Did I find a
> bug?
Yes. Interactive rebase never bothered with --whitespace options, since
it works purely with cherry-pick (the --merge option to non-interactive
rebase). Therefore, the operation is not patch based, and does not call
git-apply at all (which would handle the whitespace).
Ciao,
Dscho
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: rebase -i and --whitespace, was Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-04 15:06 ` rebase -i and --whitespace, was " Johannes Schindelin
@ 2008-02-04 15:42 ` Jakub Narebski
2008-02-04 16:32 ` Johannes Schindelin
2008-02-05 23:43 ` Junio C Hamano
0 siblings, 2 replies; 20+ messages in thread
From: Jakub Narebski @ 2008-02-04 15:42 UTC (permalink / raw)
To: Johannes Schindelin
Cc: しらいしななこ ,
Jari Aalto, git
W poniedziałek 4. lutego 2008 16:06, Johannes Schindelin napisał:
> On Mon, 4 Feb 2008, しらいしななこ wrote:
>
> > I tried to run "git rebase --interactive --whitespace=strip" but it does
> > not seem to strip blank characters at the end of my lines. Did I find a
> > bug?
>
> Yes. Interactive rebase never bothered with --whitespace options, since
> it works purely with cherry-pick (the --merge option to non-interactive
> rebase). Therefore, the operation is not patch based, and does not call
> git-apply at all (which would handle the whitespace).
So it means that synopsis should, instead of current (pre-patch)
'git-rebase' [-i | --interactive] [-v | --verbose] [-m | --merge]
[-C<n>] [ --whitespace=<option>] [-p | --preserve-merges]
[--onto <newbase>] <upstream> [<branch>]
should read
'git-rebase' [-v | --verbose] [-p | --preserve-merges]
[{-i | --interactive} | [-C<n>] [ --whitespace=<option>] [-m | --merge]]
[--onto <newbase>] <upstream> [<branch>]
or perhaps even separated into interactive / non-interactive merge?
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: rebase -i and --whitespace, was Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-04 15:42 ` Jakub Narebski
@ 2008-02-04 16:32 ` Johannes Schindelin
2008-02-05 23:43 ` Junio C Hamano
1 sibling, 0 replies; 20+ messages in thread
From: Johannes Schindelin @ 2008-02-04 16:32 UTC (permalink / raw)
To: Jakub Narebski
Cc: しらいしななこ,
Jari Aalto, git
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1425 bytes --]
Hi,
On Mon, 4 Feb 2008, Jakub Narebski wrote:
> W poniedziałek 4. lutego 2008 16:06, Johannes Schindelin napisał:
> > On Mon, 4 Feb 2008, しらいしななこ wrote:
> >
> > > I tried to run "git rebase --interactive --whitespace=strip" but it does
> > > not seem to strip blank characters at the end of my lines. Did I find a
> > > bug?
> >
> > Yes. Interactive rebase never bothered with --whitespace options, since
> > it works purely with cherry-pick (the --merge option to non-interactive
> > rebase). Therefore, the operation is not patch based, and does not call
> > git-apply at all (which would handle the whitespace).
>
> So it means that synopsis should, instead of current (pre-patch)
>
> 'git-rebase' [-i | --interactive] [-v | --verbose] [-m | --merge]
> [-C<n>] [ --whitespace=<option>] [-p | --preserve-merges]
> [--onto <newbase>] <upstream> [<branch>]
>
> should read
>
> 'git-rebase' [-v | --verbose] [-p | --preserve-merges]
> [{-i | --interactive} | [-C<n>] [ --whitespace=<option>] [-m | --merge]]
> [--onto <newbase>] <upstream> [<branch>]
>
> or perhaps even separated into interactive / non-interactive merge?
Or the --merge code path should check if there was any --whitespace
option, and re-apply each commit using something like
git reset --hard HEAD^ &&
git diff-tree HEAD@{1}..HEAD |
git apply --index <whitespace options>
Ciao,
Dscho
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: rebase -i and --whitespace, was Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-04 15:42 ` Jakub Narebski
2008-02-04 16:32 ` Johannes Schindelin
@ 2008-02-05 23:43 ` Junio C Hamano
2008-02-06 1:00 ` Johannes Schindelin
1 sibling, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2008-02-05 23:43 UTC (permalink / raw)
To: Jakub Narebski
Cc: Johannes Schindelin,
しらいしななこ,
Jari Aalto, git
Jakub Narebski <jnareb@gmail.com> writes:
> So it means that synopsis should, instead of current (pre-patch)
>
> 'git-rebase' [-i | --interactive] [-v | --verbose] [-m | --merge]
> [-C<n>] [ --whitespace=<option>] [-p | --preserve-merges]
> [--onto <newbase>] <upstream> [<branch>]
>
> should read
>
> 'git-rebase' [-v | --verbose] [-p | --preserve-merges]
> [{-i | --interactive} | [-C<n>] [ --whitespace=<option>] [-m | --merge]]
> [--onto <newbase>] <upstream> [<branch>]
>
> or perhaps even separated into interactive / non-interactive merge?
I think the reality is:
* -i ignores the lack of -m (i.e. do not use the slow "merge");
* Rebase with -m cannot use -C<n> and --whitespace (hence -i
because it forces -m);
* -p is only meaningful when using -m;
Three possible courses of actions are:
(1) fix merge codepath (this is involved --- we would need to
teach xdl_merge() to honor --whitespace={warn|error|fix}
and -C<n>); or
(2) fix -i so that it does not force -m; or
(3) adjust the description to reality.
Obviously the easiest would be to document the behaviour as-is,
but I suspect (2) would be the best practical solution if we
wanted to have any improvement compared to the current
situation.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: rebase -i and --whitespace, was Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)
2008-02-05 23:43 ` Junio C Hamano
@ 2008-02-06 1:00 ` Johannes Schindelin
0 siblings, 0 replies; 20+ messages in thread
From: Johannes Schindelin @ 2008-02-06 1:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jakub Narebski,
しらいしななこ,
Jari Aalto, git
Hi,
On Tue, 5 Feb 2008, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > So it means that synopsis should, instead of current (pre-patch)
> >
> > 'git-rebase' [-i | --interactive] [-v | --verbose] [-m | --merge]
> > [-C<n>] [ --whitespace=<option>] [-p | --preserve-merges]
> > [--onto <newbase>] <upstream> [<branch>]
> >
> > should read
> >
> > 'git-rebase' [-v | --verbose] [-p | --preserve-merges]
> > [{-i | --interactive} | [-C<n>] [ --whitespace=<option>] [-m | --merge]]
> > [--onto <newbase>] <upstream> [<branch>]
> >
> > or perhaps even separated into interactive / non-interactive merge?
>
> I think the reality is:
>
> * -i ignores the lack of -m (i.e. do not use the slow "merge");
>
> * Rebase with -m cannot use -C<n> and --whitespace (hence -i
> because it forces -m);
>
> * -p is only meaningful when using -m;
Even worse, AFAIR -p only works with -i.
> Three possible courses of actions are:
>
> (1) fix merge codepath (this is involved --- we would need to
> teach xdl_merge() to honor --whitespace={warn|error|fix}
> and -C<n>); or
>
> (2) fix -i so that it does not force -m; or
>
> (3) adjust the description to reality.
>
> Obviously the easiest would be to document the behaviour as-is,
> but I suspect (2) would be the best practical solution if we
> wanted to have any improvement compared to the current
> situation.
Granted. A long time ago, I started making a builtin from rebase/am, but
I got sidetracked pretty early.
However, I think that (2) and builtinification are pretty related, and can
be done in one go.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2008-02-06 1:01 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-03 20:19 [PATCH] git-rebase.sh: Update USAGE string Jari Aalto
2008-02-03 21:14 ` Jakub Narebski
2008-02-03 23:33 ` [PATCH] git-rebase.sh: Update USAGE string (No. 1) Jari Aalto
2008-02-03 23:41 ` Johannes Schindelin
2008-02-03 23:52 ` Jakub Narebski
2008-02-04 0:11 ` Jari Aalto
2008-02-04 0:27 ` Johannes Schindelin
2008-02-04 0:39 ` Junio C Hamano
2008-02-04 10:15 ` Jakub Narebski
2008-02-04 11:57 ` Mike Hommey
2008-02-04 12:53 ` Jakub Narebski
2008-02-04 0:24 ` Junio C Hamano
2008-02-04 8:04 ` [PATCH v3] git-rebase.sh: Update USAGE string Jari Aalto
2008-02-04 11:12 ` [PATCH] git-rebase.sh: Update USAGE string (No. 1) しらいしななこ
2008-02-04 15:06 ` rebase -i and --whitespace, was " Johannes Schindelin
2008-02-04 15:42 ` Jakub Narebski
2008-02-04 16:32 ` Johannes Schindelin
2008-02-05 23:43 ` Junio C Hamano
2008-02-06 1:00 ` Johannes Schindelin
2008-02-04 0:16 ` [PATCH v2] git-rebase.sh: Update USAGE string Jari Aalto
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).