Re: [PATCH 0/2] Avoid passing global comment_line_char repeatedly

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Oct 30, 2023, 09:59 UTC
Message-ID: <db6702ba-11a7-44c1-af2a-95b080aaeb77@gmail.com>
In-Reply-To: <20231030051034.2295242-1-gitster@pobox.com>

Hi Junio

On 30/10/2023 05:10, Junio C Hamano wrote:

Show 9 quoted lines

> 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

Show 18 quoted lines

> 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(-)
>