Re: [PATCH] rev-list-options: clarify the usage of -n/--max-number

[Pranit Bauva]

Hey Junio,

On Tue, Sep 27, 2016 at 10:21 PM, Junio C Hamano  wrote:
> Pranit Bauva  writes:
>
>> -n=, -, --max-number= shows the last n commits
>> specified in  irrespective of whether --reverse is used or not.
>> With --reverse, it just shows the last n commits in reverse order.
>
> I think it is easier to understand if you updated the description of
> "--reverse", rather than "-".  "rev-list -n $N" that stops after
> showing $N commits is something everybody understands.  What often
> dissapoints some users is that "--reverse" kicks in _after_ what
> commits are to be shown are decided.

True.

>>  Documentation/rev-list-options.txt | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/rev-list-options.txt 
>> b/Documentation/rev-list-options.txt
>> index 7e462d3..6b7c2e5 100644
>> --- a/Documentation/rev-list-options.txt
>> +++ b/Documentation/rev-list-options.txt
>> @@ -18,7 +18,7 @@ ordering and formatting options, such as `--reverse`.
>>  -::
>>  -n ::
>>  --max-count=::
>> - Limit the number of commits to output.
>> + Limit to last n number of commits to output specified in .
>
> These essentially say the same thing.  The original does not mention
> where and how  is used, but "Limit the number of commits" as
> a description for "-" would be understood by anybody halfway
> intelligent that the given number is used as that limit, so I do not
> think an updated description is making it easier to understand.

To clear out that confused I used the word "last" but I can now
understand that it can be easily misunderstood.

> There is a paragraph of interest in an earlier part of "Commit
> Limiting" section (which is the section "-n" appears in, among other
> options):
>
> Note that these are applied before commit
> ordering and formatting options, such as `--reverse`.
>
> So the documentation already makes an attempt to avoid confusion
> Ruediger saw, i.e. "rev-list traverses, limits the output to N, and
> then shows these N commits in reverse" is what it expects readers to
> understand, and that it also expects it would lead naturally to
> "these N commits are still from the newest part of the history,
> hence 'rev-list --reverse -n N' is not how you grab the earliest N".

It surely does :)

> But apparently the attempt by the current documentation is not
> enough.  Let's see how it describes the '--reverse' option:
>
> Commit Ordering
> ~~~
>
> By default, the commits are shown in reverse chronological order.
> ...
>
> --reverse::
> Output the commits in reverse order.
> Cannot be combined with `--walk-reflogs`.
>
> Perhaps "Output the commits chosen to be shown (see Commit Limiting
> section above) in reverse order." would make it clearer?

That would be a much better edit. Thanks! Will send out a re-roll.

Regards,
Pranit Bauva

Re: [PATCH] rev-list-options: clarify the usage of -n/--max-number

[Junio C Hamano]

Pranit Bauva  writes:

> -n=, -, --max-number= shows the last n commits
> specified in  irrespective of whether --reverse is used or not.
> With --reverse, it just shows the last n commits in reverse order.

I think it is easier to understand if you updated the description of
"--reverse", rather than "-".  "rev-list -n $N" that stops after
showing $N commits is something everybody understands.  What often
dissapoints some users is that "--reverse" kicks in _after_ what
commits are to be shown are decided.

>  Documentation/rev-list-options.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/rev-list-options.txt 
> b/Documentation/rev-list-options.txt
> index 7e462d3..6b7c2e5 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -18,7 +18,7 @@ ordering and formatting options, such as `--reverse`.
>  -::
>  -n ::
>  --max-count=::
> - Limit the number of commits to output.
> + Limit to last n number of commits to output specified in .

These essentially say the same thing.  The original does not mention
where and how  is used, but "Limit the number of commits" as
a description for "-" would be understood by anybody halfway
intelligent that the given number is used as that limit, so I do not
think an updated description is making it easier to understand.

There is a paragraph of interest in an earlier part of "Commit
Limiting" section (which is the section "-n" appears in, among other
options):

Note that these are applied before commit
ordering and formatting options, such as `--reverse`.

So the documentation already makes an attempt to avoid confusion
Ruediger saw, i.e. "rev-list traverses, limits the output to N, and
then shows these N commits in reverse" is what it expects readers to
understand, and that it also expects it would lead naturally to
"these N commits are still from the newest part of the history,
hence 'rev-list --reverse -n N' is not how you grab the earliest N".

But apparently the attempt by the current documentation is not
enough.  Let's see how it describes the '--reverse' option:

Commit Ordering
~~~

By default, the commits are shown in reverse chronological order.
...

--reverse::
Output the commits in reverse order.
Cannot be combined with `--walk-reflogs`.

Perhaps "Output the commits chosen to be shown (see Commit Limiting
section above) in reverse order." would make it clearer?

[PATCH] rev-list-options: clarify the usage of -n/--max-number

[Pranit Bauva]

-n=, -, --max-number= shows the last n commits
specified in  irrespective of whether --reverse is used or not.
With --reverse, it just shows the last n commits in reverse order.

Reported-by: Ruediger Meier 
Signed-off-by: Pranit Bauva 

---
Hey Ruegiger,

The description is a bit inappropriate for --max-count and thus this
patch.

I cannot comment whether --max-count=-n would be a good choice or not
because personally I never left the need of it. I normally use --reverse
so as to review my patches in a branch serially. So for me the current
usage of --reverse seems more appropriate.
---
 Documentation/rev-list-options.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 7e462d3..6b7c2e5 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -18,7 +18,7 @@ ordering and formatting options, such as `--reverse`.
 -::
 -n ::
 --max-count=::
-   Limit the number of commits to output.
+   Limit to last n number of commits to output specified in .
 
 --skip=::
Skip 'number' commits before starting to show the commit output.

--
https://github.com/git/git/pull/296