From: Junio C Hamano <gitster@pobox•com>
To: Christian Couder <christian.couder@gmail•com>
Cc: git@vger•kernel.org, Jeff King <peff@peff•net>,
Ben Peart <Ben.Peart@microsoft•com>,
Jonathan Tan <jonathantanmy@google•com>,
Nguyen Thai Ngoc Duy <pclouds@gmail•com>,
Mike Hommey <mh@glandium•org>,
Lars Schneider <larsxschneider@gmail•com>,
Eric Wong <e@80x24•org>,
Christian Couder <chriscool@tuxfamily•org>
Subject: Re: [PATCH v2 2/8] t0021/rot13-filter: refactor packet reading functions
Date: Tue, 07 Nov 2017 10:15:00 +0900 [thread overview]
Message-ID: <xmqqo9oehotn.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20171105213836.11717-3-chriscool@tuxfamily.org> (Christian Couder's message of "Sun, 5 Nov 2017 22:38:30 +0100")
Christian Couder <christian.couder@gmail•com> writes:
> +sub packet_required_key_val_read {
> + my ( $key ) = @_;
> + my ( $res, $buf ) = packet_txt_read();
> + unless ( $res == -1 or ( $buf =~ s/^$key=// and $buf ne '' ) ) {
> + die "bad $key: '$buf'";
> + }
> + return ( $res, $buf );
> +}
The function calls itself "required", but it does not die when it
sees an unexpected EOF or an empty line. Neither of these cases
gives it a key (nor val), but it is not treated as an error.
That is curious, isn't it?
By the way, is it just me who finds this "unless" even less
unerstandable than the original in packet_txt_read() you modeled
this one after? The original depended on packet_bin_read() to die
on an unexpected EOF, so its "unless" made some sense (we know we
got some input, and it is an error for the input not to end with LF
unless it is an empty string). Negating the condition and making it
"if" may make it easier to understand, perhaps. I dunno.
next prev parent reply other threads:[~2017-11-07 1:15 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-05 21:38 [PATCH v2 0/8] Create Git/Packet.pm Christian Couder
2017-11-05 21:38 ` [PATCH v2 1/8] t0021/rot13-filter: fix list comparison Christian Couder
2017-11-07 1:00 ` Junio C Hamano
2017-11-05 21:38 ` [PATCH v2 2/8] t0021/rot13-filter: refactor packet reading functions Christian Couder
2017-11-07 1:15 ` Junio C Hamano [this message]
2017-11-07 6:34 ` Christian Couder
2017-11-05 21:38 ` [PATCH v2 3/8] t0021/rot13-filter: improve 'if .. elsif .. else' style Christian Couder
2017-11-05 21:38 ` [PATCH v2 4/8] t0021/rot13-filter: improve error message Christian Couder
2017-11-05 21:38 ` [PATCH v2 5/8] t0021/rot13-filter: add packet_initialize() Christian Couder
2017-11-05 21:38 ` [PATCH v2 6/8] t0021/rot13-filter: refactor checking final lf Christian Couder
2017-11-07 1:18 ` Junio C Hamano
2017-11-05 21:38 ` [PATCH v2 7/8] t0021/rot13-filter: add capability functions Christian Couder
2017-11-07 1:24 ` Junio C Hamano
2017-11-05 21:38 ` [PATCH v2 8/8] Add Git/Packet.pm from parts of t0021/rot13-filter.pl Christian Couder
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=xmqqo9oehotn.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=Ben.Peart@microsoft$(echo .)com \
--cc=chriscool@tuxfamily$(echo .)org \
--cc=christian.couder@gmail$(echo .)com \
--cc=e@80x24$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=jonathantanmy@google$(echo .)com \
--cc=larsxschneider@gmail$(echo .)com \
--cc=mh@glandium$(echo .)org \
--cc=pclouds@gmail$(echo .)com \
--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