Re: [PATCH v3 1/2] revision: differentiate if --no-abbrev asked explicitly

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 14, 2020, 00:50 UTC
Message-ID: <xmqq8sei125k.fsf@gitster.c.googlers.com>
In-Reply-To: <9a26c5b6110081cd8d029f2ab0327c4a1d228ef7.1597364493.git.congdanhqx@gmail.com>

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

Show 13 quoted lines

> When we see --no-abbrev in command's arguments, we reset the 'abbrev'
> field in diff-options to 0 and this value will be looked at
> diff_abbrev_oid() to decide not to truncate the object name.
>
> In a later change, we want to extend --abbrev support to diff-patch
> format. When --abbrev supporting diff-patch, we need to differentiate
> those below scenarios:
>
> * None of those options --abbrev, --no-abbrev, and --full-index are
>   asked. diff-patch should keep old behavior of using DEFAULT_ABBREV
>   for the index length.
> * --no-abbrev is asked, diff-patch should treat this option as same as
>   --full-index and show full object name in index line.

Sorry, but are you saying that the above two cases cannot be differentiated in the current code?

 * If none of --abbrev, --no-abbrev, --full-index are given, then
   diff.c::prep_parse_options() will leave options->flags.full_index
   and options->abbrev untouched.  They are initialized to false and
   DEFAULT_ABBREV (typically -1 when unconfigured).

 * If --no-abbrev is given, options->abbrev is set to 0.
   options->flags.full_index is not touched.

So you should be able to tell these two apart by only looking at options->flags.full_index bit. Perhaps, even though you said "we need to differentiate", you meant something else?

> While not doing anything is very effective way to show full object id,
> we couldn't differentiate if --no-abbrev or not.

Hmph. --no-abbrev without --full-index would not set flags.full_index bit; using --full-index would set the bit. Are you planning to do something special when BOTH --no-abbrev and --full-index is given? I am confused X-<.