From: "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Glen Choo <glencbz@gmail.com>,
Christian Couder <chriscool@tuxfamily.org>,
Phillip Wood <phillip.wood123@gmail.com>,
Jonathan Tan <jonathantanmy@google.com>,
Linus Arver <linusa@google.com>
Subject: [PATCH v5 0/3] Trailer readability cleanups
Date: Fri, 20 Oct 2023 19:01:32 +0000 [thread overview]
Message-ID: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1563.v4.git.1695709372.gitgitgadget@gmail.com>
These patches were created while digging into the trailer code to better
understand how it works, in preparation for making the trailer.{c,h} files
as small as possible to make them available as a library for external users.
This series was originally created as part of [1], but are sent here
separately because the changes here are arguably more subjective in nature.
These patches do not add or change any features. Instead, their goal is to
make the code easier to understand for new contributors (like myself), by
making various cleanups and improvements. Ultimately, my hope is that with
such cleanups, we are better positioned to make larger changes (especially
the broader libification effort, as in "Introduce Git Standard Library"
[2]).
Updates in v5
=============
* Patch 4 ("trailer: only use trailer_block_* variables if trailers were
found") has been dropped.
* Patch 2 returns early if "--no-divider" is true, avoiding unnecessary
loop iterations (thanks Jonathan).
* Added missing Reported-by trailer for Patch 3 (it was originally Glen's
idea to use offsets).
* Patch 3: Fixed typo in "trailer.h" that referred to an obsolete function
name ("find_true_end_of_input()", instead of
"find_end_of_log_message()").
Updates in v4
=============
* The first 3 patches in v3 were merged into 'master'. Necessarily, those 3
patches have been dropped.
* Patch 4 in v3 ("trailer: rename *_DEFAULT enums to *_UNSPECIFIED") has
been dropped, as well as Patch 9 in v3 ("trailer: make stack variable
names match field names"). These were dropped to simplify this series for
what I think is the more immediate, important change (see next bullet
point).
* Patches 5-8 in v3 are the only ones remaining in this series. They still
solely deal with --no-divider and trailer block start/end cleanups.
Updates in v3
=============
* Patches 4 and 6 (--no-divider and trailer block start/end cleanups) have
been reorganized to Patches 5-8. This ended up touching commit.c in a
minor way, but otherwise all of the changes here are cleanups and do not
change any behavior.
Updates in v2
=============
* Patch 1: Drop the use of a #define. Instead just use an anonymous struct
named internal.
* Patch 2: Don't free info out parameter inside parse_trailers(). Instead
free it from the caller, process_trailers(). Update comment in
parse_trailers().
* Patch 3: Reword commit message.
* Patch 4: Mention be3d654343 (commit: pass --no-divider to
interpret-trailers, 2023-06-17) in commit message.
* Added Patch 6 to make trailer_info use offsets for trailer_start and
trailer_end (thanks to Glen Choo for the suggestion).
[1]
https://lore.kernel.org/git/pull.1564.git.1691210737.gitgitgadget@gmail.com/T/#mb044012670663d8eb7a548924bbcc933bef116de
[2]
https://lore.kernel.org/git/20230627195251.1973421-1-calvinwan@google.com/
[3]
https://lore.kernel.org/git/pull.1149.git.1677143700.gitgitgadget@gmail.com/
[4]
https://lore.kernel.org/git/6b4cb31b17077181a311ca87e82464a1e2ad67dd.1686797630.git.gitgitgadget@gmail.com/
[5]
https://lore.kernel.org/git/pull.1563.git.1691211879.gitgitgadget@gmail.com/T/#m0131f9829c35d8e0103ffa88f07d8e0e43dd732c
Linus Arver (3):
commit: ignore_non_trailer computes number of bytes to ignore
trailer: find the end of the log message
trailer: use offsets for trailer_start/trailer_end
builtin/commit.c | 2 +-
builtin/merge.c | 2 +-
commit.c | 2 +-
commit.h | 4 +--
sequencer.c | 2 +-
trailer.c | 85 +++++++++++++++++++++++++++++-------------------
trailer.h | 10 +++---
7 files changed, 62 insertions(+), 45 deletions(-)
base-commit: bcb6cae2966cc407ca1afc77413b3ef11103c175
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1563%2Flistx%2Ftrailer-libification-prep-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1563/listx/trailer-libification-prep-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1563
Range-diff vs v4:
1: 4ce5cf77005 = 1: 4ce5cf77005 commit: ignore_non_trailer computes number of bytes to ignore
2: c904caba7e1 ! 2: ce25420db29 trailer: find the end of the log message
@@ Commit message
the starting point which find_trailer_start() needs to start searching
backward to parse individual trailers (if any).
+ Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Linus Arver <linusa@google.com>
@@ trailer.c: static ssize_t last_line(const char *buf, size_t len)
+static size_t find_end_of_log_message(const char *input, int no_divider)
{
+ size_t end;
-+
const char *s;
- for (s = str; *s; s = next_line(s)) {
+ /* Assume the naive end of the input is already what we want. */
+ end = strlen(input);
+
++ if (no_divider) {
++ return end;
++ }
++
+ /* Optionally skip over any patch part ("---" line and below). */
+ for (s = input; *s; s = next_line(s)) {
const char *v;
- if (skip_prefix(s, "---", &v) && isspace(*v))
- return s - str;
-+ if (!no_divider && skip_prefix(s, "---", &v) && isspace(*v)) {
++ if (skip_prefix(s, "---", &v) && isspace(*v)) {
+ end = s - input;
+ break;
+ }
3: 796e47c1e5f ! 3: e3a7b150241 trailer: use offsets for trailer_start/trailer_end
@@ Commit message
more explicit about these offsets (that they are for the entire trailer
block including other trailers). Ditto for trailer_end.
+ Reported-by: Glen Choo <glencbz@gmail.com>
Signed-off-by: Linus Arver <linusa@google.com>
## sequencer.c ##
@@ trailer.h: int trailer_set_if_missing(enum trailer_if_missing *item, const char
- * input string.
+ * 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, per find_true_end_of_input().
-+ *
-+ * NOTE: This will be changed so that these point to 0 in the next
-+ * patch if no trailers are found.
++ * "true" end of the input (find_end_of_log_message()).
*/
- const char *trailer_start, *trailer_end;
+ size_t trailer_block_start, trailer_block_end;
4: 64e1bd4e4be < -: ----------- trailer: only use trailer_block_* variables if trailers were found
--
gitgitgadget
next prev parent reply other threads:[~2023-10-20 19:01 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-05 5:04 [PATCH 0/5] Trailer readability cleanups Linus Arver via GitGitGadget
2023-08-05 5:04 ` [PATCH 1/5] trailer: separate public from internal portion of trailer_iterator Linus Arver via GitGitGadget
2023-08-07 21:16 ` Glen Choo
2023-08-08 12:19 ` Phillip Wood
2023-08-10 23:15 ` Linus Arver
2023-08-10 22:50 ` Linus Arver
2023-08-05 5:04 ` [PATCH 2/5] trailer: split process_input_file into separate pieces Linus Arver via GitGitGadget
2023-08-07 22:39 ` Glen Choo
2023-08-11 0:41 ` Linus Arver
2023-08-05 5:04 ` [PATCH 3/5] trailer: split process_command_line_args into separate functions Linus Arver via GitGitGadget
2023-08-07 22:51 ` Glen Choo
2023-08-11 0:59 ` Linus Arver
2023-08-11 1:02 ` Linus Arver
2023-08-11 21:11 ` Glen Choo
2023-08-05 5:04 ` [PATCH 4/5] trailer: teach find_patch_start about --no-divider Linus Arver via GitGitGadget
2023-08-07 23:28 ` Glen Choo
2023-08-11 1:25 ` Linus Arver
2023-08-11 20:51 ` Glen Choo
2023-08-05 5:04 ` [PATCH 5/5] trailer: rename *_DEFAULT enums to *_UNSPECIFIED Linus Arver via GitGitGadget
2023-08-07 23:45 ` Glen Choo
2023-08-11 18:00 ` Linus Arver
2023-09-09 6:16 ` [PATCH v2 0/6] Trailer readability cleanups Linus Arver via GitGitGadget
2023-09-09 6:16 ` [PATCH v2 1/6] trailer: separate public from internal portion of trailer_iterator Linus Arver via GitGitGadget
2023-09-11 17:10 ` Junio C Hamano
2023-09-09 6:16 ` [PATCH v2 2/6] trailer: split process_input_file into separate pieces Linus Arver via GitGitGadget
2023-09-11 17:10 ` Junio C Hamano
2023-09-09 6:16 ` [PATCH v2 3/6] trailer: split process_command_line_args into separate functions Linus Arver via GitGitGadget
2023-09-09 6:16 ` [PATCH v2 4/6] trailer: teach find_patch_start about --no-divider Linus Arver via GitGitGadget
2023-09-11 17:25 ` Junio C Hamano
2023-09-14 2:19 ` Linus Arver
2023-09-14 3:12 ` Junio C Hamano
2023-09-14 5:31 ` Linus Arver
2023-09-09 6:16 ` [PATCH v2 5/6] trailer: rename *_DEFAULT enums to *_UNSPECIFIED Linus Arver via GitGitGadget
2023-09-11 18:54 ` Junio C Hamano
2023-09-14 2:41 ` Linus Arver
2023-09-14 3:16 ` Junio C Hamano
2023-09-22 18:23 ` Linus Arver
2023-09-22 19:48 ` Junio C Hamano
2023-09-26 5:30 ` Linus Arver
2023-09-09 6:16 ` [PATCH v2 6/6] trailer: use offsets for trailer_start/trailer_end Linus Arver via GitGitGadget
2023-09-11 19:01 ` Junio C Hamano
2023-09-14 1:21 ` Linus Arver
2023-09-14 3:18 ` Linus Arver
2023-09-22 19:50 ` [PATCH v3 0/9] Trailer readability cleanups Linus Arver via GitGitGadget
2023-09-22 19:50 ` [PATCH v3 1/9] trailer: separate public from internal portion of trailer_iterator Linus Arver via GitGitGadget
2023-09-22 19:50 ` [PATCH v3 2/9] trailer: split process_input_file into separate pieces Linus Arver via GitGitGadget
2023-09-22 19:50 ` [PATCH v3 3/9] trailer: split process_command_line_args into separate functions Linus Arver via GitGitGadget
2023-09-22 19:50 ` [PATCH v3 4/9] trailer: rename *_DEFAULT enums to *_UNSPECIFIED Linus Arver via GitGitGadget
2023-09-22 19:50 ` [PATCH v3 5/9] commit: ignore_non_trailer computes number of bytes to ignore Linus Arver via GitGitGadget
2023-09-22 19:50 ` [PATCH v3 6/9] trailer: find the end of the log message Linus Arver via GitGitGadget
2023-09-22 19:50 ` [PATCH v3 7/9] trailer: use offsets for trailer_start/trailer_end Linus Arver via GitGitGadget
2023-09-22 19:50 ` [PATCH v3 8/9] trailer: only use trailer_block_* variables if trailers were found Linus Arver via GitGitGadget
2023-09-22 19:50 ` [PATCH v3 9/9] trailer: make stack variable names match field names Linus Arver via GitGitGadget
2023-09-22 22:47 ` [PATCH v3 0/9] Trailer readability cleanups Junio C Hamano
2023-09-22 23:13 ` Linus Arver
2023-09-23 0:48 ` Junio C Hamano
2023-09-26 5:40 ` Linus Arver
2023-09-26 6:22 ` [PATCH v4 0/4] " Linus Arver via GitGitGadget
2023-09-26 6:22 ` [PATCH v4 1/4] commit: ignore_non_trailer computes number of bytes to ignore Linus Arver via GitGitGadget
2023-09-26 6:22 ` [PATCH v4 2/4] trailer: find the end of the log message Linus Arver via GitGitGadget
2023-09-28 23:16 ` Jonathan Tan
2023-10-20 0:24 ` Linus Arver
2023-10-20 0:36 ` Junio C Hamano
2023-09-26 6:22 ` [PATCH v4 3/4] trailer: use offsets for trailer_start/trailer_end Linus Arver via GitGitGadget
2023-09-26 6:22 ` [PATCH v4 4/4] trailer: only use trailer_block_* variables if trailers were found Linus Arver via GitGitGadget
2023-10-20 19:01 ` Linus Arver via GitGitGadget [this message]
2023-10-20 19:01 ` [PATCH v5 1/3] commit: ignore_non_trailer computes number of bytes to ignore Linus Arver via GitGitGadget
2023-10-20 19:01 ` [PATCH v5 2/3] trailer: find the end of the log message Linus Arver via GitGitGadget
2023-10-20 21:29 ` Junio C Hamano
2023-12-29 6:42 ` Linus Arver
2023-12-29 21:03 ` Linus Arver
2023-10-20 19:01 ` [PATCH v5 3/3] trailer: use offsets for trailer_start/trailer_end Linus Arver via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1563.v5.git.1697828495.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=chriscool@tuxfamily.org \
--cc=git@vger.kernel.org \
--cc=glencbz@gmail.com \
--cc=jonathantanmy@google.com \
--cc=linusa@google.com \
--cc=phillip.wood123@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).