git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Han-Wen Nienhuys <hanwenn@gmail.com>,
	Han-Wen Nienhuys <hanwen@google.com>
Subject: Re: [PATCH] refs: add \t to reflog in the files backend
Date: Fri, 31 Jul 2020 13:36:49 -0400
Message-ID: <20200731173649.GA843002@coredump.intra.peff.net> (raw)
In-Reply-To: <pull.688.git.1596195370757.gitgitgadget@gmail.com>

On Fri, Jul 31, 2020 at 11:36:10AM +0000, Han-Wen Nienhuys via GitGitGadget wrote:

> diff --git a/refs.c b/refs.c
> index 89814c7be4..2dd851fe81 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -907,7 +907,6 @@ static void copy_reflog_msg(struct strbuf *sb, const char *msg)
>  	char c;
>  	int wasspace = 1;
>  
> -	strbuf_addch(sb, '\t');
>  	while ((c = *msg++)) {
>  		if (wasspace && isspace(c))
>  			continue;
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index e0aba23eb2..985631f33e 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1628,8 +1628,10 @@ static int log_ref_write_fd(int fd, const struct object_id *old_oid,
>  	int ret = 0;
>  
>  	strbuf_addf(&sb, "%s %s %s", oid_to_hex(old_oid), oid_to_hex(new_oid), committer);
> -	if (msg && *msg)
> +	if (msg && *msg) {
> +		strbuf_addch(&sb, '\t');
>  		strbuf_addstr(&sb, msg);
> +	}

I wondered here whether the existing code would always write the tab,
even for an entry with no message, since we seem to unconditionally add
the tab in the hunk above.

But it looks like we only call copy_reflog_msg() if it's non-empty:

  static char *normalize_reflog_message(const char *msg)
  {
          struct strbuf sb = STRBUF_INIT;
  
          if (msg && *msg)
                  copy_reflog_msg(&sb, msg);
          return strbuf_detach(&sb, NULL);
  }

so this is retaining that behavior (and indeed, doing a simple "git
update-ref" confirms that we currently omit the tab in this case).

So the patch looks good (and the intent seems obviously good to me, as
well).

-Peff

      parent reply	other threads:[~2020-07-31 17:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-31 11:36 Han-Wen Nienhuys via GitGitGadget
2020-07-31 17:17 ` Junio C Hamano
2020-07-31 17:36 ` Jeff King [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20200731173649.GA843002@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=hanwen@google.com \
    --cc=hanwenn@gmail.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git