Re: [PATCH 3/3] notes remove: --stdin reads from the standard input
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 19, 2011, 17:55 UTC
- Message-ID
- <7vliy27exx.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20110519105009.GA11107@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 13 quoted lines
> On Wed, May 18, 2011 at 05:14:21PM -0700, Junio C Hamano wrote:
>
>> + if (from_stdin) {
>> + struct strbuf sb = STRBUF_INIT;
>> + while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) {
>> + int len = sb.len;
>> + if (len && sb.buf[len - 1] == '\n')
>> + sb.buf[--len] = '\0';
>> + retval |= remove_one_note(t, sb.buf, flag);
>> + }
>> + }
>
> Wouldn't strbuf_rtrim (or even strbuf_trim) be useful here?Thanks for noticing.
I just mimicked what was done to the result of strbuf_getwholeline() in other places (I think from revision.c but I am not sure).
An incremental correction, relative to what I had in 'pu' overnight, looks like this.
builtin/notes.c | 5 ++--- t/t3301-notes.sh | 26 ++++++++++++++------------ 2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/builtin/notes.c b/builtin/notes.c index 164d605..f8e437d 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -1000,11 +1000,10 @@ static int remove_cmd(int argc, const char **argv, const char *prefix) if (from_stdin) { struct strbuf sb = STRBUF_INIT; while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) { - int len = sb.len; - if (len && sb.buf[len - 1] == '\n') - sb.buf[--len] = '\0'; + strbuf_rtrim(&sb); retval |= remove_one_note(t, sb.buf, flag); } + strbuf_release(&sb); } if (!retval) commit_notes(t, "Notes removed by 'git notes remove'"); diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh index e1b5619..16de05a 100755 --- a/t/t3301-notes.sh +++ b/t/t3301-notes.sh @@ -478,13 +478,14 @@ test_expect_success 'removing with --ignore-missing but bogus ref' ' test_expect_success 'remove reads from --stdin' ' before=$(git rev-parse --verify refs/notes/commits) && test_when_finished "git update-ref refs/notes/commits $before" && + + # We have only two -- add another and make sure it stays + git notes add -m "extra" && + git notes list HEAD >after-removal-expect && git rev-parse HEAD^^ HEAD^^^ >input && git notes remove --stdin <input && - git diff --name-only refs/notes/commits^ refs/notes/commits >actual && - test 2 = $(wc -l <actual) && - git ls-tree -r --name-only refs/notes/commits >actual && - >empty && - test_cmp empty actual + git notes list | sed -e "s/ .*//" >actual && + test_cmp after-removal-expect actual ' test_expect_success 'remove --stdin is also atomic' ' @@ -496,16 +497,17 @@ test_expect_success 'remove --stdin is also atomic' ' test "$before" = "$after" ' -test_expect_success 'removing with --stdin --missing-ok' ' +test_expect_success 'removing with --stdin --ignore-missing' ' before=$(git rev-parse --verify refs/notes/commits) && test_when_finished "git update-ref refs/notes/commits $before" && + + # We have only two -- add another and make sure it stays + git notes add -m "extra" && + git notes list HEAD >after-removal-expect && git rev-parse HEAD^^ HEAD^^^ HEAD^ >input && - git notes remove --missing-ok --stdin <input && - git diff --name-only refs/notes/commits^ refs/notes/commits >actual && - test 2 = $(wc -l <actual) && - git ls-tree -r --name-only refs/notes/commits >actual && - >empty && - test_cmp empty actual + git notes remove --ignore-missing --stdin <input && + git notes list | sed -e "s/ .*//" >actual && + test_cmp after-removal-expect actual ' test_expect_success 'list notes with "git notes list"' '