Subject: Re: [PATCH v7 4/4] notes.c: don't do stripespace when parse file arg

From: Junio C Hamano <[email-masked]>
Date: Tue, 28 Mar 2023 08:54:02 -0700

Teng Long <[email-masked]> writes:

> From: Teng Long <[email-masked]>
>
> The file maybe is a binary and it could contains multiple line

"could contain"?

> breaks, if we do the stripespace on it's content, the notes will

"stripspace"?

> be different to the original content.

If a file is "binary" then by definition there is no "line" and no
"line break" in it, so while I can see what you are trying to say,
this needs a bit rephrasing to make it easier to follow for future
readers.  Perhaps something like...

    The file given to the "-F" option could be binary, in which case
    we want to store it verbatim without corrupting its contents.

But I do not necessarily agree with the logic here, regardless of
how it is phrased.  Existing users are used to seeing the contents
fed from "-F <file>" get cleaned up, and to them, this unconditional
removal of stripspace() will be a regression.  Besides, don't they
expect that 

	-m="$(cat file)"

be equivalent to

	-F file

for their text file?

A --binary-file=<file> option that is incompatible with -m/-F and
"append" action may make sense if we really want to have a feature
to help those who want to attach binary contents as a note to
objects.