Re: [PATCH 4/4] revision: do not fallback to default when rev_input_given is set

[Stefan Beller <sbeller@google.com>]

On Wed, Aug 2, 2017 at 3:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> If revs->def is set (as it is in "git log") and there are no
>> pending objects after parsing the user's input, then we show
>> whatever is in "def". But if the user _did_ ask for some
>> input that just happened to be empty (e.g., "--glob" that
>> does not match anything), showing the default revision is
>> confusing. We should just show nothing, as that is what the
>> user's request yielded.
>>
>> Signed-off-by: Jeff King <peff@peff.net>
>> ---
>> The "!got_rev_arg" that's already in the conditional is interesting. I
>> wondered if it could be subsumed by the rev_input_given flag. But
>> digging in the history, I think it's mostly about doing reflog walks.
>> Usually if we see a rev arg it will result either in an object added to
>> the pending queue, or a fatal error. But empty reflogs are the
>> exception. And since my other nearby series adds a separate check for
>> "are we doing an empty reflog walk", I don't think it makes sense to
>> tangle this up the new flag I'm adding here.
>
> OK, I'll have to stare at possible merge conflicts to see if I like
> this or some other design decision ;-)
>
> This shows one of the reasons why I want consumers of revision
> machinery not to be futzing these internal implementation detail
> bits in the revs structure, by the way.

cc'd Johannes to see this example and discussion.

>
>>  revision.c     | 2 +-
>>  t/t4202-log.sh | 6 ++++++
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/revision.c b/revision.c
>> index 08d5806b8..ba2b166cd 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -2316,7 +2316,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>>               opt->tweak(revs, opt);
>>       if (revs->show_merge)
>>               prepare_show_merge(revs);
>> -     if (revs->def && !revs->pending.nr && !got_rev_arg) {
>> +     if (revs->def && !revs->pending.nr && !revs->rev_input_given && !got_rev_arg) {
>>               struct object_id oid;
>>               struct object *object;
>>               struct object_context oc;
>> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
>> index 3f3531f0a..36d120c96 100755
>> --- a/t/t4202-log.sh
>> +++ b/t/t4202-log.sh
>> @@ -1523,6 +1523,12 @@ test_expect_success 'log diagnoses bogus HEAD' '
>>       test_i18ngrep broken stderr
>>  '
>>
>> +test_expect_success 'log does not default to HEAD when rev input is given' '
>> +     >expect &&
>> +     git log --branches=does-not-exist >actual &&
>> +     test_cmp expect actual
>> +'
>> +
>>  test_expect_success 'set up --source tests' '
>>       git checkout --orphan source-a &&
>>       test_commit one &&