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

Commit Message

Jonathan Tan Oct. 31, 2023, 10:28 p.m. UTC

This eliminates the dependency from strbuf to environment.

Signed-off-by: Jonathan Tan <[e-mail]>
---
 environment.c | 32 ++++++++++++++++++++++++++++++++
 environment.h | 14 ++++++++++++++
 strbuf.c      | 32 --------------------------------
 strbuf.h      | 15 ---------------
 4 files changed, 46 insertions(+), 47 deletions(-)

Comments

Junio C Hamano Nov. 1, 2023, 4:37 a.m. UTC | #1

Jonathan Tan <[e-mail]> 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.

Patch

diff --git a/environment.c b/environment.c
index bb3c2a96a3..942c5b8dd3 100644
--- a/environment.c
+++ b/environment.c
@@ -18,6 +18,7 @@ 
 #include "refs.h"
 #include "fmt-merge-msg.h"
 #include "commit.h"
+#include "strbuf.h"
 #include "strvec.h"
 #include "object-file.h"
 #include "object-store-ll.h"
@@ -416,3 +417,34 @@  int print_sha1_ellipsis(void)
 	}
 	return cached_result;
 }
+
+void strbuf_commented_addf(struct strbuf *sb,
+			   const char *fmt, ...)
+{
+	va_list params;
+	struct strbuf buf = STRBUF_INIT;
+	int incomplete_line = sb->len && sb->buf[sb->len - 1] != '\n';
+
+	va_start(params, fmt);
+	strbuf_vaddf(&buf, fmt, params);
+	va_end(params);
+
+	strbuf_add_commented_lines(sb, buf.buf, buf.len);
+	if (incomplete_line)
+		sb->buf[--sb->len] = '\0';
+
+	strbuf_release(&buf);
+}
+
+void strbuf_add_commented_lines(struct strbuf *out,
+				const char *buf, size_t size)
+{
+	static char prefix1[3];
+	static char prefix2[2];
+
+	if (prefix1[0] != comment_line_char) {
+		xsnprintf(prefix1, sizeof(prefix1), "%c ", comment_line_char);
+		xsnprintf(prefix2, sizeof(prefix2), "%c", comment_line_char);
+	}
+	strbuf_add_lines_varied_prefix(out, prefix1, prefix2, buf, size);
+}
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);
+
 #endif
diff --git a/strbuf.c b/strbuf.c
index d5ee8874f8..f6c1978ecf 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,5 +1,4 @@ 
 #include "git-compat-util.h"
-#include "environment.h"
 #include "gettext.h"
 #include "hex-ll.h"
 #include "strbuf.h"
@@ -360,37 +359,6 @@  void strbuf_add_lines_varied_prefix(struct strbuf *sb,
 	strbuf_complete_line(sb);
 }
 
-void strbuf_add_commented_lines(struct strbuf *out,
-				const char *buf, size_t size)
-{
-	static char prefix1[3];
-	static char prefix2[2];
-
-	if (prefix1[0] != comment_line_char) {
-		xsnprintf(prefix1, sizeof(prefix1), "%c ", comment_line_char);
-		xsnprintf(prefix2, sizeof(prefix2), "%c", comment_line_char);
-	}
-	strbuf_add_lines_varied_prefix(out, prefix1, prefix2, buf, size);
-}
-
-void strbuf_commented_addf(struct strbuf *sb,
-			   const char *fmt, ...)
-{
-	va_list params;
-	struct strbuf buf = STRBUF_INIT;
-	int incomplete_line = sb->len && sb->buf[sb->len - 1] != '\n';
-
-	va_start(params, fmt);
-	strbuf_vaddf(&buf, fmt, params);
-	va_end(params);
-
-	strbuf_add_commented_lines(sb, buf.buf, buf.len);
-	if (incomplete_line)
-		sb->buf[--sb->len] = '\0';
-
-	strbuf_release(&buf);
-}
-
 void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)
 {
 	int len;
diff --git a/strbuf.h b/strbuf.h
index a9333ac1ad..d5f0d4c579 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -282,14 +282,6 @@  void strbuf_remove(struct strbuf *sb, size_t pos, size_t len);
 void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
 		   const void *data, size_t data_len);
 
-/**
- * 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);
-
-
 /**
  * Add data of given length to the buffer.
  */
@@ -373,13 +365,6 @@  void strbuf_humanise_rate(struct strbuf *buf, off_t bytes);
 __attribute__((format (printf,2,3)))
 void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
 
-/**
- * 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, ...);
-
 __attribute__((format (printf,2,0)))
 void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);