From: Junio C Hamano <gitster@pobox•com>
To: Kevin Daudt <me@ikke•info>
Cc: Jeff King <peff@peff•net>, Jonathan Nieder <jrnieder@gmail•com>,
Marko Kungla <marko.kungla@gmail•com>,
git@vger•kernel.org
Subject: Re: [PATCH] check-ref-format: require a repository for --branch
Date: Tue, 17 Oct 2017 11:40:17 +0900 [thread overview]
Message-ID: <xmqq376iqyy6.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20171017012718.GC19335@alpha.vpn.ikke.info> (Kevin Daudt's message of "Tue, 17 Oct 2017 03:27:18 +0200")
Kevin Daudt <me@ikke•info> writes:
>> + setup_git_directory_gently(&nongit);
>> +
>> + if (!nongit)
>> + malformed = (strbuf_check_branch_ref(&sb, arg) ||
>> + !skip_prefix(sb.buf, "refs/heads/", &name));
>> + else
>> + malformed = check_branch_ref_format(arg);
>> +
>
> Would it make sense to swap the logic and get rid of the double
> negative (!nongit)?
I am trying to follow the pattern "handle the normal case that have
been supported forever first, and then handle new exception next",
so that it is easier to see that there is no behaviour change in the
normal case, so I do not think it makes it easier to see to swap the
if/else cases.
>
>> + if (malformed)
>> die("'%s' is not a valid branch name", arg);
>> - printf("%s\n", sb.buf + 11);
>> + printf("%s\n", name);
>> + strbuf_release(&sb);
>> return 0;
>> }
>>
next prev parent reply other threads:[~2017-10-17 2:40 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-08 1:17 bug with check-ref-format outside of repository Marko Kungla
2017-07-14 18:03 ` Jeff King
2017-07-14 18:18 ` [PATCH] check-ref-format: require a repository for --branch Jeff King
2017-07-17 17:27 ` Jonathan Nieder
2017-07-17 21:18 ` Marko Kungla
2017-08-17 10:23 ` Jeff King
2017-08-17 10:22 ` Jeff King
2017-08-17 21:30 ` Junio C Hamano
2017-08-18 6:20 ` Jeff King
2017-08-18 7:45 ` Junio C Hamano
2017-10-16 6:44 ` Junio C Hamano
2017-10-16 10:45 ` Junio C Hamano
2017-10-16 22:45 ` Jeff King
2017-10-16 23:00 ` Jonathan Nieder
2017-10-17 1:22 ` Junio C Hamano
2017-10-17 2:42 ` Jeff King
2017-10-17 3:33 ` Junio C Hamano
2017-10-17 4:44 ` Junio C Hamano
2017-10-17 7:06 ` [PATCH 0/3] " Jonathan Nieder
2017-10-17 7:08 ` [PATCH 1/3] check-ref-format --branch: do not expand @{...} outside repository Jonathan Nieder
2017-10-17 7:10 ` [PATCH 2/3] check-ref-format --branch: strip refs/heads/ using skip_prefix Jonathan Nieder
2017-10-17 7:12 ` [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch Junio C Hamano
2017-10-17 7:17 ` Jonathan Nieder
2017-10-17 8:21 ` Junio C Hamano
2017-10-17 7:12 ` [PATCH 3/3] check-ref-format doc: --branch validates and expands <branch> Jonathan Nieder
2017-10-17 20:55 ` Junio C Hamano
2017-10-17 21:06 ` Jonathan Nieder
2017-10-18 5:10 ` Jeff King
2017-10-17 1:27 ` [PATCH] check-ref-format: require a repository for --branch Kevin Daudt
2017-10-17 2:40 ` Junio C Hamano [this message]
2017-10-17 4:30 ` Kevin Daudt
2017-10-16 22:42 ` Jeff King
2017-10-17 4:41 ` Jonathan Nieder
2017-10-17 7:05 ` Junio C Hamano
2017-10-17 7:25 ` Jonathan Nieder
2017-10-17 7:34 ` Jonathan Nieder
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=xmqq376iqyy6.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jrnieder@gmail$(echo .)com \
--cc=marko.kungla@gmail$(echo .)com \
--cc=me@ikke$(echo .)info \
--cc=peff@peff$(echo .)net \
/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