| Message ID | 20231030051034.2295242-1-gitster@pobox.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Avoid passing global comment_line_char repeatedly | expand |
On 2023-10-30 06:10, Junio C Hamano wrote: > Two strbuf functions used to produce commented lines take the > comment_line_char as their parameter, but in practice, all callers > feed the global variable comment_line_char from environment.[ch]. > > Dropping the parameter from the callchain will make the interface > less flexible, and less error prone. If we choose to change the > implementation of the customizable comment line character (e.g., we > may want to stop referencing the global variable and instead use a > getter function), we will have fewer places we need to modify. > > Junio C Hamano (2): > strbuf_commented_addf(): drop the comment_line_char parameter > strbuf_add_commented_lines(): drop the comment_line_char parameter This series looks good to me. It removes unnecessary complexity that provided pretty much no value. > add-patch.c | 8 ++++---- > builtin/branch.c | 2 +- > builtin/merge.c | 8 ++++---- > builtin/notes.c | 9 ++++----- > builtin/stripspace.c | 2 +- > builtin/tag.c | 4 ++-- > fmt-merge-msg.c | 9 +++------ > rebase-interactive.c | 8 ++++---- > sequencer.c | 14 ++++++-------- > strbuf.c | 9 +++++---- > strbuf.h | 7 +++---- > wt-status.c | 6 +++--- > 12 files changed, 40 insertions(+), 46 deletions(-)
Hi Junio On 30/10/2023 05:10, Junio C Hamano wrote: > Two strbuf functions used to produce commented lines take the > comment_line_char as their parameter, but in practice, all callers > feed the global variable comment_line_char from environment.[ch]. > > Dropping the parameter from the callchain will make the interface > less flexible, and less error prone. If we choose to change the > implementation of the customizable comment line character (e.g., we > may want to stop referencing the global variable and instead use a > getter function), we will have fewer places we need to modify. While I agree with your reasoning here, I think that parameter was recently added as part of the libification effort - I can't remember exactly why and am too lazy to look it up so I've cc'd Calvin and Johathan instead. Best Wishes Phillip > Junio C Hamano (2): > strbuf_commented_addf(): drop the comment_line_char parameter > strbuf_add_commented_lines(): drop the comment_line_char parameter > > add-patch.c | 8 ++++---- > builtin/branch.c | 2 +- > builtin/merge.c | 8 ++++---- > builtin/notes.c | 9 ++++----- > builtin/stripspace.c | 2 +- > builtin/tag.c | 4 ++-- > fmt-merge-msg.c | 9 +++------ > rebase-interactive.c | 8 ++++---- > sequencer.c | 14 ++++++-------- > strbuf.c | 9 +++++---- > strbuf.h | 7 +++---- > wt-status.c | 6 +++--- > 12 files changed, 40 insertions(+), 46 deletions(-) >