Re: [PATCH/WIP v3 06/31] am: detect mbox patches
- From
Paul Tan <pyokagan@gmail.com>
- Date
- Jun 24, 2015, 08:41 UTC
- Message-ID
- <CACRoPnSoFDnuJC-q3nR7xwpqOcpmzYReDPPPao1NJdY11TV0cQ@mail.gmail.com>
- In-Reply-To
- <xmqqy4jgd92l.fsf@gitster.dls.corp.google.com>
On Fri, Jun 19, 2015 at 5:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
> Paul Tan <pyokagan@gmail.com> writes:
>
>> +static int is_email(const char *filename)
>> +{
>> + struct strbuf sb = STRBUF_INIT;
>> + FILE *fp = xfopen(filename, "r");
>> + int ret = 1;
>> +
>> + while (!strbuf_getline(&sb, fp, '\n')) {
>> + const char *x;
>> +
>> + strbuf_rtrim(&sb);
>
> Is this a good thing? strbuf_getline() already has stripped the LF
> at the end, so you'd be treating a line with only whitespaces as if
> it is a truly empty line.
>
> I know the series is about literal translation and the script may
> lose the distinction between the two, but I do not think you need
> (or want) to be literally same for things like this.
>
> Same comment applies to other uses of "trim" in this patch.No, the uses of strbuf_trim() are not good at all. Will remove them.
Thanks, Paul