git-rebase.sh: Update USAGE string

Present the options in -s|--long (short, long) order.

Mention merge and new whitespace option.

git-rebase.sh +1 -1

-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>]'

or break synopsis into lines 80-column lines max.

ed-off-by: Jari Aalto <jari.aalto AT cante.net>

Welcome to FOSS revolution: we fix and modify until it shines

Subject: Re: [PATCH] git-rebase.sh: Update USAGE string (No. 1)

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>

+[-m|--merge] [--onto <newbase>] <upstream> [<branch>]' +

git-rebase.sh line 7

Quoted 4 lines · Jari Aalto

git-rebase.sh +2 -1

-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]

Quoted 2 lines

+[-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?

Quoted 3 lines · しらいしななこ

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?

Quoted 6 lines · Jakub Narebski

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>]

Quoted 7 lines

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.

Quoted 10 lines · Johannes Schindelin

Quoted 3 lines · しらいしななこ

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?

Besides here the mutually exclusive options are naturelly delimited, so you can say just (I think):
[--whitespace=nowarn|warn|error|error-all|strip]

Quoted 1 line

[--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.

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).

* Sun 2008-02-03 Jakub Narebski <[email protected]> INBOX

Quoted 3 lines

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] ...

Quoted 8 lines · Jari Aalto

* Sun 2008-02-03 Jakub Narebski <[email protected]> INBOX

Quoted 3 lines

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).

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...

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} ]

Quoted 7 lines · Jakub Narebski

Quoted 5 lines · Junio C Hamano

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

Quoted 4 lines

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.

* Sun 2008-02-03 Jakub Narebski <[email protected]> INBOX

Quoted 3 lines

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).

break synopsis into lines 80-column lines max.