From: Jakub Narebski <hidden> Date: 2016-06-15 22:44:09
Jari Aalto [off-list ref] 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.
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 <redacted>
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]
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]
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
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:44:10
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
From: Jakub Narebski <hidden> Date: 2016-06-15 22:44:10
W poniedziałek 4. lutego 2008 16:06, Johannes Schindelin napisał:
On Mon, 4 Feb 2008, しらいしななこ wrote:
quoted
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
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:44:10
Hi,
On Mon, 4 Feb 2008, Jakub Narebski wrote:
W poniedziałek 4. lutego 2008 16:06, Johannes Schindelin napisał:
quoted
On Mon, 4 Feb 2008, しらいしななこ wrote:
quoted
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
From: Junio C Hamano <hidden> Date: 2016-06-15 22:44:10
Jakub Narebski [off-list ref] 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.
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:44:10
Hi,
On Tue, 5 Feb 2008, Junio C Hamano wrote:
Jakub Narebski [off-list ref] writes:
quoted
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