Subject: Re: [PATCH v2 4/4] strbuf: move env-using functions to environment.c

From: Junio C Hamano <[email-masked]>
Date: Wed, 01 Nov 2023 13:37:16 +0900

Jonathan Tan <[email-masked]> writes:

> diff --git a/environment.h b/environment.h
> index e5351c9dd9..f801dbe36e 100644
> --- a/environment.h
> +++ b/environment.h
> @@ -229,4 +229,18 @@ extern const char *excludes_file;
>   */
>  int print_sha1_ellipsis(void);
>  
> +/**
> + * Add a formatted string prepended by a comment character and a
> + * blank to the buffer.
> + */
> +__attribute__((format (printf, 2, 3)))
> +void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);
> +
> +/**
> + * Add a NUL-terminated string to the buffer. Each line will be prepended
> + * by a comment character and a blank.
> + */
> +void strbuf_add_commented_lines(struct strbuf *out,
> +				const char *buf, size_t size);
> +

What's your plans for globals kept in ident.c for example?

The reason why I ask is because I do not quite see how making the
use of the global comment-line-char variable hidden like this patch
does would help your libification effort.  There are many settings
that are reasonably expected to be used by many places, and if you
want to avoid them, it appears to me that your only way forward
after applying this patch would be to recreate the implementation
the public git has in environment.[ch] in your version of Git.
You'd have to do something similar for what is in ident.c for the
same reason.

The relative size of the logic necessary to split the original
into lines and prefix the comment prefix character (which is much
larger) and the idea that there is a system wide setting of what the
comment prefix character should be (which is miniscule) makes me
wonder if this is going in the right direction.

Thanks.