{"thread":{"id":"60447","subject":"[PATCH 0/2] Avoid passing global comment_line_char repeatedly","startedAt":"2023-10-30T05:10:42Z","lastAt":"2023-11-01T04:37:23Z","messageCount":21,"participants":["Junio C Hamano","Dragan Simic","Phillip Wood","Jonathan Tan"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"484071","messageId":"20231030051034.2295242-1-gitster@pobox.com","threadId":"60447","inReplyTo":null,"subject":"[PATCH 0/2] Avoid passing global comment_line_char repeatedly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-30T05:10:32Z","receivedAt":"2023-10-30T05:10:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Two strbuf functions used to produce commented lines take the\ncomment_line_char as their parameter, but in practice, all callers\nfeed the global variable comment_line_char from environment.[ch].\n\nDropping the parameter from the callchain will make the interface\nless flexible, and less error prone. If we choose to change the\nimplementation of the customizable comment line character (e.g., we\nmay want to stop referencing the global variable and instead use a\ngetter function), we will have fewer places we need to modify.\n\nJunio C Hamano (2):\n strbuf_commented_addf(): drop the comment_line_char parameter\n strbuf_add_commented_lines(): drop the comment_line_char parameter\n\n add-patch.c | 8 ++++----\n builtin/branch.c | 2 +-\n builtin/merge.c | 8 ++++----\n builtin/notes.c | 9 ++++-----\n builtin/stripspace.c | 2 +-\n builtin/tag.c | 4 ++--\n fmt-merge-msg.c | 9 +++------\n rebase-interactive.c | 8 ++++----\n sequencer.c | 14 ++++++--------\n strbuf.c | 9 +++++----\n strbuf.h | 7 +++----\n wt-status.c | 6 +++---\n 12 files changed, 40 insertions(+), 46 deletions(-)\n\n-- \n2.42.0-526-g3130c155df\n\n"},{"id":"484072","messageId":"20231030051034.2295242-2-gitster@pobox.com","threadId":"60447","inReplyTo":"20231030051034.2295242-1-gitster@pobox.com","subject":"[PATCH 1/2] strbuf_commented_addf(): drop the comment_line_char parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-30T05:10:33Z","receivedAt":"2023-10-30T05:10:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"All the callers of this function supply the global variable\ncomment_line_char as an argument to its second parameter. Remove\nthe parameter to allow us in the future to change the reference to\nthe global variable with something else, like a function call.\n\nSigned-off-by: Junio C Hamano \n---\n add-patch.c | 8 ++++----\n builtin/branch.c | 2 +-\n builtin/merge.c | 8 ++++----\n builtin/tag.c | 4 ++--\n rebase-interactive.c | 2 +-\n sequencer.c | 4 ++--\n strbuf.c | 3 ++-\n strbuf.h | 4 ++--\n wt-status.c | 2 +-\n 9 files changed, 19 insertions(+), 18 deletions(-)\n\ndiff --git a/add-patch.c b/add-patch.c\nindex bfe19876cd..471a0037be 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -1106,11 +1106,11 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)\n \tsize_t i;\n \n \tstrbuf_reset(&s->buf);\n-\tstrbuf_commented_addf(&s->buf, comment_line_char,\n+\tstrbuf_commented_addf(&s->buf,\n \t\t\t _(\"Manual hunk edit mode -- see bottom for \"\n \t\t\t\t\"a quick guide.\\n\"));\n \trender_hunk(s, hunk, 0, 0, &s->buf);\n-\tstrbuf_commented_addf(&s->buf, comment_line_char,\n+\tstrbuf_commented_addf(&s->buf,\n \t\t\t _(\"---\\n\"\n \t\t\t\t\"To remove '%c' lines, make them ' ' lines \"\n \t\t\t\t\"(context).\\n\"\n@@ -1119,13 +1119,13 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)\n \t\t\t s->mode->is_reverse ? '+' : '-',\n \t\t\t s->mode->is_reverse ? '-' : '+',\n \t\t\t comment_line_char);\n-\tstrbuf_commented_addf(&s->buf, comment_line_char, \"%s\",\n+\tstrbuf_commented_addf(&s->buf, \"%s\",\n \t\t\t _(s->mode->edit_hunk_hint));\n \t/*\n \t * TRANSLATORS: 'it' refers to the patch mentioned in the previous\n \t * messages.\n \t */\n-\tstrbuf_commented_addf(&s->buf, comment_line_char,\n+\tstrbuf_commented_addf(&s->buf,\n \t\t\t _(\"If it does not apply cleanly, you will be \"\n \t\t\t\t\"given an opportunity to\\n\"\n \t\t\t\t\"edit again. If all lines of the hunk are \"\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 2ec190b14a..b2f171e10b 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -668,7 +668,7 @@ static int edit_branch_description(const char *branch_name)\n \texists = !read_branch_desc(&buf, branch_name);\n \tif (!buf.len || buf.buf[buf.len-1] != '\\n')\n \t\tstrbuf_addch(&buf, '\\n');\n-\tstrbuf_commented_addf(&buf, comment_line_char,\n+\tstrbuf_commented_addf(&buf,\n \t\t _(\"Please edit the description for the branch\\n\"\n \t\t \" %s\\n\"\n \t\t \"Lines starting with '%c' will be stripped.\\n\"),\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex d748d46e13..8f0e8be7c3 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -857,15 +857,15 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t\tstrbuf_addch(&msg, '\\n');\n \t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n \t\t\twt_status_append_cut_line(&msg);\n-\t\t\tstrbuf_commented_addf(&msg, comment_line_char, \"\\n\");\n+\t\t\tstrbuf_commented_addf(&msg, \"\\n\");\n \t\t}\n-\t\tstrbuf_commented_addf(&msg, comment_line_char,\n+\t\tstrbuf_commented_addf(&msg,\n \t\t\t\t _(merge_editor_comment));\n \t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)\n-\t\t\tstrbuf_commented_addf(&msg, comment_line_char,\n+\t\t\tstrbuf_commented_addf(&msg,\n \t\t\t\t\t _(scissors_editor_comment));\n \t\telse\n-\t\t\tstrbuf_commented_addf(&msg, comment_line_char,\n+\t\t\tstrbuf_commented_addf(&msg,\n \t\t\t\t_(no_scissors_editor_comment), comment_line_char);\n \t}\n \tif (signoff)\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 3918eacbb5..a85a0d8def 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -314,10 +314,10 @@ static void create_tag(const struct object_id *object, const char *object_ref,\n \t\t\tstruct strbuf buf = STRBUF_INIT;\n \t\t\tstrbuf_addch(&buf, '\\n');\n \t\t\tif (opt->cleanup_mode == CLEANUP_ALL)\n-\t\t\t\tstrbuf_commented_addf(&buf, comment_line_char,\n+\t\t\t\tstrbuf_commented_addf(&buf,\n \t\t\t\t _(tag_template), tag, comment_line_char);\n \t\t\telse\n-\t\t\t\tstrbuf_commented_addf(&buf, comment_line_char,\n+\t\t\t\tstrbuf_commented_addf(&buf,\n \t\t\t\t _(tag_template_nocleanup), tag, comment_line_char);\n \t\t\twrite_or_die(fd, buf.buf, buf.len);\n \t\t\tstrbuf_release(&buf);\ndiff --git a/rebase-interactive.c b/rebase-interactive.c\nindex d9718409b3..3f33da7f03 100644\n--- a/rebase-interactive.c\n+++ b/rebase-interactive.c\n@@ -71,7 +71,7 @@ void append_todo_help(int command_count,\n \n \tif (!edit_todo) {\n \t\tstrbuf_addch(buf, '\\n');\n-\t\tstrbuf_commented_addf(buf, comment_line_char,\n+\t\tstrbuf_commented_addf(buf,\n \t\t\t\t Q_(\"Rebase %s onto %s (%d command)\",\n \t\t\t\t\t \"Rebase %s onto %s (%d commands)\",\n \t\t\t\t\t command_count),\ndiff --git a/sequencer.c b/sequencer.c\nindex d584cac8ed..5d348a3f12 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -675,11 +675,11 @@ void append_conflicts_hint(struct index_state *istate,\n \t}\n \n \tstrbuf_addch(msgbuf, '\\n');\n-\tstrbuf_commented_addf(msgbuf, comment_line_char, \"Conflicts:\\n\");\n+\tstrbuf_commented_addf(msgbuf, \"Conflicts:\\n\");\n \tfor (i = 0; i < istate->cache_nr;) {\n \t\tconst struct cache_entry *ce = istate->cache[i++];\n \t\tif (ce_stage(ce)) {\n-\t\t\tstrbuf_commented_addf(msgbuf, comment_line_char,\n+\t\t\tstrbuf_commented_addf(msgbuf,\n \t\t\t\t\t \"\\t%s\\n\", ce->name);\n \t\t\twhile (i < istate->cache_nr &&\n \t\t\t !strcmp(ce->name, istate->cache[i]->name))\ndiff --git a/strbuf.c b/strbuf.c\nindex 7827178d8e..15550b2619 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -1,4 +1,5 @@\n #include \"git-compat-util.h\"\n+#include \"environment.h\"\n #include \"gettext.h\"\n #include \"hex-ll.h\"\n #include \"strbuf.h\"\n@@ -372,7 +373,7 @@ void strbuf_add_commented_lines(struct strbuf *out, const char *buf,\n \tadd_lines(out, prefix1, prefix2, buf, size);\n }\n \n-void strbuf_commented_addf(struct strbuf *sb, char comment_line_char,\n+void strbuf_commented_addf(struct strbuf *sb,\n \t\t\t const char *fmt, ...)\n {\n \tva_list params;\ndiff --git a/strbuf.h b/strbuf.h\nindex e959caca87..981617dc77 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -378,8 +378,8 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...);\n * Add a formatted string prepended by a comment character and a\n * blank to the buffer.\n */\n-__attribute__((format (printf, 3, 4)))\n-void strbuf_commented_addf(struct strbuf *sb, char comment_line_char, const char *fmt, ...);\n+__attribute__((format (printf, 2, 3)))\n+void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);\n \n __attribute__((format (printf,2,0)))\n void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);\ndiff --git a/wt-status.c b/wt-status.c\nindex 9f45bf6949..54b2775730 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1102,7 +1102,7 @@ void wt_status_append_cut_line(struct strbuf *buf)\n {\n \tconst char *explanation = _(\"Do not modify or remove the line above.\\nEverything below it will be ignored.\");\n \n-\tstrbuf_commented_addf(buf, comment_line_char, \"%s\", cut_line);\n+\tstrbuf_commented_addf(buf, \"%s\", cut_line);\n \tstrbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);\n }\n \n-- \n2.42.0-526-g3130c155df\n\n"},{"id":"484073","messageId":"20231030051034.2295242-3-gitster@pobox.com","threadId":"60447","inReplyTo":"20231030051034.2295242-1-gitster@pobox.com","subject":"[PATCH 2/2] strbuf_add_commented_lines(): drop the comment_line_char parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-30T05:10:34Z","receivedAt":"2023-10-30T05:10:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"All the callers of this function supply the global variable\ncomment_line_char as an argument to its last parameter. Remove the\nparameter to allow us in the future to change the reference to the\nglobal variable with something else, like a function call.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/notes.c | 9 ++++-----\n builtin/stripspace.c | 2 +-\n fmt-merge-msg.c | 9 +++------\n rebase-interactive.c | 6 +++---\n sequencer.c | 10 ++++------\n strbuf.c | 6 +++---\n strbuf.h | 3 +--\n wt-status.c | 4 ++--\n 8 files changed, 21 insertions(+), 28 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 9f38863dd5..355ecce07a 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -181,7 +181,7 @@ static void write_commented_object(int fd, const struct object_id *object)\n \n \tif (strbuf_read(&buf, show.out, 0) < 0)\n \t\tdie_errno(_(\"could not read 'show' output\"));\n-\tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_char);\n+\tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len);\n \twrite_or_die(fd, cbuf.buf, cbuf.len);\n \n \tstrbuf_release(&cbuf);\n@@ -209,10 +209,9 @@ static void prepare_note_data(const struct object_id *object, struct note_data *\n \t\t\tcopy_obj_to_fd(fd, old_note);\n \n \t\tstrbuf_addch(&buf, '\\n');\n-\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"), comment_line_char);\n-\t\tstrbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)),\n-\t\t\t\t\t comment_line_char);\n-\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"), comment_line_char);\n+\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"));\n+\t\tstrbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));\n+\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"));\n \t\twrite_or_die(fd, buf.buf, buf.len);\n \n \t\twrite_commented_object(fd, object);\ndiff --git a/builtin/stripspace.c b/builtin/stripspace.c\nindex 7b700a9fb1..11e475760c 100644\n--- a/builtin/stripspace.c\n+++ b/builtin/stripspace.c\n@@ -13,7 +13,7 @@ static void comment_lines(struct strbuf *buf)\n \tsize_t len;\n \n \tmsg = strbuf_detach(buf, &len);\n-\tstrbuf_add_commented_lines(buf, msg, len, comment_line_char);\n+\tstrbuf_add_commented_lines(buf, msg, len);\n \tfree(msg);\n }\n \ndiff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\nindex 66e47449a0..adc85d2a72 100644\n--- a/fmt-merge-msg.c\n+++ b/fmt-merge-msg.c\n@@ -509,8 +509,7 @@ static void fmt_tag_signature(struct strbuf *tagbuf,\n \tstrbuf_complete_line(tagbuf);\n \tif (sig->len) {\n \t\tstrbuf_addch(tagbuf, '\\n');\n-\t\tstrbuf_add_commented_lines(tagbuf, sig->buf, sig->len,\n-\t\t\t\t\t comment_line_char);\n+\t\tstrbuf_add_commented_lines(tagbuf, sig->buf, sig->len);\n \t}\n }\n \n@@ -556,8 +555,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\t\t\tstrbuf_addch(&tagline, '\\n');\n \t\t\t\tstrbuf_add_commented_lines(&tagline,\n \t\t\t\t\t\torigins.items[first_tag].string,\n-\t\t\t\t\t\tstrlen(origins.items[first_tag].string),\n-\t\t\t\t\t\tcomment_line_char);\n+\t\t\t\t\t\tstrlen(origins.items[first_tag].string));\n \t\t\t\tstrbuf_insert(&tagbuf, 0, tagline.buf,\n \t\t\t\t\t tagline.len);\n \t\t\t\tstrbuf_release(&tagline);\n@@ -565,8 +563,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\t\tstrbuf_addch(&tagbuf, '\\n');\n \t\t\tstrbuf_add_commented_lines(&tagbuf,\n \t\t\t\t\torigins.items[i].string,\n-\t\t\t\t\tstrlen(origins.items[i].string),\n-\t\t\t\t\tcomment_line_char);\n+\t\t\t\t\tstrlen(origins.items[i].string));\n \t\t\tfmt_tag_signature(&tagbuf, &sig, buf, len);\n \t\t}\n \t\tstrbuf_release(&payload);\ndiff --git a/rebase-interactive.c b/rebase-interactive.c\nindex 3f33da7f03..1138bd37ba 100644\n--- a/rebase-interactive.c\n+++ b/rebase-interactive.c\n@@ -78,7 +78,7 @@ void append_todo_help(int command_count,\n \t\t\t\t shortrevisions, shortonto, command_count);\n \t}\n \n-\tstrbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char);\n+\tstrbuf_add_commented_lines(buf, msg, strlen(msg));\n \n \tif (get_missing_commit_check_level() == MISSING_COMMIT_CHECK_ERROR)\n \t\tmsg = _(\"\\nDo not remove any line. Use 'drop' \"\n@@ -87,7 +87,7 @@ void append_todo_help(int command_count,\n \t\tmsg = _(\"\\nIf you remove a line here \"\n \t\t\t \"THAT COMMIT WILL BE LOST.\\n\");\n \n-\tstrbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char);\n+\tstrbuf_add_commented_lines(buf, msg, strlen(msg));\n \n \tif (edit_todo)\n \t\tmsg = _(\"\\nYou are editing the todo file \"\n@@ -98,7 +98,7 @@ void append_todo_help(int command_count,\n \t\tmsg = _(\"\\nHowever, if you remove everything, \"\n \t\t\t\"the rebase will be aborted.\\n\\n\");\n \n-\tstrbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char);\n+\tstrbuf_add_commented_lines(buf, msg, strlen(msg));\n }\n \n int edit_todo_list(struct repository *r, struct todo_list *todo_list,\ndiff --git a/sequencer.c b/sequencer.c\nindex 5d348a3f12..29c8b5e32b 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1859,7 +1859,7 @@ static void add_commented_lines(struct strbuf *buf, const void *str, size_t len)\n \t\ts += count;\n \t\tlen -= count;\n \t}\n-\tstrbuf_add_commented_lines(buf, s, len, comment_line_char);\n+\tstrbuf_add_commented_lines(buf, s, len);\n }\n \n /* Does the current fixup chain contain a squash command? */\n@@ -1958,7 +1958,7 @@ static int append_squash_message(struct strbuf *buf, const char *body,\n \tstrbuf_addf(buf, _(nth_commit_msg_fmt),\n \t\t ++opts->current_fixup_count + 1);\n \tstrbuf_addstr(buf, \"\\n\\n\");\n-\tstrbuf_add_commented_lines(buf, body, commented_len, comment_line_char);\n+\tstrbuf_add_commented_lines(buf, body, commented_len);\n \t/* buf->buf may be reallocated so store an offset into the buffer */\n \tfixup_off = buf->len;\n \tstrbuf_addstr(buf, body + commented_len);\n@@ -2048,8 +2048,7 @@ static int update_squash_messages(struct repository *r,\n \t\t\t _(first_commit_msg_str));\n \t\tstrbuf_addstr(&buf, \"\\n\\n\");\n \t\tif (is_fixup_flag(command, flag))\n-\t\t\tstrbuf_add_commented_lines(&buf, body, strlen(body),\n-\t\t\t\t\t\t comment_line_char);\n+\t\t\tstrbuf_add_commented_lines(&buf, body, strlen(body));\n \t\telse\n \t\t\tstrbuf_addstr(&buf, body);\n \n@@ -2068,8 +2067,7 @@ static int update_squash_messages(struct repository *r,\n \t\tstrbuf_addf(&buf, _(skip_nth_commit_msg_fmt),\n \t\t\t ++opts->current_fixup_count + 1);\n \t\tstrbuf_addstr(&buf, \"\\n\\n\");\n-\t\tstrbuf_add_commented_lines(&buf, body, strlen(body),\n-\t\t\t\t\t comment_line_char);\n+\t\tstrbuf_add_commented_lines(&buf, body, strlen(body));\n \t} else\n \t\treturn error(_(\"unknown command: %d\"), command);\n \trepo_unuse_commit_buffer(r, commit, message);\ndiff --git a/strbuf.c b/strbuf.c\nindex 15550b2619..2088f7800a 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -360,8 +360,8 @@ static void add_lines(struct strbuf *out,\n \tstrbuf_complete_line(out);\n }\n \n-void strbuf_add_commented_lines(struct strbuf *out, const char *buf,\n-\t\t\t\tsize_t size, char comment_line_char)\n+void strbuf_add_commented_lines(struct strbuf *out,\n+\t\t\t\tconst char *buf, size_t size)\n {\n \tstatic char prefix1[3];\n \tstatic char prefix2[2];\n@@ -384,7 +384,7 @@ void strbuf_commented_addf(struct strbuf *sb,\n \tstrbuf_vaddf(&buf, fmt, params);\n \tva_end(params);\n \n-\tstrbuf_add_commented_lines(sb, buf.buf, buf.len, comment_line_char);\n+\tstrbuf_add_commented_lines(sb, buf.buf, buf.len);\n \tif (incomplete_line)\n \t\tsb->buf[--sb->len] = '\\0';\n \ndiff --git a/strbuf.h b/strbuf.h\nindex 981617dc77..4547efa62e 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -287,8 +287,7 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n * by a comment character and a blank.\n */\n void strbuf_add_commented_lines(struct strbuf *out,\n-\t\t\t\tconst char *buf, size_t size,\n-\t\t\t\tchar comment_line_char);\n+\t\t\t\tconst char *buf, size_t size);\n \n \n /**\ndiff --git a/wt-status.c b/wt-status.c\nindex 54b2775730..b390c77334 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1027,7 +1027,7 @@ static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncom\n \tif (s->display_comment_prefix) {\n \t\tsize_t len;\n \t\tsummary_content = strbuf_detach(&summary, &len);\n-\t\tstrbuf_add_commented_lines(&summary, summary_content, len, comment_line_char);\n+\t\tstrbuf_add_commented_lines(&summary, summary_content, len);\n \t\tfree(summary_content);\n \t}\n \n@@ -1103,7 +1103,7 @@ void wt_status_append_cut_line(struct strbuf *buf)\n \tconst char *explanation = _(\"Do not modify or remove the line above.\\nEverything below it will be ignored.\");\n \n \tstrbuf_commented_addf(buf, \"%s\", cut_line);\n-\tstrbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);\n+\tstrbuf_add_commented_lines(buf, explanation, strlen(explanation));\n }\n \n void wt_status_add_cut_line(FILE *fp)\n-- \n2.42.0-526-g3130c155df\n\n"},{"id":"484074","messageId":"cb82440280c7112a0c1599ba4b6c90b6@manjaro.org","threadId":"60447","inReplyTo":"20231030051034.2295242-1-gitster@pobox.com","subject":"Re: [PATCH 0/2] Avoid passing global comment_line_char repeatedly","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2023-10-30T05:36:03Z","receivedAt":"2023-10-30T05:36:06Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2023-10-30 06:10, Junio C Hamano wrote:\n> Two strbuf functions used to produce commented lines take the\n> comment_line_char as their parameter, but in practice, all callers\n> feed the global variable comment_line_char from environment.[ch].\n> \n> Dropping the parameter from the callchain will make the interface\n> less flexible, and less error prone. If we choose to change the\n> implementation of the customizable comment line character (e.g., we\n> may want to stop referencing the global variable and instead use a\n> getter function), we will have fewer places we need to modify.\n> \n> Junio C Hamano (2):\n> strbuf_commented_addf(): drop the comment_line_char parameter\n> strbuf_add_commented_lines(): drop the comment_line_char parameter\n\nThis series looks good to me. It removes unnecessary complexity that \nprovided pretty much no value.\n\n> add-patch.c | 8 ++++----\n> builtin/branch.c | 2 +-\n> builtin/merge.c | 8 ++++----\n> builtin/notes.c | 9 ++++-----\n> builtin/stripspace.c | 2 +-\n> builtin/tag.c | 4 ++--\n> fmt-merge-msg.c | 9 +++------\n> rebase-interactive.c | 8 ++++----\n> sequencer.c | 14 ++++++--------\n> strbuf.c | 9 +++++----\n> strbuf.h | 7 +++----\n> wt-status.c | 6 +++---\n> 12 files changed, 40 insertions(+), 46 deletions(-)\n"},{"id":"484093","messageId":"db6702ba-11a7-44c1-af2a-95b080aaeb77@gmail.com","threadId":"60447","inReplyTo":"20231030051034.2295242-1-gitster@pobox.com","subject":"Re: [PATCH 0/2] Avoid passing global comment_line_char repeatedly","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-10-30T09:59:44Z","receivedAt":"2023-10-30T10:00:05Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 30/10/2023 05:10, Junio C Hamano wrote:\n> Two strbuf functions used to produce commented lines take the\n> comment_line_char as their parameter, but in practice, all callers\n> feed the global variable comment_line_char from environment.[ch].\n> \n> Dropping the parameter from the callchain will make the interface\n> less flexible, and less error prone. If we choose to change the\n> implementation of the customizable comment line character (e.g., we\n> may want to stop referencing the global variable and instead use a\n> getter function), we will have fewer places we need to modify.\n\nWhile I agree with your reasoning here, I think that parameter was \nrecently added as part of the libification effort - I can't remember \nexactly why and am too lazy to look it up so I've cc'd Calvin and \nJohathan instead.\n\nBest Wishes\n\nPhillip\n\n> Junio C Hamano (2):\n> strbuf_commented_addf(): drop the comment_line_char parameter\n> strbuf_add_commented_lines(): drop the comment_line_char parameter\n> \n> add-patch.c | 8 ++++----\n> builtin/branch.c | 2 +-\n> builtin/merge.c | 8 ++++----\n> builtin/notes.c | 9 ++++-----\n> builtin/stripspace.c | 2 +-\n> builtin/tag.c | 4 ++--\n> fmt-merge-msg.c | 9 +++------\n> rebase-interactive.c | 8 ++++----\n> sequencer.c | 14 ++++++--------\n> strbuf.c | 9 +++++----\n> strbuf.h | 7 +++----\n> wt-status.c | 6 +++---\n> 12 files changed, 40 insertions(+), 46 deletions(-)\n> \n"},{"id":"484145","messageId":"cover.1698696798.git.jonathantanmy@google.com","threadId":"60447","inReplyTo":"db6702ba-11a7-44c1-af2a-95b080aaeb77@gmail.com","subject":"[RFC PATCH 0/3] Avoid passing global comment_line_char repeatedly","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2023-10-30T20:22:45Z","receivedAt":"2023-10-30T20:22:56Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> While I agree with your reasoning here, I think that parameter was \n> recently added as part of the libification effort - I can't remember \n> exactly why and am too lazy to look it up so I've cc'd Calvin and \n> Johathan instead.\n\nThanks Phillip for noticing this. Putting on my libification hat,\nthis was probably because we wanted to remove strbuf's dependency on\nenvironment, so that we wouldn't need to include it in git-std-lib. If\nwe were to merge these patches, libification would probably still be\ndoable if we stubbed the global comment_line_char.\n\nRemoving my libification hat, I think it's better to solve this issue\nby moving the functions into environment.{c,h} instead, following\nthe example of functions like strbuf_worktree_ref() in worktree.h\nand strbuf_utf8_align() in utf8.h that, when operating on both strbuf\nand a specific domain, are placed in the domain's header file, not in\nstrbuf.h. This avoids a situation in which strbuf.h contains everything\nstring-related.\n\nThe main issue with this is that by not centralizing all strbuf-related\nfunctionality, some strbuf-related helper functions that could have been\nprivate now need to be made public, but I think that a similar issue\nwould be faced if we don't centralize, say, all environment-related\nfunctionality (some environment-related helper functions would have to\nbe made public, although I didn't encounter this problem with this patch\nset).\n\nI've attached some patches to illustrate what I've described above.\n\nJonathan Tan (1):\n strbuf: make add_lines() public\n\nJunio C Hamano (2):\n strbuf_commented_addf(): drop the comment_line_char parameter\n strbuf_add_commented_lines(): drop the comment_line_char parameter\n\n add-patch.c | 8 ++---\n branch.c | 3 +-\n builtin/branch.c | 2 +-\n builtin/merge.c | 8 ++---\n builtin/notes.c | 9 +++---\n builtin/stripspace.c | 2 +-\n builtin/tag.c | 4 +--\n commit.c | 2 +-\n environment.c | 31 ++++++++++++++++++++\n environment.h | 14 +++++++++\n fmt-merge-msg.c | 9 ++----\n rebase-interactive.c | 8 ++---\n sequencer.c | 14 ++++-----\n strbuf.c | 69 ++++++++++----------------------------------\n strbuf.h | 19 ++----------\n wt-status.c | 6 ++--\n 16 files changed, 98 insertions(+), 110 deletions(-)\n\n-- \n2.42.0.820.g83a721a137-goog\n\n"},{"id":"484146","messageId":"d96633a2919ac619ccf29e87abc6f25314a8bfb1.1698696798.git.jonathantanmy@google.com","threadId":"60447","inReplyTo":"cover.1698696798.git.jonathantanmy@google.com","subject":"[RFC PATCH 1/3] strbuf: make add_lines() public","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2023-10-30T20:22:46Z","receivedAt":"2023-10-30T20:23:00Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Subsequent patches will require the ability to add different prefixes\nto different lines (depending on their contents), so make this\nfunctionality available from outside strbuf.c.\n\nSigned-off-by: Jonathan Tan \n---\n branch.c | 3 ++-\n commit.c | 2 +-\n strbuf.c | 39 ++++++++++++++++-----------------------\n strbuf.h | 3 ++-\n 4 files changed, 21 insertions(+), 26 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 06f7af9dd4..04a8b90b6a 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -721,7 +721,8 @@ static int submodule_create_branch(struct repository *r,\n \t\treturn ret;\n \tret = finish_command(&child);\n \tstrbuf_read(&child_err, child.err, 0);\n-\tstrbuf_add_lines(&out_buf, out_prefix, child_err.buf, child_err.len);\n+\tstrbuf_add_lines(&out_buf, out_prefix, out_prefix,\n+\t\t\t child_err.buf, child_err.len);\n \n \tif (ret)\n \t\tfprintf(stderr, \"%s\", out_buf.buf);\ndiff --git a/commit.c b/commit.c\nindex b3223478bc..7caafcde01 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1361,7 +1361,7 @@ static void add_extra_header(struct strbuf *buffer,\n {\n \tstrbuf_addstr(buffer, extra->key);\n \tif (extra->len)\n-\t\tstrbuf_add_lines(buffer, \" \", extra->value, extra->len);\n+\t\tstrbuf_add_lines(buffer, \" \", \" \", extra->value, extra->len);\n \telse\n \t\tstrbuf_addch(buffer, '\\n');\n }\ndiff --git a/strbuf.c b/strbuf.c\nindex 7827178d8e..9ee639519a 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -339,26 +339,6 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)\n \tva_end(ap);\n }\n \n-static void add_lines(struct strbuf *out,\n-\t\t\tconst char *prefix1,\n-\t\t\tconst char *prefix2,\n-\t\t\tconst char *buf, size_t size)\n-{\n-\twhile (size) {\n-\t\tconst char *prefix;\n-\t\tconst char *next = memchr(buf, '\\n', size);\n-\t\tnext = next ? (next + 1) : (buf + size);\n-\n-\t\tprefix = ((prefix2 && (buf[0] == '\\n' || buf[0] == '\\t'))\n-\t\t\t ? prefix2 : prefix1);\n-\t\tstrbuf_addstr(out, prefix);\n-\t\tstrbuf_add(out, buf, next - buf);\n-\t\tsize -= next - buf;\n-\t\tbuf = next;\n-\t}\n-\tstrbuf_complete_line(out);\n-}\n-\n void strbuf_add_commented_lines(struct strbuf *out, const char *buf,\n \t\t\t\tsize_t size, char comment_line_char)\n {\n@@ -369,7 +349,7 @@ void strbuf_add_commented_lines(struct strbuf *out, const char *buf,\n \t\txsnprintf(prefix1, sizeof(prefix1), \"%c \", comment_line_char);\n \t\txsnprintf(prefix2, sizeof(prefix2), \"%c\", comment_line_char);\n \t}\n-\tadd_lines(out, prefix1, prefix2, buf, size);\n+\tstrbuf_add_lines(out, prefix1, prefix2, buf, size);\n }\n \n void strbuf_commented_addf(struct strbuf *sb, char comment_line_char,\n@@ -747,10 +727,23 @@ ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)\n \treturn len;\n }\n \n-void strbuf_add_lines(struct strbuf *out, const char *prefix,\n+void strbuf_add_lines(struct strbuf *out, const char *default_prefix,\n+\t\t const char *tab_or_nl_prefix,\n \t\t const char *buf, size_t size)\n {\n-\tadd_lines(out, prefix, NULL, buf, size);\n+\twhile (size) {\n+\t\tconst char *prefix;\n+\t\tconst char *next = memchr(buf, '\\n', size);\n+\t\tnext = next ? (next + 1) : (buf + size);\n+\n+\t\tprefix = (buf[0] == '\\n' || buf[0] == '\\t')\n+\t\t\t ? tab_or_nl_prefix : default_prefix;\n+\t\tstrbuf_addstr(out, prefix);\n+\t\tstrbuf_add(out, buf, next - buf);\n+\t\tsize -= next - buf;\n+\t\tbuf = next;\n+\t}\n+\tstrbuf_complete_line(out);\n }\n \n void strbuf_addstr_xml_quoted(struct strbuf *buf, const char *s)\ndiff --git a/strbuf.h b/strbuf.h\nindex e959caca87..3559e73dd8 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -599,7 +599,8 @@ void strbuf_list_free(struct strbuf **list);\n void strbuf_strip_file_from_path(struct strbuf *sb);\n \n void strbuf_add_lines(struct strbuf *sb,\n-\t\t const char *prefix,\n+\t\t const char *default_prefix,\n+\t\t const char *tab_or_nl_prefix,\n \t\t const char *buf,\n \t\t size_t size);\n \n-- \n2.42.0.820.g83a721a137-goog\n\n"},{"id":"484147","messageId":"bb01336233b30d46960d6eb15f036e6346a9cd2b.1698696798.git.jonathantanmy@google.com","threadId":"60447","inReplyTo":"cover.1698696798.git.jonathantanmy@google.com","subject":"[RFC PATCH 2/3] strbuf_commented_addf(): drop the comment_line_char parameter","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2023-10-30T20:22:47Z","receivedAt":"2023-10-30T20:23:03Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"From: Junio C Hamano \n\nAll the callers of this function supply the global variable\ncomment_line_char as an argument to its second parameter. Remove\nthe parameter to allow us in the future to change the reference to\nthe global variable with something else, like a function call.\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Jonathan Tan \n---\n add-patch.c | 8 ++++----\n builtin/branch.c | 2 +-\n builtin/merge.c | 8 ++++----\n builtin/tag.c | 4 ++--\n environment.c | 18 ++++++++++++++++++\n environment.h | 7 +++++++\n rebase-interactive.c | 2 +-\n sequencer.c | 4 ++--\n strbuf.c | 19 +------------------\n strbuf.h | 7 -------\n wt-status.c | 2 +-\n 11 files changed, 41 insertions(+), 40 deletions(-)\n\ndiff --git a/add-patch.c b/add-patch.c\nindex bfe19876cd..471a0037be 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -1106,11 +1106,11 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)\n \tsize_t i;\n \n \tstrbuf_reset(&s->buf);\n-\tstrbuf_commented_addf(&s->buf, comment_line_char,\n+\tstrbuf_commented_addf(&s->buf,\n \t\t\t _(\"Manual hunk edit mode -- see bottom for \"\n \t\t\t\t\"a quick guide.\\n\"));\n \trender_hunk(s, hunk, 0, 0, &s->buf);\n-\tstrbuf_commented_addf(&s->buf, comment_line_char,\n+\tstrbuf_commented_addf(&s->buf,\n \t\t\t _(\"---\\n\"\n \t\t\t\t\"To remove '%c' lines, make them ' ' lines \"\n \t\t\t\t\"(context).\\n\"\n@@ -1119,13 +1119,13 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)\n \t\t\t s->mode->is_reverse ? '+' : '-',\n \t\t\t s->mode->is_reverse ? '-' : '+',\n \t\t\t comment_line_char);\n-\tstrbuf_commented_addf(&s->buf, comment_line_char, \"%s\",\n+\tstrbuf_commented_addf(&s->buf, \"%s\",\n \t\t\t _(s->mode->edit_hunk_hint));\n \t/*\n \t * TRANSLATORS: 'it' refers to the patch mentioned in the previous\n \t * messages.\n \t */\n-\tstrbuf_commented_addf(&s->buf, comment_line_char,\n+\tstrbuf_commented_addf(&s->buf,\n \t\t\t _(\"If it does not apply cleanly, you will be \"\n \t\t\t\t\"given an opportunity to\\n\"\n \t\t\t\t\"edit again. If all lines of the hunk are \"\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 2ec190b14a..b2f171e10b 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -668,7 +668,7 @@ static int edit_branch_description(const char *branch_name)\n \texists = !read_branch_desc(&buf, branch_name);\n \tif (!buf.len || buf.buf[buf.len-1] != '\\n')\n \t\tstrbuf_addch(&buf, '\\n');\n-\tstrbuf_commented_addf(&buf, comment_line_char,\n+\tstrbuf_commented_addf(&buf,\n \t\t _(\"Please edit the description for the branch\\n\"\n \t\t \" %s\\n\"\n \t\t \"Lines starting with '%c' will be stripped.\\n\"),\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex d748d46e13..8f0e8be7c3 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -857,15 +857,15 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t\tstrbuf_addch(&msg, '\\n');\n \t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n \t\t\twt_status_append_cut_line(&msg);\n-\t\t\tstrbuf_commented_addf(&msg, comment_line_char, \"\\n\");\n+\t\t\tstrbuf_commented_addf(&msg, \"\\n\");\n \t\t}\n-\t\tstrbuf_commented_addf(&msg, comment_line_char,\n+\t\tstrbuf_commented_addf(&msg,\n \t\t\t\t _(merge_editor_comment));\n \t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)\n-\t\t\tstrbuf_commented_addf(&msg, comment_line_char,\n+\t\t\tstrbuf_commented_addf(&msg,\n \t\t\t\t\t _(scissors_editor_comment));\n \t\telse\n-\t\t\tstrbuf_commented_addf(&msg, comment_line_char,\n+\t\t\tstrbuf_commented_addf(&msg,\n \t\t\t\t_(no_scissors_editor_comment), comment_line_char);\n \t}\n \tif (signoff)\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 3918eacbb5..a85a0d8def 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -314,10 +314,10 @@ static void create_tag(const struct object_id *object, const char *object_ref,\n \t\t\tstruct strbuf buf = STRBUF_INIT;\n \t\t\tstrbuf_addch(&buf, '\\n');\n \t\t\tif (opt->cleanup_mode == CLEANUP_ALL)\n-\t\t\t\tstrbuf_commented_addf(&buf, comment_line_char,\n+\t\t\t\tstrbuf_commented_addf(&buf,\n \t\t\t\t _(tag_template), tag, comment_line_char);\n \t\t\telse\n-\t\t\t\tstrbuf_commented_addf(&buf, comment_line_char,\n+\t\t\t\tstrbuf_commented_addf(&buf,\n \t\t\t\t _(tag_template_nocleanup), tag, comment_line_char);\n \t\t\twrite_or_die(fd, buf.buf, buf.len);\n \t\t\tstrbuf_release(&buf);\ndiff --git a/environment.c b/environment.c\nindex bb3c2a96a3..d9f64cffa0 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -416,3 +416,21 @@ int print_sha1_ellipsis(void)\n \t}\n \treturn cached_result;\n }\n+\n+void strbuf_commented_addf(struct strbuf *sb,\n+\t\t\t const char *fmt, ...)\n+{\n+\tva_list params;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint incomplete_line = sb->len && sb->buf[sb->len - 1] != '\\n';\n+\n+\tva_start(params, fmt);\n+\tstrbuf_vaddf(&buf, fmt, params);\n+\tva_end(params);\n+\n+\tstrbuf_add_commented_lines(sb, buf.buf, buf.len, comment_line_char);\n+\tif (incomplete_line)\n+\t\tsb->buf[--sb->len] = '\\0';\n+\n+\tstrbuf_release(&buf);\n+}\ndiff --git a/environment.h b/environment.h\nindex e5351c9dd9..5778f5a8e4 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -229,4 +229,11 @@ extern const char *excludes_file;\n */\n int print_sha1_ellipsis(void);\n \n+/**\n+ * Add a formatted string prepended by a comment character and a\n+ * blank to the buffer.\n+ */\n+__attribute__((format (printf, 2, 3)))\n+void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);\n+\n #endif\ndiff --git a/rebase-interactive.c b/rebase-interactive.c\nindex d9718409b3..3f33da7f03 100644\n--- a/rebase-interactive.c\n+++ b/rebase-interactive.c\n@@ -71,7 +71,7 @@ void append_todo_help(int command_count,\n \n \tif (!edit_todo) {\n \t\tstrbuf_addch(buf, '\\n');\n-\t\tstrbuf_commented_addf(buf, comment_line_char,\n+\t\tstrbuf_commented_addf(buf,\n \t\t\t\t Q_(\"Rebase %s onto %s (%d command)\",\n \t\t\t\t\t \"Rebase %s onto %s (%d commands)\",\n \t\t\t\t\t command_count),\ndiff --git a/sequencer.c b/sequencer.c\nindex d584cac8ed..5d348a3f12 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -675,11 +675,11 @@ void append_conflicts_hint(struct index_state *istate,\n \t}\n \n \tstrbuf_addch(msgbuf, '\\n');\n-\tstrbuf_commented_addf(msgbuf, comment_line_char, \"Conflicts:\\n\");\n+\tstrbuf_commented_addf(msgbuf, \"Conflicts:\\n\");\n \tfor (i = 0; i < istate->cache_nr;) {\n \t\tconst struct cache_entry *ce = istate->cache[i++];\n \t\tif (ce_stage(ce)) {\n-\t\t\tstrbuf_commented_addf(msgbuf, comment_line_char,\n+\t\t\tstrbuf_commented_addf(msgbuf,\n \t\t\t\t\t \"\\t%s\\n\", ce->name);\n \t\t\twhile (i < istate->cache_nr &&\n \t\t\t !strcmp(ce->name, istate->cache[i]->name))\ndiff --git a/strbuf.c b/strbuf.c\nindex 9ee639519a..b1717270a2 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -1,4 +1,5 @@\n #include \"git-compat-util.h\"\n+#include \"environment.h\"\n #include \"gettext.h\"\n #include \"hex-ll.h\"\n #include \"strbuf.h\"\n@@ -352,24 +353,6 @@ void strbuf_add_commented_lines(struct strbuf *out, const char *buf,\n \tstrbuf_add_lines(out, prefix1, prefix2, buf, size);\n }\n \n-void strbuf_commented_addf(struct strbuf *sb, char comment_line_char,\n-\t\t\t const char *fmt, ...)\n-{\n-\tva_list params;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tint incomplete_line = sb->len && sb->buf[sb->len - 1] != '\\n';\n-\n-\tva_start(params, fmt);\n-\tstrbuf_vaddf(&buf, fmt, params);\n-\tva_end(params);\n-\n-\tstrbuf_add_commented_lines(sb, buf.buf, buf.len, comment_line_char);\n-\tif (incomplete_line)\n-\t\tsb->buf[--sb->len] = '\\0';\n-\n-\tstrbuf_release(&buf);\n-}\n-\n void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)\n {\n \tint len;\ndiff --git a/strbuf.h b/strbuf.h\nindex 3559e73dd8..4c58dc25e9 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -374,13 +374,6 @@ void strbuf_humanise_rate(struct strbuf *buf, off_t bytes);\n __attribute__((format (printf,2,3)))\n void strbuf_addf(struct strbuf *sb, const char *fmt, ...);\n \n-/**\n- * Add a formatted string prepended by a comment character and a\n- * blank to the buffer.\n- */\n-__attribute__((format (printf, 3, 4)))\n-void strbuf_commented_addf(struct strbuf *sb, char comment_line_char, const char *fmt, ...);\n-\n __attribute__((format (printf,2,0)))\n void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);\n \ndiff --git a/wt-status.c b/wt-status.c\nindex 9f45bf6949..54b2775730 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1102,7 +1102,7 @@ void wt_status_append_cut_line(struct strbuf *buf)\n {\n \tconst char *explanation = _(\"Do not modify or remove the line above.\\nEverything below it will be ignored.\");\n \n-\tstrbuf_commented_addf(buf, comment_line_char, \"%s\", cut_line);\n+\tstrbuf_commented_addf(buf, \"%s\", cut_line);\n \tstrbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);\n }\n \n-- \n2.42.0.820.g83a721a137-goog\n\n"},{"id":"484148","messageId":"a1dd551107c804ab1dd4a9c17b1917b3ccec2279.1698696798.git.jonathantanmy@google.com","threadId":"60447","inReplyTo":"cover.1698696798.git.jonathantanmy@google.com","subject":"[RFC PATCH 3/3] strbuf_add_commented_lines(): drop the comment_line_char parameter","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2023-10-30T20:22:48Z","receivedAt":"2023-10-30T20:23:14Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"From: Junio C Hamano \n\nAll the callers of this function supply the global variable\ncomment_line_char as an argument to its last parameter. Remove the\nparameter to allow us in the future to change the reference to the\nglobal variable with something else, like a function call.\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Jonathan Tan \n---\n builtin/notes.c | 9 ++++-----\n builtin/stripspace.c | 2 +-\n environment.c | 15 ++++++++++++++-\n environment.h | 7 +++++++\n fmt-merge-msg.c | 9 +++------\n rebase-interactive.c | 6 +++---\n sequencer.c | 10 ++++------\n strbuf.c | 13 -------------\n strbuf.h | 9 ---------\n wt-status.c | 4 ++--\n 10 files changed, 38 insertions(+), 46 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 9f38863dd5..355ecce07a 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -181,7 +181,7 @@ static void write_commented_object(int fd, const struct object_id *object)\n \n \tif (strbuf_read(&buf, show.out, 0) < 0)\n \t\tdie_errno(_(\"could not read 'show' output\"));\n-\tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_char);\n+\tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len);\n \twrite_or_die(fd, cbuf.buf, cbuf.len);\n \n \tstrbuf_release(&cbuf);\n@@ -209,10 +209,9 @@ static void prepare_note_data(const struct object_id *object, struct note_data *\n \t\t\tcopy_obj_to_fd(fd, old_note);\n \n \t\tstrbuf_addch(&buf, '\\n');\n-\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"), comment_line_char);\n-\t\tstrbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)),\n-\t\t\t\t\t comment_line_char);\n-\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"), comment_line_char);\n+\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"));\n+\t\tstrbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));\n+\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"));\n \t\twrite_or_die(fd, buf.buf, buf.len);\n \n \t\twrite_commented_object(fd, object);\ndiff --git a/builtin/stripspace.c b/builtin/stripspace.c\nindex 7b700a9fb1..11e475760c 100644\n--- a/builtin/stripspace.c\n+++ b/builtin/stripspace.c\n@@ -13,7 +13,7 @@ static void comment_lines(struct strbuf *buf)\n \tsize_t len;\n \n \tmsg = strbuf_detach(buf, &len);\n-\tstrbuf_add_commented_lines(buf, msg, len, comment_line_char);\n+\tstrbuf_add_commented_lines(buf, msg, len);\n \tfree(msg);\n }\n \ndiff --git a/environment.c b/environment.c\nindex d9f64cffa0..cc1b85afb6 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -428,9 +428,22 @@ void strbuf_commented_addf(struct strbuf *sb,\n \tstrbuf_vaddf(&buf, fmt, params);\n \tva_end(params);\n \n-\tstrbuf_add_commented_lines(sb, buf.buf, buf.len, comment_line_char);\n+\tstrbuf_add_commented_lines(sb, buf.buf, buf.len);\n \tif (incomplete_line)\n \t\tsb->buf[--sb->len] = '\\0';\n \n \tstrbuf_release(&buf);\n }\n+\n+void strbuf_add_commented_lines(struct strbuf *out,\n+\t\t\t\tconst char *buf, size_t size)\n+{\n+\tstatic char prefix1[3];\n+\tstatic char prefix2[2];\n+\n+\tif (prefix1[0] != comment_line_char) {\n+\t\txsnprintf(prefix1, sizeof(prefix1), \"%c \", comment_line_char);\n+\t\txsnprintf(prefix2, sizeof(prefix2), \"%c\", comment_line_char);\n+\t}\n+\tstrbuf_add_lines(out, prefix1, prefix2, buf, size);\n+}\ndiff --git a/environment.h b/environment.h\nindex 5778f5a8e4..f801dbe36e 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -236,4 +236,11 @@ int print_sha1_ellipsis(void);\n __attribute__((format (printf, 2, 3)))\n void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);\n \n+/**\n+ * Add a NUL-terminated string to the buffer. Each line will be prepended\n+ * by a comment character and a blank.\n+ */\n+void strbuf_add_commented_lines(struct strbuf *out,\n+\t\t\t\tconst char *buf, size_t size);\n+\n #endif\ndiff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\nindex 66e47449a0..adc85d2a72 100644\n--- a/fmt-merge-msg.c\n+++ b/fmt-merge-msg.c\n@@ -509,8 +509,7 @@ static void fmt_tag_signature(struct strbuf *tagbuf,\n \tstrbuf_complete_line(tagbuf);\n \tif (sig->len) {\n \t\tstrbuf_addch(tagbuf, '\\n');\n-\t\tstrbuf_add_commented_lines(tagbuf, sig->buf, sig->len,\n-\t\t\t\t\t comment_line_char);\n+\t\tstrbuf_add_commented_lines(tagbuf, sig->buf, sig->len);\n \t}\n }\n \n@@ -556,8 +555,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\t\t\tstrbuf_addch(&tagline, '\\n');\n \t\t\t\tstrbuf_add_commented_lines(&tagline,\n \t\t\t\t\t\torigins.items[first_tag].string,\n-\t\t\t\t\t\tstrlen(origins.items[first_tag].string),\n-\t\t\t\t\t\tcomment_line_char);\n+\t\t\t\t\t\tstrlen(origins.items[first_tag].string));\n \t\t\t\tstrbuf_insert(&tagbuf, 0, tagline.buf,\n \t\t\t\t\t tagline.len);\n \t\t\t\tstrbuf_release(&tagline);\n@@ -565,8 +563,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\t\tstrbuf_addch(&tagbuf, '\\n');\n \t\t\tstrbuf_add_commented_lines(&tagbuf,\n \t\t\t\t\torigins.items[i].string,\n-\t\t\t\t\tstrlen(origins.items[i].string),\n-\t\t\t\t\tcomment_line_char);\n+\t\t\t\t\tstrlen(origins.items[i].string));\n \t\t\tfmt_tag_signature(&tagbuf, &sig, buf, len);\n \t\t}\n \t\tstrbuf_release(&payload);\ndiff --git a/rebase-interactive.c b/rebase-interactive.c\nindex 3f33da7f03..1138bd37ba 100644\n--- a/rebase-interactive.c\n+++ b/rebase-interactive.c\n@@ -78,7 +78,7 @@ void append_todo_help(int command_count,\n \t\t\t\t shortrevisions, shortonto, command_count);\n \t}\n \n-\tstrbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char);\n+\tstrbuf_add_commented_lines(buf, msg, strlen(msg));\n \n \tif (get_missing_commit_check_level() == MISSING_COMMIT_CHECK_ERROR)\n \t\tmsg = _(\"\\nDo not remove any line. Use 'drop' \"\n@@ -87,7 +87,7 @@ void append_todo_help(int command_count,\n \t\tmsg = _(\"\\nIf you remove a line here \"\n \t\t\t \"THAT COMMIT WILL BE LOST.\\n\");\n \n-\tstrbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char);\n+\tstrbuf_add_commented_lines(buf, msg, strlen(msg));\n \n \tif (edit_todo)\n \t\tmsg = _(\"\\nYou are editing the todo file \"\n@@ -98,7 +98,7 @@ void append_todo_help(int command_count,\n \t\tmsg = _(\"\\nHowever, if you remove everything, \"\n \t\t\t\"the rebase will be aborted.\\n\\n\");\n \n-\tstrbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char);\n+\tstrbuf_add_commented_lines(buf, msg, strlen(msg));\n }\n \n int edit_todo_list(struct repository *r, struct todo_list *todo_list,\ndiff --git a/sequencer.c b/sequencer.c\nindex 5d348a3f12..29c8b5e32b 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1859,7 +1859,7 @@ static void add_commented_lines(struct strbuf *buf, const void *str, size_t len)\n \t\ts += count;\n \t\tlen -= count;\n \t}\n-\tstrbuf_add_commented_lines(buf, s, len, comment_line_char);\n+\tstrbuf_add_commented_lines(buf, s, len);\n }\n \n /* Does the current fixup chain contain a squash command? */\n@@ -1958,7 +1958,7 @@ static int append_squash_message(struct strbuf *buf, const char *body,\n \tstrbuf_addf(buf, _(nth_commit_msg_fmt),\n \t\t ++opts->current_fixup_count + 1);\n \tstrbuf_addstr(buf, \"\\n\\n\");\n-\tstrbuf_add_commented_lines(buf, body, commented_len, comment_line_char);\n+\tstrbuf_add_commented_lines(buf, body, commented_len);\n \t/* buf->buf may be reallocated so store an offset into the buffer */\n \tfixup_off = buf->len;\n \tstrbuf_addstr(buf, body + commented_len);\n@@ -2048,8 +2048,7 @@ static int update_squash_messages(struct repository *r,\n \t\t\t _(first_commit_msg_str));\n \t\tstrbuf_addstr(&buf, \"\\n\\n\");\n \t\tif (is_fixup_flag(command, flag))\n-\t\t\tstrbuf_add_commented_lines(&buf, body, strlen(body),\n-\t\t\t\t\t\t comment_line_char);\n+\t\t\tstrbuf_add_commented_lines(&buf, body, strlen(body));\n \t\telse\n \t\t\tstrbuf_addstr(&buf, body);\n \n@@ -2068,8 +2067,7 @@ static int update_squash_messages(struct repository *r,\n \t\tstrbuf_addf(&buf, _(skip_nth_commit_msg_fmt),\n \t\t\t ++opts->current_fixup_count + 1);\n \t\tstrbuf_addstr(&buf, \"\\n\\n\");\n-\t\tstrbuf_add_commented_lines(&buf, body, strlen(body),\n-\t\t\t\t\t comment_line_char);\n+\t\tstrbuf_add_commented_lines(&buf, body, strlen(body));\n \t} else\n \t\treturn error(_(\"unknown command: %d\"), command);\n \trepo_unuse_commit_buffer(r, commit, message);\ndiff --git a/strbuf.c b/strbuf.c\nindex b1717270a2..43027eab76 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -340,19 +340,6 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)\n \tva_end(ap);\n }\n \n-void strbuf_add_commented_lines(struct strbuf *out, const char *buf,\n-\t\t\t\tsize_t size, char comment_line_char)\n-{\n-\tstatic char prefix1[3];\n-\tstatic char prefix2[2];\n-\n-\tif (prefix1[0] != comment_line_char) {\n-\t\txsnprintf(prefix1, sizeof(prefix1), \"%c \", comment_line_char);\n-\t\txsnprintf(prefix2, sizeof(prefix2), \"%c\", comment_line_char);\n-\t}\n-\tstrbuf_add_lines(out, prefix1, prefix2, buf, size);\n-}\n-\n void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)\n {\n \tint len;\ndiff --git a/strbuf.h b/strbuf.h\nindex 4c58dc25e9..ffa2fe3055 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -282,15 +282,6 @@ void strbuf_remove(struct strbuf *sb, size_t pos, size_t len);\n void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n \t\t const void *data, size_t data_len);\n \n-/**\n- * Add a NUL-terminated string to the buffer. Each line will be prepended\n- * by a comment character and a blank.\n- */\n-void strbuf_add_commented_lines(struct strbuf *out,\n-\t\t\t\tconst char *buf, size_t size,\n-\t\t\t\tchar comment_line_char);\n-\n-\n /**\n * Add data of given length to the buffer.\n */\ndiff --git a/wt-status.c b/wt-status.c\nindex 54b2775730..b390c77334 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1027,7 +1027,7 @@ static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncom\n \tif (s->display_comment_prefix) {\n \t\tsize_t len;\n \t\tsummary_content = strbuf_detach(&summary, &len);\n-\t\tstrbuf_add_commented_lines(&summary, summary_content, len, comment_line_char);\n+\t\tstrbuf_add_commented_lines(&summary, summary_content, len);\n \t\tfree(summary_content);\n \t}\n \n@@ -1103,7 +1103,7 @@ void wt_status_append_cut_line(struct strbuf *buf)\n \tconst char *explanation = _(\"Do not modify or remove the line above.\\nEverything below it will be ignored.\");\n \n \tstrbuf_commented_addf(buf, \"%s\", cut_line);\n-\tstrbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);\n+\tstrbuf_add_commented_lines(buf, explanation, strlen(explanation));\n }\n \n void wt_status_add_cut_line(FILE *fp)\n-- \n2.42.0.820.g83a721a137-goog\n\n"},{"id":"484166","messageId":"xmqqy1fj8y5m.fsf@gitster.g","threadId":"60447","inReplyTo":"d96633a2919ac619ccf29e87abc6f25314a8bfb1.1698696798.git.jonathantanmy@google.com","subject":"Re: [RFC PATCH 1/3] strbuf: make add_lines() public","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-30T23:53:57Z","receivedAt":"2023-10-30T23:54:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> Subsequent patches will require the ability to add different prefixes\n> to different lines (depending on their contents), so make this\n> functionality available from outside strbuf.c.\n\nI do not think it is a good idea to force almost everybody to repeat\nthemselves. As we can see here, all but just a single caller of\nstrbuf_add_lines() with this patch pass the same prefix for both\nparameters. If we need to make the current strbuf.c:add_lines()\nalso available to some specific callers, that is fine, but let's\nkeep the simpler version that almost everybody uses as-is, and give\nthe more complex and featureful one that is used only by selected\ncallers a longer and more cumbersome name.\n\nThanks.\n"},{"id":"484186","messageId":"xmqqh6m74bdo.fsf@gitster.g","threadId":"60447","inReplyTo":"bb01336233b30d46960d6eb15f036e6346a9cd2b.1698696798.git.jonathantanmy@google.com","subject":"Re: [RFC PATCH 2/3] strbuf_commented_addf(): drop the comment_line_char parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-31T05:19:31Z","receivedAt":"2023-10-31T05:19:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> From: Junio C Hamano \n>\n> All the callers of this function supply the global variable\n> comment_line_char as an argument to its second parameter. Remove\n> the parameter to allow us in the future to change the reference to\n> the global variable with something else, like a function call.\n>\n> Signed-off-by: Junio C Hamano \n> Signed-off-by: Jonathan Tan \n> ---\n\n> diff --git a/environment.c b/environment.c\n> index bb3c2a96a3..d9f64cffa0 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -416,3 +416,21 @@ int print_sha1_ellipsis(void)\n> \t}\n> \treturn cached_result;\n> }\n> +\n> +void strbuf_commented_addf(struct strbuf *sb,\n> +\t\t\t const char *fmt, ...)\n> +{\n> +\tva_list params;\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tint incomplete_line = sb->len && sb->buf[sb->len - 1] != '\\n';\n> +\n> +\tva_start(params, fmt);\n> +\tstrbuf_vaddf(&buf, fmt, params);\n> +\tva_end(params);\n> +\n> +\tstrbuf_add_commented_lines(sb, buf.buf, buf.len, comment_line_char);\n> +\tif (incomplete_line)\n> +\t\tsb->buf[--sb->len] = '\\0';\n> +\n> +\tstrbuf_release(&buf);\n> +}\n\nThis moving of the helper function does not belong to the \"fix\ncommented_addf() not to take the comment_line_char\" step.\n\nThe series should be restructured to have the two patches from me\nfirst, and then your moving some stuff to environment.c, probably.\n"},{"id":"484193","messageId":"xmqq1qdb49ff.fsf@gitster.g","threadId":"60447","inReplyTo":"xmqqy1fj8y5m.fsf@gitster.g","subject":"Re: [RFC PATCH 1/3] strbuf: make add_lines() public","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-31T06:01:40Z","receivedAt":"2023-10-31T06:18:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jonathan Tan writes:\n>\n>> Subsequent patches will require the ability to add different prefixes\n>> to different lines (depending on their contents), so make this\n>> functionality available from outside strbuf.c.\n>\n> I do not think it is a good idea to force almost everybody to repeat\n> themselves. As we can see here, all but just a single caller of\n> strbuf_add_lines() with this patch pass the same prefix for both\n> parameters. If we need to make the current strbuf.c:add_lines()\n> also available to some specific callers, that is fine, but let's\n> keep the simpler version that almost everybody uses as-is, and give\n> the more complex and featureful one that is used only by selected\n> callers a longer and more cumbersome name.\n>\n> Thanks.\n\nAnother practical downside of this patch is that it breaks other\nin-flight topics that adds new users of strbuf_add_lines(), and that\nbreakage is totally unnecessary.\n\n\n"},{"id":"484257","messageId":"20231031222400.2048688-1-jonathantanmy@google.com","threadId":"60447","inReplyTo":"xmqqh6m74bdo.fsf@gitster.g","subject":"Re: [RFC PATCH 2/3] strbuf_commented_addf(): drop the comment_line_char parameter","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2023-10-31T22:24:00Z","receivedAt":"2023-10-31T22:24:06Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Junio C Hamano writes:\n> This moving of the helper function does not belong to the \"fix\n> commented_addf() not to take the comment_line_char\" step.\n> \n> The series should be restructured to have the two patches from me\n> first, and then your moving some stuff to environment.c, probably.\n\nThis means that #include \"environment.h\" will be added and then removed\nin the same series, but I don't feel too strongly about that. I'll send\nan updated set of patches.\n"},{"id":"484258","messageId":"cover.1698791220.git.jonathantanmy@google.com","threadId":"60447","inReplyTo":"db6702ba-11a7-44c1-af2a-95b080aaeb77@gmail.com","subject":"[PATCH v2 0/4] Avoid passing global comment_line_char repeatedly","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2023-10-31T22:28:29Z","receivedAt":"2023-10-31T22:28:41Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Here's an updated patchset. The first 2 are the exact same as what Junio\nhas sent.\n\nJonathan Tan (2):\n strbuf: make add_lines() public\n strbuf: move env-using functions to environment.c\n\nJunio C Hamano (2):\n strbuf_commented_addf(): drop the comment_line_char parameter\n strbuf_add_commented_lines(): drop the comment_line_char parameter\n\n add-patch.c | 8 +++----\n builtin/branch.c | 2 +-\n builtin/merge.c | 8 +++----\n builtin/notes.c | 9 ++++----\n builtin/stripspace.c | 2 +-\n builtin/tag.c | 4 ++--\n environment.c | 32 +++++++++++++++++++++++++++\n environment.h | 14 ++++++++++++\n fmt-merge-msg.c | 9 +++-----\n rebase-interactive.c | 8 +++----\n sequencer.c | 14 ++++++------\n strbuf.c | 51 +++++++++-----------------------------------\n strbuf.h | 20 ++++-------------\n wt-status.c | 6 +++---\n 14 files changed, 92 insertions(+), 95 deletions(-)\n\n-- \n2.42.0.820.g83a721a137-goog\n\n"},{"id":"484259","messageId":"98c9f70608d7b6f7bee49a89634de100b2dd5ecf.1698791220.git.jonathantanmy@google.com","threadId":"60447","inReplyTo":"cover.1698791220.git.jonathantanmy@google.com","subject":"[PATCH v2 1/4] strbuf_commented_addf(): drop the comment_line_char parameter","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2023-10-31T22:28:30Z","receivedAt":"2023-10-31T22:28:42Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"From: Junio C Hamano \n\nAll the callers of this function supply the global variable\ncomment_line_char as an argument to its second parameter. Remove\nthe parameter to allow us in the future to change the reference to\nthe global variable with something else, like a function call.\n\nSigned-off-by: Junio C Hamano \n---\n add-patch.c | 8 ++++----\n builtin/branch.c | 2 +-\n builtin/merge.c | 8 ++++----\n builtin/tag.c | 4 ++--\n rebase-interactive.c | 2 +-\n sequencer.c | 4 ++--\n strbuf.c | 3 ++-\n strbuf.h | 4 ++--\n wt-status.c | 2 +-\n 9 files changed, 19 insertions(+), 18 deletions(-)\n\ndiff --git a/add-patch.c b/add-patch.c\nindex bfe19876cd..471a0037be 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -1106,11 +1106,11 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)\n \tsize_t i;\n \n \tstrbuf_reset(&s->buf);\n-\tstrbuf_commented_addf(&s->buf, comment_line_char,\n+\tstrbuf_commented_addf(&s->buf,\n \t\t\t _(\"Manual hunk edit mode -- see bottom for \"\n \t\t\t\t\"a quick guide.\\n\"));\n \trender_hunk(s, hunk, 0, 0, &s->buf);\n-\tstrbuf_commented_addf(&s->buf, comment_line_char,\n+\tstrbuf_commented_addf(&s->buf,\n \t\t\t _(\"---\\n\"\n \t\t\t\t\"To remove '%c' lines, make them ' ' lines \"\n \t\t\t\t\"(context).\\n\"\n@@ -1119,13 +1119,13 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)\n \t\t\t s->mode->is_reverse ? '+' : '-',\n \t\t\t s->mode->is_reverse ? '-' : '+',\n \t\t\t comment_line_char);\n-\tstrbuf_commented_addf(&s->buf, comment_line_char, \"%s\",\n+\tstrbuf_commented_addf(&s->buf, \"%s\",\n \t\t\t _(s->mode->edit_hunk_hint));\n \t/*\n \t * TRANSLATORS: 'it' refers to the patch mentioned in the previous\n \t * messages.\n \t */\n-\tstrbuf_commented_addf(&s->buf, comment_line_char,\n+\tstrbuf_commented_addf(&s->buf,\n \t\t\t _(\"If it does not apply cleanly, you will be \"\n \t\t\t\t\"given an opportunity to\\n\"\n \t\t\t\t\"edit again. If all lines of the hunk are \"\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 2ec190b14a..b2f171e10b 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -668,7 +668,7 @@ static int edit_branch_description(const char *branch_name)\n \texists = !read_branch_desc(&buf, branch_name);\n \tif (!buf.len || buf.buf[buf.len-1] != '\\n')\n \t\tstrbuf_addch(&buf, '\\n');\n-\tstrbuf_commented_addf(&buf, comment_line_char,\n+\tstrbuf_commented_addf(&buf,\n \t\t _(\"Please edit the description for the branch\\n\"\n \t\t \" %s\\n\"\n \t\t \"Lines starting with '%c' will be stripped.\\n\"),\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex d748d46e13..8f0e8be7c3 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -857,15 +857,15 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t\tstrbuf_addch(&msg, '\\n');\n \t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n \t\t\twt_status_append_cut_line(&msg);\n-\t\t\tstrbuf_commented_addf(&msg, comment_line_char, \"\\n\");\n+\t\t\tstrbuf_commented_addf(&msg, \"\\n\");\n \t\t}\n-\t\tstrbuf_commented_addf(&msg, comment_line_char,\n+\t\tstrbuf_commented_addf(&msg,\n \t\t\t\t _(merge_editor_comment));\n \t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)\n-\t\t\tstrbuf_commented_addf(&msg, comment_line_char,\n+\t\t\tstrbuf_commented_addf(&msg,\n \t\t\t\t\t _(scissors_editor_comment));\n \t\telse\n-\t\t\tstrbuf_commented_addf(&msg, comment_line_char,\n+\t\t\tstrbuf_commented_addf(&msg,\n \t\t\t\t_(no_scissors_editor_comment), comment_line_char);\n \t}\n \tif (signoff)\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 3918eacbb5..a85a0d8def 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -314,10 +314,10 @@ static void create_tag(const struct object_id *object, const char *object_ref,\n \t\t\tstruct strbuf buf = STRBUF_INIT;\n \t\t\tstrbuf_addch(&buf, '\\n');\n \t\t\tif (opt->cleanup_mode == CLEANUP_ALL)\n-\t\t\t\tstrbuf_commented_addf(&buf, comment_line_char,\n+\t\t\t\tstrbuf_commented_addf(&buf,\n \t\t\t\t _(tag_template), tag, comment_line_char);\n \t\t\telse\n-\t\t\t\tstrbuf_commented_addf(&buf, comment_line_char,\n+\t\t\t\tstrbuf_commented_addf(&buf,\n \t\t\t\t _(tag_template_nocleanup), tag, comment_line_char);\n \t\t\twrite_or_die(fd, buf.buf, buf.len);\n \t\t\tstrbuf_release(&buf);\ndiff --git a/rebase-interactive.c b/rebase-interactive.c\nindex d9718409b3..3f33da7f03 100644\n--- a/rebase-interactive.c\n+++ b/rebase-interactive.c\n@@ -71,7 +71,7 @@ void append_todo_help(int command_count,\n \n \tif (!edit_todo) {\n \t\tstrbuf_addch(buf, '\\n');\n-\t\tstrbuf_commented_addf(buf, comment_line_char,\n+\t\tstrbuf_commented_addf(buf,\n \t\t\t\t Q_(\"Rebase %s onto %s (%d command)\",\n \t\t\t\t\t \"Rebase %s onto %s (%d commands)\",\n \t\t\t\t\t command_count),\ndiff --git a/sequencer.c b/sequencer.c\nindex d584cac8ed..5d348a3f12 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -675,11 +675,11 @@ void append_conflicts_hint(struct index_state *istate,\n \t}\n \n \tstrbuf_addch(msgbuf, '\\n');\n-\tstrbuf_commented_addf(msgbuf, comment_line_char, \"Conflicts:\\n\");\n+\tstrbuf_commented_addf(msgbuf, \"Conflicts:\\n\");\n \tfor (i = 0; i < istate->cache_nr;) {\n \t\tconst struct cache_entry *ce = istate->cache[i++];\n \t\tif (ce_stage(ce)) {\n-\t\t\tstrbuf_commented_addf(msgbuf, comment_line_char,\n+\t\t\tstrbuf_commented_addf(msgbuf,\n \t\t\t\t\t \"\\t%s\\n\", ce->name);\n \t\t\twhile (i < istate->cache_nr &&\n \t\t\t !strcmp(ce->name, istate->cache[i]->name))\ndiff --git a/strbuf.c b/strbuf.c\nindex 7827178d8e..15550b2619 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -1,4 +1,5 @@\n #include \"git-compat-util.h\"\n+#include \"environment.h\"\n #include \"gettext.h\"\n #include \"hex-ll.h\"\n #include \"strbuf.h\"\n@@ -372,7 +373,7 @@ void strbuf_add_commented_lines(struct strbuf *out, const char *buf,\n \tadd_lines(out, prefix1, prefix2, buf, size);\n }\n \n-void strbuf_commented_addf(struct strbuf *sb, char comment_line_char,\n+void strbuf_commented_addf(struct strbuf *sb,\n \t\t\t const char *fmt, ...)\n {\n \tva_list params;\ndiff --git a/strbuf.h b/strbuf.h\nindex e959caca87..981617dc77 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -378,8 +378,8 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...);\n * Add a formatted string prepended by a comment character and a\n * blank to the buffer.\n */\n-__attribute__((format (printf, 3, 4)))\n-void strbuf_commented_addf(struct strbuf *sb, char comment_line_char, const char *fmt, ...);\n+__attribute__((format (printf, 2, 3)))\n+void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);\n \n __attribute__((format (printf,2,0)))\n void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);\ndiff --git a/wt-status.c b/wt-status.c\nindex 9f45bf6949..54b2775730 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1102,7 +1102,7 @@ void wt_status_append_cut_line(struct strbuf *buf)\n {\n \tconst char *explanation = _(\"Do not modify or remove the line above.\\nEverything below it will be ignored.\");\n \n-\tstrbuf_commented_addf(buf, comment_line_char, \"%s\", cut_line);\n+\tstrbuf_commented_addf(buf, \"%s\", cut_line);\n \tstrbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);\n }\n \n-- \n2.42.0.820.g83a721a137-goog\n\n"},{"id":"484260","messageId":"22e96f5ba1a36e96d9577ba279711f7c61308441.1698791220.git.jonathantanmy@google.com","threadId":"60447","inReplyTo":"cover.1698791220.git.jonathantanmy@google.com","subject":"[PATCH v2 2/4] strbuf_add_commented_lines(): drop the comment_line_char parameter","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2023-10-31T22:28:31Z","receivedAt":"2023-10-31T22:28:44Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"From: Junio C Hamano \n\nAll the callers of this function supply the global variable\ncomment_line_char as an argument to its last parameter. Remove the\nparameter to allow us in the future to change the reference to the\nglobal variable with something else, like a function call.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/notes.c | 9 ++++-----\n builtin/stripspace.c | 2 +-\n fmt-merge-msg.c | 9 +++------\n rebase-interactive.c | 6 +++---\n sequencer.c | 10 ++++------\n strbuf.c | 6 +++---\n strbuf.h | 3 +--\n wt-status.c | 4 ++--\n 8 files changed, 21 insertions(+), 28 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 9f38863dd5..355ecce07a 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -181,7 +181,7 @@ static void write_commented_object(int fd, const struct object_id *object)\n \n \tif (strbuf_read(&buf, show.out, 0) < 0)\n \t\tdie_errno(_(\"could not read 'show' output\"));\n-\tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_char);\n+\tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len);\n \twrite_or_die(fd, cbuf.buf, cbuf.len);\n \n \tstrbuf_release(&cbuf);\n@@ -209,10 +209,9 @@ static void prepare_note_data(const struct object_id *object, struct note_data *\n \t\t\tcopy_obj_to_fd(fd, old_note);\n \n \t\tstrbuf_addch(&buf, '\\n');\n-\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"), comment_line_char);\n-\t\tstrbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)),\n-\t\t\t\t\t comment_line_char);\n-\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"), comment_line_char);\n+\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"));\n+\t\tstrbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));\n+\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"));\n \t\twrite_or_die(fd, buf.buf, buf.len);\n \n \t\twrite_commented_object(fd, object);\ndiff --git a/builtin/stripspace.c b/builtin/stripspace.c\nindex 7b700a9fb1..11e475760c 100644\n--- a/builtin/stripspace.c\n+++ b/builtin/stripspace.c\n@@ -13,7 +13,7 @@ static void comment_lines(struct strbuf *buf)\n \tsize_t len;\n \n \tmsg = strbuf_detach(buf, &len);\n-\tstrbuf_add_commented_lines(buf, msg, len, comment_line_char);\n+\tstrbuf_add_commented_lines(buf, msg, len);\n \tfree(msg);\n }\n \ndiff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\nindex 66e47449a0..adc85d2a72 100644\n--- a/fmt-merge-msg.c\n+++ b/fmt-merge-msg.c\n@@ -509,8 +509,7 @@ static void fmt_tag_signature(struct strbuf *tagbuf,\n \tstrbuf_complete_line(tagbuf);\n \tif (sig->len) {\n \t\tstrbuf_addch(tagbuf, '\\n');\n-\t\tstrbuf_add_commented_lines(tagbuf, sig->buf, sig->len,\n-\t\t\t\t\t comment_line_char);\n+\t\tstrbuf_add_commented_lines(tagbuf, sig->buf, sig->len);\n \t}\n }\n \n@@ -556,8 +555,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\t\t\tstrbuf_addch(&tagline, '\\n');\n \t\t\t\tstrbuf_add_commented_lines(&tagline,\n \t\t\t\t\t\torigins.items[first_tag].string,\n-\t\t\t\t\t\tstrlen(origins.items[first_tag].string),\n-\t\t\t\t\t\tcomment_line_char);\n+\t\t\t\t\t\tstrlen(origins.items[first_tag].string));\n \t\t\t\tstrbuf_insert(&tagbuf, 0, tagline.buf,\n \t\t\t\t\t tagline.len);\n \t\t\t\tstrbuf_release(&tagline);\n@@ -565,8 +563,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\t\tstrbuf_addch(&tagbuf, '\\n');\n \t\t\tstrbuf_add_commented_lines(&tagbuf,\n \t\t\t\t\torigins.items[i].string,\n-\t\t\t\t\tstrlen(origins.items[i].string),\n-\t\t\t\t\tcomment_line_char);\n+\t\t\t\t\tstrlen(origins.items[i].string));\n \t\t\tfmt_tag_signature(&tagbuf, &sig, buf, len);\n \t\t}\n \t\tstrbuf_release(&payload);\ndiff --git a/rebase-interactive.c b/rebase-interactive.c\nindex 3f33da7f03..1138bd37ba 100644\n--- a/rebase-interactive.c\n+++ b/rebase-interactive.c\n@@ -78,7 +78,7 @@ void append_todo_help(int command_count,\n \t\t\t\t shortrevisions, shortonto, command_count);\n \t}\n \n-\tstrbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char);\n+\tstrbuf_add_commented_lines(buf, msg, strlen(msg));\n \n \tif (get_missing_commit_check_level() == MISSING_COMMIT_CHECK_ERROR)\n \t\tmsg = _(\"\\nDo not remove any line. Use 'drop' \"\n@@ -87,7 +87,7 @@ void append_todo_help(int command_count,\n \t\tmsg = _(\"\\nIf you remove a line here \"\n \t\t\t \"THAT COMMIT WILL BE LOST.\\n\");\n \n-\tstrbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char);\n+\tstrbuf_add_commented_lines(buf, msg, strlen(msg));\n \n \tif (edit_todo)\n \t\tmsg = _(\"\\nYou are editing the todo file \"\n@@ -98,7 +98,7 @@ void append_todo_help(int command_count,\n \t\tmsg = _(\"\\nHowever, if you remove everything, \"\n \t\t\t\"the rebase will be aborted.\\n\\n\");\n \n-\tstrbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char);\n+\tstrbuf_add_commented_lines(buf, msg, strlen(msg));\n }\n \n int edit_todo_list(struct repository *r, struct todo_list *todo_list,\ndiff --git a/sequencer.c b/sequencer.c\nindex 5d348a3f12..29c8b5e32b 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1859,7 +1859,7 @@ static void add_commented_lines(struct strbuf *buf, const void *str, size_t len)\n \t\ts += count;\n \t\tlen -= count;\n \t}\n-\tstrbuf_add_commented_lines(buf, s, len, comment_line_char);\n+\tstrbuf_add_commented_lines(buf, s, len);\n }\n \n /* Does the current fixup chain contain a squash command? */\n@@ -1958,7 +1958,7 @@ static int append_squash_message(struct strbuf *buf, const char *body,\n \tstrbuf_addf(buf, _(nth_commit_msg_fmt),\n \t\t ++opts->current_fixup_count + 1);\n \tstrbuf_addstr(buf, \"\\n\\n\");\n-\tstrbuf_add_commented_lines(buf, body, commented_len, comment_line_char);\n+\tstrbuf_add_commented_lines(buf, body, commented_len);\n \t/* buf->buf may be reallocated so store an offset into the buffer */\n \tfixup_off = buf->len;\n \tstrbuf_addstr(buf, body + commented_len);\n@@ -2048,8 +2048,7 @@ static int update_squash_messages(struct repository *r,\n \t\t\t _(first_commit_msg_str));\n \t\tstrbuf_addstr(&buf, \"\\n\\n\");\n \t\tif (is_fixup_flag(command, flag))\n-\t\t\tstrbuf_add_commented_lines(&buf, body, strlen(body),\n-\t\t\t\t\t\t comment_line_char);\n+\t\t\tstrbuf_add_commented_lines(&buf, body, strlen(body));\n \t\telse\n \t\t\tstrbuf_addstr(&buf, body);\n \n@@ -2068,8 +2067,7 @@ static int update_squash_messages(struct repository *r,\n \t\tstrbuf_addf(&buf, _(skip_nth_commit_msg_fmt),\n \t\t\t ++opts->current_fixup_count + 1);\n \t\tstrbuf_addstr(&buf, \"\\n\\n\");\n-\t\tstrbuf_add_commented_lines(&buf, body, strlen(body),\n-\t\t\t\t\t comment_line_char);\n+\t\tstrbuf_add_commented_lines(&buf, body, strlen(body));\n \t} else\n \t\treturn error(_(\"unknown command: %d\"), command);\n \trepo_unuse_commit_buffer(r, commit, message);\ndiff --git a/strbuf.c b/strbuf.c\nindex 15550b2619..2088f7800a 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -360,8 +360,8 @@ static void add_lines(struct strbuf *out,\n \tstrbuf_complete_line(out);\n }\n \n-void strbuf_add_commented_lines(struct strbuf *out, const char *buf,\n-\t\t\t\tsize_t size, char comment_line_char)\n+void strbuf_add_commented_lines(struct strbuf *out,\n+\t\t\t\tconst char *buf, size_t size)\n {\n \tstatic char prefix1[3];\n \tstatic char prefix2[2];\n@@ -384,7 +384,7 @@ void strbuf_commented_addf(struct strbuf *sb,\n \tstrbuf_vaddf(&buf, fmt, params);\n \tva_end(params);\n \n-\tstrbuf_add_commented_lines(sb, buf.buf, buf.len, comment_line_char);\n+\tstrbuf_add_commented_lines(sb, buf.buf, buf.len);\n \tif (incomplete_line)\n \t\tsb->buf[--sb->len] = '\\0';\n \ndiff --git a/strbuf.h b/strbuf.h\nindex 981617dc77..4547efa62e 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -287,8 +287,7 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n * by a comment character and a blank.\n */\n void strbuf_add_commented_lines(struct strbuf *out,\n-\t\t\t\tconst char *buf, size_t size,\n-\t\t\t\tchar comment_line_char);\n+\t\t\t\tconst char *buf, size_t size);\n \n \n /**\ndiff --git a/wt-status.c b/wt-status.c\nindex 54b2775730..b390c77334 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1027,7 +1027,7 @@ static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncom\n \tif (s->display_comment_prefix) {\n \t\tsize_t len;\n \t\tsummary_content = strbuf_detach(&summary, &len);\n-\t\tstrbuf_add_commented_lines(&summary, summary_content, len, comment_line_char);\n+\t\tstrbuf_add_commented_lines(&summary, summary_content, len);\n \t\tfree(summary_content);\n \t}\n \n@@ -1103,7 +1103,7 @@ void wt_status_append_cut_line(struct strbuf *buf)\n \tconst char *explanation = _(\"Do not modify or remove the line above.\\nEverything below it will be ignored.\");\n \n \tstrbuf_commented_addf(buf, \"%s\", cut_line);\n-\tstrbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);\n+\tstrbuf_add_commented_lines(buf, explanation, strlen(explanation));\n }\n \n void wt_status_add_cut_line(FILE *fp)\n-- \n2.42.0.820.g83a721a137-goog\n\n"},{"id":"484261","messageId":"283f502acb68910cb43d6077eef99d6345aaea4b.1698791220.git.jonathantanmy@google.com","threadId":"60447","inReplyTo":"cover.1698791220.git.jonathantanmy@google.com","subject":"[PATCH v2 3/4] strbuf: make add_lines() public","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2023-10-31T22:28:32Z","receivedAt":"2023-10-31T22:28:45Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"A subsequent patch will require the ability to add different\nprefixes to different lines (depending on their contents), so make\nthis functionality available from outside strbuf.c. The function\nname is chosen to avoid a conflict with the existing function named\nstrbuf_add_lines().\n\nSigned-off-by: Jonathan Tan \n---\n strbuf.c | 22 +++++++++++-----------\n strbuf.h | 4 ++++\n 2 files changed, 15 insertions(+), 11 deletions(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 2088f7800a..d5ee8874f8 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -340,24 +340,24 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)\n \tva_end(ap);\n }\n \n-static void add_lines(struct strbuf *out,\n-\t\t\tconst char *prefix1,\n-\t\t\tconst char *prefix2,\n-\t\t\tconst char *buf, size_t size)\n+void strbuf_add_lines_varied_prefix(struct strbuf *sb,\n+\t\t\t\t const char *default_prefix,\n+\t\t\t\t const char *tab_nl_prefix,\n+\t\t\t\t const char *buf, size_t size)\n {\n \twhile (size) {\n \t\tconst char *prefix;\n \t\tconst char *next = memchr(buf, '\\n', size);\n \t\tnext = next ? (next + 1) : (buf + size);\n \n-\t\tprefix = ((prefix2 && (buf[0] == '\\n' || buf[0] == '\\t'))\n-\t\t\t ? prefix2 : prefix1);\n-\t\tstrbuf_addstr(out, prefix);\n-\t\tstrbuf_add(out, buf, next - buf);\n+\t\tprefix = (buf[0] == '\\n' || buf[0] == '\\t')\n+\t\t\t ? tab_nl_prefix : default_prefix;\n+\t\tstrbuf_addstr(sb, prefix);\n+\t\tstrbuf_add(sb, buf, next - buf);\n \t\tsize -= next - buf;\n \t\tbuf = next;\n \t}\n-\tstrbuf_complete_line(out);\n+\tstrbuf_complete_line(sb);\n }\n \n void strbuf_add_commented_lines(struct strbuf *out,\n@@ -370,7 +370,7 @@ void strbuf_add_commented_lines(struct strbuf *out,\n \t\txsnprintf(prefix1, sizeof(prefix1), \"%c \", comment_line_char);\n \t\txsnprintf(prefix2, sizeof(prefix2), \"%c\", comment_line_char);\n \t}\n-\tadd_lines(out, prefix1, prefix2, buf, size);\n+\tstrbuf_add_lines_varied_prefix(out, prefix1, prefix2, buf, size);\n }\n \n void strbuf_commented_addf(struct strbuf *sb,\n@@ -751,7 +751,7 @@ ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)\n void strbuf_add_lines(struct strbuf *out, const char *prefix,\n \t\t const char *buf, size_t size)\n {\n-\tadd_lines(out, prefix, NULL, buf, size);\n+\tstrbuf_add_lines_varied_prefix(out, prefix, prefix, buf, size);\n }\n \n void strbuf_addstr_xml_quoted(struct strbuf *buf, const char *s)\ndiff --git a/strbuf.h b/strbuf.h\nindex 4547efa62e..a9333ac1ad 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -601,6 +601,10 @@ void strbuf_add_lines(struct strbuf *sb,\n \t\t const char *prefix,\n \t\t const char *buf,\n \t\t size_t size);\n+void strbuf_add_lines_varied_prefix(struct strbuf *sb,\n+\t\t\t\t const char *default_prefix,\n+\t\t\t\t const char *tab_nl_prefix,\n+\t\t\t\t const char *buf, size_t size);\n \n /**\n * Append s to sb, with the characters '<', '>', '&' and '\"' converted\n-- \n2.42.0.820.g83a721a137-goog\n\n"},{"id":"484262","messageId":"4097385820973b30a78f2e45741444a3f6eee98d.1698791220.git.jonathantanmy@google.com","threadId":"60447","inReplyTo":"cover.1698791220.git.jonathantanmy@google.com","subject":"[PATCH v2 4/4] strbuf: move env-using functions to environment.c","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2023-10-31T22:28:33Z","receivedAt":"2023-10-31T22:28:47Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"This eliminates the dependency from strbuf to environment.\n\nSigned-off-by: Jonathan Tan \n---\n environment.c | 32 ++++++++++++++++++++++++++++++++\n environment.h | 14 ++++++++++++++\n strbuf.c | 32 --------------------------------\n strbuf.h | 15 ---------------\n 4 files changed, 46 insertions(+), 47 deletions(-)\n\ndiff --git a/environment.c b/environment.c\nindex bb3c2a96a3..942c5b8dd3 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -18,6 +18,7 @@\n #include \"refs.h\"\n #include \"fmt-merge-msg.h\"\n #include \"commit.h\"\n+#include \"strbuf.h\"\n #include \"strvec.h\"\n #include \"object-file.h\"\n #include \"object-store-ll.h\"\n@@ -416,3 +417,34 @@ int print_sha1_ellipsis(void)\n \t}\n \treturn cached_result;\n }\n+\n+void strbuf_commented_addf(struct strbuf *sb,\n+\t\t\t const char *fmt, ...)\n+{\n+\tva_list params;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint incomplete_line = sb->len && sb->buf[sb->len - 1] != '\\n';\n+\n+\tva_start(params, fmt);\n+\tstrbuf_vaddf(&buf, fmt, params);\n+\tva_end(params);\n+\n+\tstrbuf_add_commented_lines(sb, buf.buf, buf.len);\n+\tif (incomplete_line)\n+\t\tsb->buf[--sb->len] = '\\0';\n+\n+\tstrbuf_release(&buf);\n+}\n+\n+void strbuf_add_commented_lines(struct strbuf *out,\n+\t\t\t\tconst char *buf, size_t size)\n+{\n+\tstatic char prefix1[3];\n+\tstatic char prefix2[2];\n+\n+\tif (prefix1[0] != comment_line_char) {\n+\t\txsnprintf(prefix1, sizeof(prefix1), \"%c \", comment_line_char);\n+\t\txsnprintf(prefix2, sizeof(prefix2), \"%c\", comment_line_char);\n+\t}\n+\tstrbuf_add_lines_varied_prefix(out, prefix1, prefix2, buf, size);\n+}\ndiff --git a/environment.h b/environment.h\nindex e5351c9dd9..f801dbe36e 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -229,4 +229,18 @@ extern const char *excludes_file;\n */\n int print_sha1_ellipsis(void);\n \n+/**\n+ * Add a formatted string prepended by a comment character and a\n+ * blank to the buffer.\n+ */\n+__attribute__((format (printf, 2, 3)))\n+void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);\n+\n+/**\n+ * Add a NUL-terminated string to the buffer. Each line will be prepended\n+ * by a comment character and a blank.\n+ */\n+void strbuf_add_commented_lines(struct strbuf *out,\n+\t\t\t\tconst char *buf, size_t size);\n+\n #endif\ndiff --git a/strbuf.c b/strbuf.c\nindex d5ee8874f8..f6c1978ecf 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -1,5 +1,4 @@\n #include \"git-compat-util.h\"\n-#include \"environment.h\"\n #include \"gettext.h\"\n #include \"hex-ll.h\"\n #include \"strbuf.h\"\n@@ -360,37 +359,6 @@ void strbuf_add_lines_varied_prefix(struct strbuf *sb,\n \tstrbuf_complete_line(sb);\n }\n \n-void strbuf_add_commented_lines(struct strbuf *out,\n-\t\t\t\tconst char *buf, size_t size)\n-{\n-\tstatic char prefix1[3];\n-\tstatic char prefix2[2];\n-\n-\tif (prefix1[0] != comment_line_char) {\n-\t\txsnprintf(prefix1, sizeof(prefix1), \"%c \", comment_line_char);\n-\t\txsnprintf(prefix2, sizeof(prefix2), \"%c\", comment_line_char);\n-\t}\n-\tstrbuf_add_lines_varied_prefix(out, prefix1, prefix2, buf, size);\n-}\n-\n-void strbuf_commented_addf(struct strbuf *sb,\n-\t\t\t const char *fmt, ...)\n-{\n-\tva_list params;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tint incomplete_line = sb->len && sb->buf[sb->len - 1] != '\\n';\n-\n-\tva_start(params, fmt);\n-\tstrbuf_vaddf(&buf, fmt, params);\n-\tva_end(params);\n-\n-\tstrbuf_add_commented_lines(sb, buf.buf, buf.len);\n-\tif (incomplete_line)\n-\t\tsb->buf[--sb->len] = '\\0';\n-\n-\tstrbuf_release(&buf);\n-}\n-\n void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)\n {\n \tint len;\ndiff --git a/strbuf.h b/strbuf.h\nindex a9333ac1ad..d5f0d4c579 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -282,14 +282,6 @@ void strbuf_remove(struct strbuf *sb, size_t pos, size_t len);\n void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n \t\t const void *data, size_t data_len);\n \n-/**\n- * Add a NUL-terminated string to the buffer. Each line will be prepended\n- * by a comment character and a blank.\n- */\n-void strbuf_add_commented_lines(struct strbuf *out,\n-\t\t\t\tconst char *buf, size_t size);\n-\n-\n /**\n * Add data of given length to the buffer.\n */\n@@ -373,13 +365,6 @@ void strbuf_humanise_rate(struct strbuf *buf, off_t bytes);\n __attribute__((format (printf,2,3)))\n void strbuf_addf(struct strbuf *sb, const char *fmt, ...);\n \n-/**\n- * Add a formatted string prepended by a comment character and a\n- * blank to the buffer.\n- */\n-__attribute__((format (printf, 2, 3)))\n-void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);\n-\n __attribute__((format (printf,2,0)))\n void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);\n \n-- \n2.42.0.820.g83a721a137-goog\n\n"},{"id":"484268","messageId":"xmqqpm0uz6tw.fsf@gitster.g","threadId":"60447","inReplyTo":"20231031222400.2048688-1-jonathantanmy@google.com","subject":"Re: [RFC PATCH 2/3] strbuf_commented_addf(): drop the comment_line_char parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-31T23:54:19Z","receivedAt":"2023-10-31T23:54:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> Junio C Hamano writes:\n>> This moving of the helper function does not belong to the \"fix\n>> commented_addf() not to take the comment_line_char\" step.\n>> \n>> The series should be restructured to have the two patches from me\n>> first, and then your moving some stuff to environment.c, probably.\n>\n> This means that #include \"environment.h\" will be added and then removed\n> in the same series,\n\nI do prefer it that way, because that is exactly what we are doing.\nFirst we fix the duplicated parameter in the API by relying on the\nglobal, and then we move things around to hide the dependence on the\nglobal.\n\nThanks.\n"},{"id":"484281","messageId":"xmqq5y2mun2y.fsf@gitster.g","threadId":"60447","inReplyTo":"283f502acb68910cb43d6077eef99d6345aaea4b.1698791220.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 3/4] strbuf: make add_lines() public","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-01T04:14:29Z","receivedAt":"2023-11-01T04:14:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> -static void add_lines(struct strbuf *out,\n> -\t\t\tconst char *prefix1,\n> -\t\t\tconst char *prefix2,\n> -\t\t\tconst char *buf, size_t size)\n> +void strbuf_add_lines_varied_prefix(struct strbuf *sb,\n> +\t\t\t\t const char *default_prefix,\n> +\t\t\t\t const char *tab_nl_prefix,\n> +\t\t\t\t const char *buf, size_t size)\n> {\n> \twhile (size) {\n> \t\tconst char *prefix;\n> \t\tconst char *next = memchr(buf, '\\n', size);\n> \t\tnext = next ? (next + 1) : (buf + size);\n> \n> -\t\tprefix = ((prefix2 && (buf[0] == '\\n' || buf[0] == '\\t'))\n> -\t\t\t ? prefix2 : prefix1);\n> -\t\tstrbuf_addstr(out, prefix);\n> -\t\tstrbuf_add(out, buf, next - buf);\n> +\t\tprefix = (buf[0] == '\\n' || buf[0] == '\\t')\n> +\t\t\t ? tab_nl_prefix : default_prefix;\n> +\t\tstrbuf_addstr(sb, prefix);\n> +\t\tstrbuf_add(sb, buf, next - buf);\n\nThe original allowed callers to pass NULL for the second prefix when\nthey want to use the same prefix, even for commenting out an empty\nline or a line that begins with a tab. The new one does not allow\nthe callers to do so. As long as updating the existing callers are\ndone carefully, the difference would not matter, but would it help\nnew callers in the future to rid the usability feature like this\npatch does while performing a refactoring? The loss of feature is\nnot even documented, by the way.\n\nWhile \"tab_nl\" sound a bit more specific than \"2\", I am not sure if\nwe made it better. It does not make it clear why it makes sense to\n(and it is necessary to) special case HT and LF. A developer who is\nwriting a new caller would not know why there are two prefixes\nsupported, or why the function is named \"varied prefix\", with these\nnames.\n\nGiving a name that explains the reason might help the readability.\nI've been thinking what the best name for this function would be but\nnot successfully.\n\nIt may be that we shouldn't take two prefixes in the first place.\nThe ONLY case callers want to pass prefix2 that is different from\nprefix1 is when prefix1 ends with a space, and prefix2 is identical\nto prefix1 without the trailing space. The reason they use such a\npair of prefixes is to avoid leaving a trailing whitespace (when\nbuf[0] == '\\n') or having a space before tab (when buf[0] == '\\t')\non the generated lines.\n\nSo eventually we may want to have something like this as the final\ninterface given to the public callers, simply because ...\n\n strbuf_add_lines_as_comments(struct strbuf *sb,\n\t\t\t const char *comment_prefix,\n\t\t\t\t const char *buf, size_t size)\n {\n\twhile (size) {\n const char *next = memchr(buf, '\\n', size);\n\t next = next ? (next + 1) : (buf + size);\n\t strbuf_addstr(sb, comment_prefix);\n\t /* avoid trailing-whitespace and space-before-tab */\n\t if (buf[0] != '\\n' && buf[0] != '\\t')\n \t\tstrbuf_addch(sb, ' ');\n\t strbuf_add(sb, buf, next - buf);\n\t ... loop control ...\n\t}\n ... strbuf completion ...\n }\n\n... there is no need for totally unrelated two prefix variants. And\nboth the function name and the parameter name would be a bit easier\nto understand than your version (and far easier than the original).\nThe function is about commenting out all the lines in buf with the\ncomment prefix, and most of the time we add a space between the\ncomment character and the commented out text, but in some cases we\ndo not want to add the space.\n\nBut as I said already, I'd prefer to see a patch that claims to be a\nrefactoring to do as little as necessary. Giving it a name better\nthan add_lines() is inevitable, because you are making it extern.\nBut I'd prefer to see the parameter naems and the function body left\nuntouched and kept the same as the original. It should be left to a\nseparate step to improve the interface and the implementation.\n\nThanks.\n\n\n"},{"id":"484282","messageId":"xmqqy1fit7gj.fsf@gitster.g","threadId":"60447","inReplyTo":"4097385820973b30a78f2e45741444a3f6eee98d.1698791220.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 4/4] strbuf: move env-using functions to environment.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-01T04:37:16Z","receivedAt":"2023-11-01T04:37:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> diff --git a/environment.h b/environment.h\n> index e5351c9dd9..f801dbe36e 100644\n> --- a/environment.h\n> +++ b/environment.h\n> @@ -229,4 +229,18 @@ extern const char *excludes_file;\n> */\n> int print_sha1_ellipsis(void);\n> \n> +/**\n> + * Add a formatted string prepended by a comment character and a\n> + * blank to the buffer.\n> + */\n> +__attribute__((format (printf, 2, 3)))\n> +void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);\n> +\n> +/**\n> + * Add a NUL-terminated string to the buffer. Each line will be prepended\n> + * by a comment character and a blank.\n> + */\n> +void strbuf_add_commented_lines(struct strbuf *out,\n> +\t\t\t\tconst char *buf, size_t size);\n> +\n\nWhat's your plans for globals kept in ident.c for example?\n\nThe reason why I ask is because I do not quite see how making the\nuse of the global comment-line-char variable hidden like this patch\ndoes would help your libification effort. There are many settings\nthat are reasonably expected to be used by many places, and if you\nwant to avoid them, it appears to me that your only way forward\nafter applying this patch would be to recreate the implementation\nthe public git has in environment.[ch] in your version of Git.\nYou'd have to do something similar for what is in ident.c for the\nsame reason.\n\nThe relative size of the logic necessary to split the original\ninto lines and prefix the comment prefix character (which is much\nlarger) and the idea that there is a system wide setting of what the\ncomment prefix character should be (which is miniscule) makes me\nwonder if this is going in the right direction.\n\nThanks.\n\n"}]}