Re: [PATCH] sequencer: remove use of comment character
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 30, 2023, 04:00 UTC
- Message-ID
- <xmqq7cn4g3nx.fsf@gitster.g>
- In-Reply-To
- <pull.1603.git.1698635292629.gitgitgadget@gmail.com>
"Tony Tung via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 5 quoted lines
> From: Tony Tung <tonytung@merly.org> > > Instead of using the hardcoded `# `, use the > user-defined comment_line_char. Adds a test > to prevent regressions.
Good spotting.
Two observations.
(1) There are a few more places that need similar treatment in the
same file; you may want to fix them all while at it. (2) The second argument to strbuf_commented_addf() is always the
comment_line_char global variable, not just inside this file
but all callers across the codebase. We probably should drop
it and have the strbuf_commented_addf() helper itself refer to
the global. That way, if we ever want to change the global
variable reference to something else (e.g. function call), we
only have to touch a single place.The latter is meant as #leftoverbits and will be a lot wider clean-up that we may want to do long after this patch hits out codebase. The "other places" I spotted for the former are the following, but needs to be taken with a huge grain of salt, as it has not even been compile tested.
Thanks.
sequencer.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git c/sequencer.c w/sequencer.c index d584cac8ed..33208b1660 100644 --- c/sequencer.c +++ w/sequencer.c @@ -1893,8 +1893,8 @@ static void update_squash_message_for_fixup(struct strbuf *msg) size_t orig_msg_len; int i = 1; - strbuf_addf(&buf1, "# %s\n", _(first_commit_msg_str)); - strbuf_addf(&buf2, "# %s\n", _(skip_first_commit_msg_str)); + strbuf_addf(&buf1, comment_line_char, "%s\n", _(first_commit_msg_str)); + strbuf_addf(&buf2, comment_line_char, "%s\n", _(skip_first_commit_msg_str)); s = start = orig_msg = strbuf_detach(msg, &orig_msg_len); while (s) { const char *next; @@ -2269,8 +2269,8 @@ static int do_pick_commit(struct repository *r, next = parent; next_label = msg.parent_label; if (opts->commit_use_reference) { - strbuf_addstr(&msgbuf, - "# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***"); + strbuf_commented_addf(&msgbuf, comment_line_char, "%s", + "*** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***"); } else if (skip_prefix(msg.subject, "Revert \"", &orig_subject) && /* * We don't touch pre-existing repeated reverts, because