Re: [PATCH v9 4/6] notes.c: introduce '--separator=<paragraph-break>' option

From: Kristoffer Haugsbakk <code@khaugsbakk.name>
Date: May 10, 2023, 19:19 UTC
Message-ID: <2908b005-9478-4c59-ae8e-150be44a15a9@app.fastmail.com>
In-Reply-To: <ed930ef4f795f30792bc14d9c1939484e4976db8.1682671758.git.dyroneteng@gmail.com>

I realize that this series is going to be merged to `master`.[1] I was trying out this new change since I might have some use for it when the next version is released.

On Fri, Apr 28, 2023, at 11:23, Teng Long wrote:

> From: Teng Long <dyroneteng@gmail.com>
>
> When adding new notes or appending to an existing notes, we will
> insert a blank line between the paragraphs, like:

The test case[2] `append: specify an empty separator` demonstrates that `--separator=""` is the same as the default behavior, namely to add a blank line. It has (according to the commit messages) been like this since v5 of this patch.[3]

v4 of this patch special-cased `--separator=""` to mean “no separator”. And this was the same behavior as the original `--no-blankline`,[4] which eventually mutated into `--separator`.

Why was this changed to act the same as the default behavior (add a blank line)? I can’t seem to find a note for it on the cover letter of v5 or in the relevant replies.

It seemed that v4 of this patch (with special-cased empty argument) was perhaps based on Eric Sunshine’s suggestion:[5]

Show 5 quoted lines

> Taking a step back, perhaps think of this in terms of "separator". The
> default behavior is to insert "\n" as a separator between notes. If
> you add a --separator option, then users could supply their own
> separator, such as "----\n" or, in your case, "" to suppress the blank
> line.

(And then reiterated in a v4 email [6])

Was the idea perhaps to eventually (separately) add a separate option which functions like `--each-message-is-line-not-paragraph`, like what was mentioned in [7]?

Maybe I’ve missed something. (I probably have.)

[1] https://lore.kernel.org/git/xmqqpm785erg.fsf@gitster.g/T/#md9b20801457c3eb24dc0e793f5dfbeae2f2707fd [2] On `next`, 74a8c73209 (Sync with 'master', 2023-05-09) [3] https://lore.kernel.org/git/a74c96d6dd23f2f1df6d3492093f3fd27451e24c.1676551077.git.dyroneteng@gmail.com/

   Commit message on v4:

   >  * --separator='': we specify an empty separator which means will
   >  append the message directly without inserting any separator at
   >  first.

   Commit message on v5:

   > * --separator='': we specify an empty separator which has the same
   > behavour with --separator='\n' and or not specified the option.

[4] https://lore.kernel.org/git/20221013055654.39628-1-tenglong.tl@alibaba-inc.com/ [5] https://lore.kernel.org/git/CAPig+cRcezSp4Rqt1Y9bD-FT6+7b0g9qHfbGRx65AOnw2FQXKg@mail.gmail.com/ [6] https://lore.kernel.org/git/CAPig+cSF7Fp3oM4TRU1QbiSzTeKNd1qGtqU7goPc1r-p4g8mkg@mail.gmail.com/ [7] https://lore.kernel.org/git/xmqqh6yh3nk4.fsf@gitster.g/

-- 
Kristoffer Haugsbakk