Đoàn Trần Công Danh <congdanhqx@xxxxxxxxx> writes:
> When we see `--no-abbrev' in command's arguments, we reset `abbrev' of
> `diff_options` to 0, thus, on a later stage, the object id won't
> be shortened (by not set object_id[abbrev] to '\0').
s/by not set/&ting/ probably?
Please do not write pseudo-code that does not exist anywhere in the
codebase that pretends to be a quote from some real code. It wasts
reviewer's time looking for non-existing code to see what really is
going on.
I think you meant this line in diff_abbrev_oid():
if (abbrev)
hex[abbrev] = '\0';
so you can say that more explicitly, perhaps:
... we reset the 'abbrev' field in diff-options to 0 and this
value is looked at diff_abbrev_oid() to decide not to truncate
the object name.
or somesuch.
> While not doing anything is very effective way to show full object id,
> we couldn't differentiate if --no-abbrev or not.
You mean you want to tell if --no-abbrev and/or --full-index was
given from the command line and act differently? You need to
justify why you need to be able to do so (which you attempted in the
remainder of the proposed log message), and also you need to justify
why changing revs->abbrev's meaning is the best way to do so (which
you did not). In fill_metainfo(), which you are going to touch in
[2/2], you can peek o->flags.full_index to see if --full-index was
given, and the fact that revs->abbrev is not DEFAULT_ABBREV (which
is how the field is initialized in repo_init_revisions()) but is 0
tells us that "--no-abbrev" was given. --abbrev=0 would have busted
minimum abbrev and set the field to MINIMUM_ABBREV, --abbrev=99 would
have busted hash size and set the field to hexsz. So 0 is the sign
that --no-abbrev was given. No?
> In a later change, we want to extend --abbrev support to diff-patch
> format.
And it is unclear why this planned change requires it.
> Let's ask for full object id if we see --no-abbrev instead.
Hence, this "let's ask" is not justified very well.
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@xxxxxxxxx>
> ---
> revision.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/revision.c b/revision.c
> index 3dcf689341..24027b1af3 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2430,7 +2430,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
> } else if (!strcmp(arg, "--always")) {
> revs->always_show_header = 1;
> } else if (!strcmp(arg, "--no-abbrev")) {
> - revs->abbrev = 0;
> + revs->abbrev = hexsz;
> } else if (!strcmp(arg, "--abbrev")) {
> revs->abbrev = DEFAULT_ABBREV;
> } else if (skip_prefix(arg, "--abbrev=", &optarg)) {
