[PATCH] trailer: spread usage of "trailer_block" language

[PATCH] trailer: spread usage of "trailer_block" language

[Linus Arver via GitGitGadget]

From: Linus Arver <linusa@google.com>

Deprecate the "trailer_info" struct name and replace it with
"trailer_block". This is more readable, for two reasons:

  1. "trailer_info" on the surface sounds like it's about a single
     trailer when in reality it is a collection of one or more trailers,
     and

  2. the "*_block" suffix is more informative than "*_info", because it
     describes a block (or region) of contiguous text which has trailers
     in it, which has been parsed into the trailer_block structure.

Rename the

    size_t trailer_block_start, trailer_block_end;

members of trailer_info to just "start" and "end". Rename the "info"
pointer to "trailer_block" because it is more descriptive. Update
comments accordingly.

Signed-off-by: Linus Arver <linus@ucla.edu>
---
    trailer: spread usage of "trailer_block" language

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1811%2Flistx%2Ftrailer-cleanup-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1811/listx/trailer-cleanup-v1
Pull-Request: https://github.com/git/git/pull/1811

 builtin/interpret-trailers.c | 25 +++++-----
 trailer.c                    | 95 ++++++++++++++++++------------------
 trailer.h                    | 30 ++++++------
 3 files changed, 76 insertions(+), 74 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index c5e56e2cd3d..44d8ccddc9d 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -141,8 +141,8 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 {
 	LIST_HEAD(head);
 	struct strbuf sb = STRBUF_INIT;
-	struct strbuf trailer_block = STRBUF_INIT;
-	struct trailer_info *info;
+	struct strbuf trailer_block_sb = STRBUF_INIT;
+	struct trailer_block *trailer_block;
 	FILE *outfile = stdout;
 
 	trailer_config_init();
@@ -152,13 +152,13 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 	if (opts->in_place)
 		outfile = create_in_place_tempfile(file);
 
-	info = parse_trailers(opts, sb.buf, &head);
+	trailer_block = parse_trailers(opts, sb.buf, &head);
 
-	/* Print the lines before the trailers */
+	/* Print the lines before the trailer block */
 	if (!opts->only_trailers)
-		fwrite(sb.buf, 1, trailer_block_start(info), outfile);
+		fwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);
 
-	if (!opts->only_trailers && !blank_line_before_trailer_block(info))
+	if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
 		fprintf(outfile, "\n");
 
 
@@ -172,15 +172,16 @@ static void interpret_trailers(const struct process_trailer_options *opts,
 	}
 
 	/* Print trailer block. */
-	format_trailers(opts, &head, &trailer_block);
+	format_trailers(opts, &head, &trailer_block_sb);
 	free_trailers(&head);
-	fwrite(trailer_block.buf, 1, trailer_block.len, outfile);
-	strbuf_release(&trailer_block);
+	fwrite(trailer_block_sb.buf, 1, trailer_block_sb.len, outfile);
+	strbuf_release(&trailer_block_sb);
 
-	/* Print the lines after the trailers as is */
+	/* Print the lines after the trailer block as is. */
 	if (!opts->only_trailers)
-		fwrite(sb.buf + trailer_block_end(info), 1, sb.len - trailer_block_end(info), outfile);
-	trailer_info_release(info);
+		fwrite(sb.buf + trailer_block_end(trailer_block), 1,
+		       sb.len - trailer_block_end(trailer_block), outfile);
+	trailer_block_release(trailer_block);
 
 	if (opts->in_place)
 		if (rename_tempfile(&trailers_tempfile, file))
diff --git a/trailer.c b/trailer.c
index 682d74505bf..59affa2159b 100644
--- a/trailer.c
+++ b/trailer.c
@@ -13,19 +13,20 @@
  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
  */
 
-struct trailer_info {
+struct trailer_block {
 	/*
 	 * True if there is a blank line before the location pointed to by
-	 * trailer_block_start.
+	 * "start".
 	 */
 	int blank_line_before_trailer;
 
 	/*
-	 * Offsets to the trailer block start and end positions in the input
-	 * string. If no trailer block is found, these are both set to the
-	 * "true" end of the input (find_end_of_log_message()).
+	 * The locations of the start and end positions of the trailer block
+	 * found, as offsets from the beginning of the source text from which
+	 * this trailer block was parsed. If no trailer block is found, these
+	 * are both set to 0.
 	 */
-	size_t trailer_block_start, trailer_block_end;
+	size_t start, end;
 
 	/*
 	 * Array of trailers found.
@@ -975,16 +976,16 @@ static void unfold_value(struct strbuf *val)
 	strbuf_release(&out);
 }
 
-static struct trailer_info *trailer_info_new(void)
+static struct trailer_block *trailer_block_new(void)
 {
-	struct trailer_info *info = xcalloc(1, sizeof(*info));
-	return info;
+	struct trailer_block *trailer_block = xcalloc(1, sizeof(*trailer_block));
+	return trailer_block;
 }
 
-static struct trailer_info *trailer_info_get(const struct process_trailer_options *opts,
-					     const char *str)
+static struct trailer_block *trailer_block_get(const struct process_trailer_options *opts,
+					       const char *str)
 {
-	struct trailer_info *info = trailer_info_new();
+	struct trailer_block *trailer_block = trailer_block_new();
 	size_t end_of_log_message = 0, trailer_block_start = 0;
 	struct strbuf **trailer_lines, **ptr;
 	char **trailer_strings = NULL;
@@ -1017,34 +1018,34 @@ static struct trailer_info *trailer_info_get(const struct process_trailer_option
 	}
 	strbuf_list_free(trailer_lines);
 
-	info->blank_line_before_trailer = ends_with_blank_line(str,
-							       trailer_block_start);
-	info->trailer_block_start = trailer_block_start;
-	info->trailer_block_end = end_of_log_message;
-	info->trailers = trailer_strings;
-	info->trailer_nr = nr;
+	trailer_block->blank_line_before_trailer = ends_with_blank_line(str,
+									trailer_block_start);
+	trailer_block->start = trailer_block_start;
+	trailer_block->end = end_of_log_message;
+	trailer_block->trailers = trailer_strings;
+	trailer_block->trailer_nr = nr;
 
-	return info;
+	return trailer_block;
 }
 
 /*
- * Parse trailers in "str", populating the trailer info and "trailer_objects"
+ * Parse trailers in "str", populating the trailer_block and "trailer_objects"
  * linked list structure.
  */
-struct trailer_info *parse_trailers(const struct process_trailer_options *opts,
-				    const char *str,
-				    struct list_head *trailer_objects)
+struct trailer_block *parse_trailers(const struct process_trailer_options *opts,
+				     const char *str,
+				     struct list_head *trailer_objects)
 {
-	struct trailer_info *info;
+	struct trailer_block *trailer_block;
 	struct strbuf tok = STRBUF_INIT;
 	struct strbuf val = STRBUF_INIT;
 	size_t i;
 
-	info = trailer_info_get(opts, str);
+	trailer_block = trailer_block_get(opts, str);
 
-	for (i = 0; i < info->trailer_nr; i++) {
+	for (i = 0; i < trailer_block->trailer_nr; i++) {
 		int separator_pos;
-		char *trailer = info->trailers[i];
+		char *trailer = trailer_block->trailers[i];
 		if (starts_with(trailer, comment_line_str))
 			continue;
 		separator_pos = find_separator(trailer, separators);
@@ -1065,7 +1066,7 @@ struct trailer_info *parse_trailers(const struct process_trailer_options *opts,
 		}
 	}
 
-	return info;
+	return trailer_block;
 }
 
 void free_trailers(struct list_head *trailers)
@@ -1077,28 +1078,28 @@ void free_trailers(struct list_head *trailers)
 	}
 }
 
-size_t trailer_block_start(struct trailer_info *info)
+size_t trailer_block_start(struct trailer_block *trailer_block)
 {
-	return info->trailer_block_start;
+	return trailer_block->start;
 }
 
-size_t trailer_block_end(struct trailer_info *info)
+size_t trailer_block_end(struct trailer_block *trailer_block)
 {
-	return info->trailer_block_end;
+	return trailer_block->end;
 }
 
-int blank_line_before_trailer_block(struct trailer_info *info)
+int blank_line_before_trailer_block(struct trailer_block *trailer_block)
 {
-	return info->blank_line_before_trailer;
+	return trailer_block->blank_line_before_trailer;
 }
 
-void trailer_info_release(struct trailer_info *info)
+void trailer_block_release(struct trailer_block *trailer_block)
 {
 	size_t i;
-	for (i = 0; i < info->trailer_nr; i++)
-		free(info->trailers[i]);
-	free(info->trailers);
-	free(info);
+	for (i = 0; i < trailer_block->trailer_nr; i++)
+		free(trailer_block->trailers[i]);
+	free(trailer_block->trailers);
+	free(trailer_block);
 }
 
 void format_trailers(const struct process_trailer_options *opts,
@@ -1166,19 +1167,19 @@ void format_trailers_from_commit(const struct process_trailer_options *opts,
 				 struct strbuf *out)
 {
 	LIST_HEAD(trailer_objects);
-	struct trailer_info *info = parse_trailers(opts, msg, &trailer_objects);
+	struct trailer_block *trailer_block = parse_trailers(opts, msg, &trailer_objects);
 
 	/* If we want the whole block untouched, we can take the fast path. */
 	if (!opts->only_trailers && !opts->unfold && !opts->filter &&
 	    !opts->separator && !opts->key_only && !opts->value_only &&
 	    !opts->key_value_separator) {
-		strbuf_add(out, msg + info->trailer_block_start,
-			   info->trailer_block_end - info->trailer_block_start);
+		strbuf_add(out, msg + trailer_block->start,
+			   trailer_block->end - trailer_block->start);
 	} else
 		format_trailers(opts, &trailer_objects, out);
 
 	free_trailers(&trailer_objects);
-	trailer_info_release(info);
+	trailer_block_release(trailer_block);
 }
 
 void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
@@ -1187,14 +1188,14 @@ void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
 	strbuf_init(&iter->key, 0);
 	strbuf_init(&iter->val, 0);
 	opts.no_divider = 1;
-	iter->internal.info = trailer_info_get(&opts, msg);
+	iter->internal.trailer_block = trailer_block_get(&opts, msg);
 	iter->internal.cur = 0;
 }
 
 int trailer_iterator_advance(struct trailer_iterator *iter)
 {
-	if (iter->internal.cur < iter->internal.info->trailer_nr) {
-		char *line = iter->internal.info->trailers[iter->internal.cur++];
+	if (iter->internal.cur < iter->internal.trailer_block->trailer_nr) {
+		char *line = iter->internal.trailer_block->trailers[iter->internal.cur++];
 		int separator_pos = find_separator(line, separators);
 
 		iter->raw = line;
@@ -1211,7 +1212,7 @@ int trailer_iterator_advance(struct trailer_iterator *iter)
 
 void trailer_iterator_release(struct trailer_iterator *iter)
 {
-	trailer_info_release(iter->internal.info);
+	trailer_block_release(iter->internal.trailer_block);
 	strbuf_release(&iter->val);
 	strbuf_release(&iter->key);
 }
diff --git a/trailer.h b/trailer.h
index 6eb53df155e..4740549586a 100644
--- a/trailer.h
+++ b/trailer.h
@@ -4,7 +4,7 @@
 #include "list.h"
 #include "strbuf.h"
 
-struct trailer_info;
+struct trailer_block;
 struct strvec;
 
 enum trailer_where {
@@ -72,12 +72,12 @@ void process_trailers_lists(struct list_head *head,
 			    struct list_head *arg_head);
 
 /*
- * Given some input string "str", return a pointer to an opaque trailer_info
+ * Given some input string "str", return a pointer to an opaque trailer_block
  * structure. Also populate the trailer_objects list with parsed trailer
  * objects. Internally this calls trailer_info_get() to get the opaque pointer,
  * but does some extra work to populate the trailer_objects linked list.
  *
- * The opaque trailer_info pointer can be used to check the position of the
+ * The opaque trailer_block pointer can be used to check the position of the
  * trailer block as offsets relative to the beginning of "str" in
  * trailer_block_start() and trailer_block_end().
  * blank_line_before_trailer_block() returns 1 if there is a blank line just
@@ -89,21 +89,21 @@ void process_trailers_lists(struct list_head *head,
  * For iterating through the parsed trailer block (if you don't care about the
  * position of the trailer block itself in the context of the larger string text
  * from which it was parsed), please see trailer_iterator_init() which uses the
- * trailer_info struct internally.
+ * trailer_block struct internally.
  *
  * Lastly, callers should call trailer_info_release() when they are done using
  * the opaque pointer.
  *
- * NOTE: Callers should treat both trailer_info and trailer_objects as
- * read-only items, because there is some overlap between the two (trailer_info
+ * NOTE: Callers should treat both trailer_block and trailer_objects as
+ * read-only items, because there is some overlap between the two (trailer_block
  * has "char **trailers" string array, and trailer_objects will have the same
  * data but as a linked list of trailer_item objects). This API does not perform
  * any synchronization between the two. In the future we should be able to
  * reduce the duplication and use just the linked list.
  */
-struct trailer_info *parse_trailers(const struct process_trailer_options *,
-				    const char *str,
-				    struct list_head *trailer_objects);
+struct trailer_block *parse_trailers(const struct process_trailer_options *,
+				     const char *str,
+				     struct list_head *trailer_objects);
 
 /*
  * Return the offset of the start of the trailer block. That is, 0 is the start
@@ -111,24 +111,24 @@ struct trailer_info *parse_trailers(const struct process_trailer_options *,
  * indicates how many bytes we have to skip over before we get to the beginning
  * of the trailer block.
  */
-size_t trailer_block_start(struct trailer_info *);
+size_t trailer_block_start(struct trailer_block *);
 
 /*
  * Return the end of the trailer block, again relative to the start of the
  * input.
  */
-size_t trailer_block_end(struct trailer_info *);
+size_t trailer_block_end(struct trailer_block *);
 
 /*
  * Return 1 if the trailer block had an extra newline (blank line) just before
  * it.
  */
-int blank_line_before_trailer_block(struct trailer_info *);
+int blank_line_before_trailer_block(struct trailer_block *);
 
 /*
- * Free trailer_info struct.
+ * Free trailer_block struct.
  */
-void trailer_info_release(struct trailer_info *info);
+void trailer_block_release(struct trailer_block *);
 
 void trailer_config_init(void);
 void format_trailers(const struct process_trailer_options *,
@@ -167,7 +167,7 @@ struct trailer_iterator {
 
 	/* private */
 	struct {
-		struct trailer_info *info;
+		struct trailer_block *trailer_block;
 		size_t cur;
 	} internal;
 };

base-commit: ef8ce8f3d4344fd3af049c17eeba5cd20d98b69f
-- 
gitgitgadget

Re: [PATCH] trailer: spread usage of "trailer_block" language

[Linus Arver]

"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Linus Arver <linusa@google.com>
>

Hmm. I just noticed that GGG (?) is somehow inferring my defunct
@google.com address. Not sure how to fix this... any tips?

Re: [PATCH] trailer: spread usage of "trailer_block" language

[Taylor Blau]

On Tue, Oct 15, 2024 at 04:32:42AM -0700, Linus Arver wrote:
> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Linus Arver <linusa@google.com>
> >
>
> Hmm. I just noticed that GGG (?) is somehow inferring my defunct
> @google.com address. Not sure how to fix this... any tips?

Perhaps Johannes (CC'd) would know?

Thanks,
Taylor

Re: [PATCH] trailer: spread usage of "trailer_block" language

[Johannes Schindelin]

Hi Linus,

On Tue, 15 Oct 2024, Linus Arver wrote:

> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Linus Arver <linusa@google.com>
> >
>
> Hmm. I just noticed that GGG (?) is somehow inferring my defunct
> @google.com address. Not sure how to fix this... any tips?

This email address is apparently part of the commit object, see
https://github.com/git/git/commit/a556a5c05c44e521b572d595d5d32cc4158612c0.patch

(you can also see it locally if you call `git show --no-mailmap
a556a5c05c44e521b572d595d5d32cc4158612c0`, but not if you omit
`--no-mailmap`)

Ciao,
Johannes

Re: [PATCH] trailer: spread usage of "trailer_block" language

[Junio C Hamano]

"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Linus Arver <linusa@google.com>
>
> Deprecate the "trailer_info" struct name and replace it with
> "trailer_block". This is more readable, for two reasons:
>
>   1. "trailer_info" on the surface sounds like it's about a single
>      trailer when in reality it is a collection of one or more trailers,
>      and
>
>   2. the "*_block" suffix is more informative than "*_info", because it
>      describes a block (or region) of contiguous text which has trailers
>      in it, which has been parsed into the trailer_block structure.
>
> Rename the
>
>     size_t trailer_block_start, trailer_block_end;
>
> members of trailer_info to just "start" and "end". Rename the "info"
> pointer to "trailer_block" because it is more descriptive. Update
> comments accordingly.

All makes sense.  Often "_info" suffix has very low information
density, as everything is "info" in a sense ;-)

This was a more-or-less mechanical and straight-forward renaming of
a handful of variables and structure fields.  It is a shame that
nobody bothered to review these changes (or say "this does not make
anything worse, but is it worth it?" to object to it, for that
matter) for almost a month.

Will merge to 'next' (unless there is a belated "it may not break,
but it is not a good idea because ...", that is).

Thanks.

Re: [PATCH] trailer: spread usage of "trailer_block" language

[Linus Arver]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Linus,
>
> On Tue, 15 Oct 2024, Linus Arver wrote:
>
>> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>> > From: Linus Arver <linusa@google.com>
>> >
>>
>> Hmm. I just noticed that GGG (?) is somehow inferring my defunct
>> @google.com address. Not sure how to fix this... any tips?
>
> This email address is apparently part of the commit object, see
> https://github.com/git/git/commit/a556a5c05c44e521b572d595d5d32cc4158612c0.patch
>
> (you can also see it locally if you call `git show --no-mailmap
> a556a5c05c44e521b572d595d5d32cc4158612c0`, but not if you omit
> `--no-mailmap`)

Ah, thanks! So the commit's Author field is actually set to the google
email, but tig and other Git subcommands were using the .mailmap file to
replace it with the proper (ucla) one. I didn't realize this was
happening, and didn't bother updating the commit itself to use the
email.

Re: [PATCH] trailer: spread usage of "trailer_block" language

[Linus Arver]

Junio C Hamano <gitster@pobox.com> writes:

> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Linus Arver <linusa@google.com>
>>
>> Deprecate the "trailer_info" struct name and replace it with
>> "trailer_block". This is more readable, for two reasons:
>>
>>   1. "trailer_info" on the surface sounds like it's about a single
>>      trailer when in reality it is a collection of one or more trailers,
>>      and
>>
>>   2. the "*_block" suffix is more informative than "*_info", because it
>>      describes a block (or region) of contiguous text which has trailers
>>      in it, which has been parsed into the trailer_block structure.
>>
>> Rename the
>>
>>     size_t trailer_block_start, trailer_block_end;
>>
>> members of trailer_info to just "start" and "end". Rename the "info"
>> pointer to "trailer_block" because it is more descriptive. Update
>> comments accordingly.
>
> All makes sense.  Often "_info" suffix has very low information
> density, as everything is "info" in a sense ;-)

Exactly.

> Will merge to 'next' (unless there is a belated "it may not break,
> but it is not a good idea because ...", that is).

Yup, sounds good. Thanks!