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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 1, 2023, 04:37 UTC
Message-ID: <xmqqy1fit7gj.fsf@gitster.g>
In-Reply-To: <4097385820973b30a78f2e45741444a3f6eee98d.1698791220.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 22 quoted lines

> 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.