public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Vladimir Panteleev <git@thecybershadow•net>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] show-ref: Allow --head to work with --verify
Date: Fri, 20 Jan 2017 11:03:23 -0800	[thread overview]
Message-ID: <xmqqa8aly2o4.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170120155015.4360-1-git@thecybershadow.net> (Vladimir Panteleev's message of "Fri, 20 Jan 2017 15:50:15 +0000")

Vladimir Panteleev <git@thecybershadow•net> writes:

> This patch adds --head support to show-ref's --verify logic, by
> explicitly checking if the "HEAD" ref is specified when --head is
> present.

> @@ -207,6 +207,8 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
>  				if (!quiet)
>  					show_one(*pattern, &oid);
>  			}
> +			else if (show_head && !strcmp(*pattern, "HEAD"))
> +				head_ref(show_ref, NULL);
>  			else if (!quiet)
>  				die("'%s' - not a valid ref", *pattern);
>  			else

The context around here look like this:

		while (*pattern) {
			struct object_id oid;

			if (starts_with(*pattern, "refs/") &&
			    !read_ref(*pattern, oid.hash)) {
				if (!quiet)
					show_one(*pattern, &oid);
			}
			else if (!quiet)
				die("'%s' - not a valid ref", *pattern);
			else
				return 1;
			pattern++;
		}

and viewed in the wider context, I notice that quiet is not honored
in the added code.  I think that is easily fixable by replacing this
hunk with something like:

-	if (starts_with(*pattern, "refs/") &&
+	if (to_show_ref(*pattern) &&

and then another hunk that implements to_show_ref() helper function,
perhaps like

	static int to_show_ref(const char *pattern)
	{
		if (starts_with(pattern, "refs/"))
			return 1;
		if (show_head && !strcmp(pattern, "HEAD"))
			return 1;
		return 0;
	}

or something.

Having said all that, using --verify on HEAD does not make much
sense, because if HEAD is missing in .git/, I do not think Git
considers that directory as a Git repository to begin with.  So from
that point of view, I am not sure what value this change adds to the
system, even though the change is almost correct (modulo the "quiet"
thing).

next prev parent reply	other threads:[~2017-01-20 19:03 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-20 15:50 [PATCH] show-ref: Allow --head to work with --verify Vladimir Panteleev
2017-01-20 19:03 ` Junio C Hamano [this message]
2017-01-20 20:26   ` Vladimir Panteleev
2017-01-20 20:51     ` Vladimir Panteleev
2017-01-20 22:22     ` Junio C Hamano
2017-01-20 22:31       ` Junio C Hamano
2017-01-20 23:15       ` Junio C Hamano
2017-01-20 22:55   ` Vladimir Panteleev
2017-01-20 23:20     ` Junio C Hamano
2017-01-20 23:29       ` Junio C Hamano
2017-01-21  0:25         ` Vladimir Panteleev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqa8aly2o4.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@thecybershadow$(echo .)net \
    --cc=git@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox