From: Junio C Hamano <gitster@pobox•com>
To: Ronnie Sahlberg <sahlberg@google•com>
Cc: git@vger•kernel.org, jrnieder@gmail•com, sunshine@sunshineco•com
Subject: Re: [PATCH] refs.c: change read_ref_at to use the reflog iterators
Date: Mon, 02 Jun 2014 14:21:48 -0700 [thread overview]
Message-ID: <xmqq4n033t3n.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1401732941-6498-1-git-send-email-sahlberg@google.com> (Ronnie Sahlberg's message of "Mon, 2 Jun 2014 11:15:41 -0700")
Ronnie Sahlberg <sahlberg@google•com> writes:
> read_ref_at has its own parsing of the reflog file for no really good reason
> so lets change this to use the existing reflog iterators. This removes one
> instance where we manually unmarshall the reflog file format.
>
> Log messages for errors are changed slightly. We no longer print the file
> name for the reflog, instead we refer to it as 'Log for ref <refname>'.
> This might be a minor useability regression, but I don't really think so, since
> experienced users would know where the log is anyway and inexperienced users
> would not know what to do about/how to repair 'Log ... has gap ...' anyway.
>
> Adapt the t1400 test to handle the change in log messages.
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@google•com>
> ---
> refs.c | 202 ++++++++++++++++++++++++++------------------------
> t/t1400-update-ref.sh | 4 +-
> 2 files changed, 107 insertions(+), 99 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 6898263..b45bb2f 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2936,109 +2936,117 @@ static char *ref_msg(const char *line, const char *endp)
> return xmemdupz(line, ep - line);
> }
If I am not mistaken, this function will become unused with this
rewrite. Let's drop it and justify it in the log message.
> +struct read_ref_at_cb {
> + const char *refname;
> + unsigned long at_time;
> + int cnt;
> + int reccnt;
> + unsigned char *sha1;
> + int found_it;
> +
> + char osha1[20];
> + char nsha1[20];
These should be unsigned char, shouldn't they?
> + for_each_reflog_ent_reverse(refname, read_ref_at_ent, &cb);
> +
> + if (!cb.reccnt)
> + die("Log for %s is empty.", refname);
> + if (cb.found_it)
> + return 0;
> +
> + for_each_reflog_ent(refname, read_ref_at_ent_oldest, &cb);
Hmph.
We have already scanned the same reflog in the backward direction in
full. Do we need to make another call just to pick up the entry at
the beginning? Is this because the callback is not told that it is
fed the last entry (in other words, if there is some clue that this
is the last call from for-each-reflog-ent-reverse to the callback,
the callback could record the value it received in its cb-data)?
next prev parent reply other threads:[~2014-06-02 21:21 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-02 18:15 [PATCH] refs.c: change read_ref_at to use the reflog iterators Ronnie Sahlberg
2014-06-02 21:21 ` Junio C Hamano [this message]
2014-06-03 16:08 ` Ronnie Sahlberg
-- strict thread matches above, loose matches on Subject: below --
2014-06-03 16:09 Ronnie Sahlberg
2014-05-30 19:51 Ronnie Sahlberg
2014-05-30 21:59 ` Junio C Hamano
2014-06-02 18:12 ` Ronnie Sahlberg
2014-05-30 22:51 ` Eric Sunshine
2014-06-02 17:59 ` Ronnie Sahlberg
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=xmqq4n033t3n.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jrnieder@gmail$(echo .)com \
--cc=sahlberg@google$(echo .)com \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox