Teng Long <dyroneteng@xxxxxxxxx> writes:
> Actually, -m/-F/-c/-C in 'git notes add' is quite like the
> options in 'git commit', rather than "--binary-file=", I think
> maybe '--[no-]stripspace' is a bit better. Defaults to
> '--stripspace' which keeps old logic (stripspace the string fed by
> -m/-F). If specifying '--no-stripspace', we do not stripespace.
It is a fairly easy way to both implement and explain. The approach
is certainly attractive from the "expectation management" point of
view.
With "git notes add --no-stripspace -F file0 -F file1 ...", we may
still be making the resulting note unusable as a binary file due to
splicing the inter-paragraph separator, but the option name does not
hint that we are giving this option to users to help them use binary
contents at all, to prevent them from complaining. If it is truly
useful is a separate matter, but I somehow suspect that there will
be a lot more users who want "no stripspace on text contents" than
those who want "binary contents in note", so it probably is a good
way to go after all. I like that.
Before proceeding further on this front, we'd probably need a bit
more tests that are specifically designed to catch behaviour change
before and after this series with respect to stripspace behaviour.
The current code without these patches calls stripspace() on the
whole thing every time it receives and appends an additional piece
of contents. The parse_msg_arg() function, for example, handles "-m
<possibly multiline message>" by first appending a blank line (if we
already have some contents) and then appending the new message, and
runs stripspace on the whole thing. "-F <file>" is handled by
parse_file_arg() the same way, i.e. append the file data and then
stripspace the result. This can of course be optimized somewhat
(e.g. an early part of the buffer may have already been cleansed by
stripspace before appending new data, so as long as we do not
introduce double-blanks and other things that are to be corrected at
the boundary of appending new data, we shouldn't have to run
stripspace on the earlier part in order to behave "as if we did a
stripspace the whole thing over and over again"), but we should make
sure we do not change the behaviour to cause regression. After
doing something silly like this:
cat >file1 <<-\EOF &&
With a leading blank line, this is the second line of the file.
And with a trailing blank line, this is the second from the last.
EOF
git notes add -m '
Message from the command line.
Two blank lines above.' -F file1
we should see the leading blank line before "Message from the
command line." removed, two blank lines from the "-m" option
squeezed into one, only one blank line to appear before the contents
taken from file1, and the trailing blank line removed.
Thanks.
