Re: [PATCH v5 1/4] revision: improve commit_rewrite_person()
From: Junio C Hamano <hidden>
Date: 2022-07-17 22:11:37
Siddharth Asthana [off-list ref] writes:
+/* + * Returns the difference between the new and old length of the ident line. + */ +static ssize_t rewrite_ident_line(const char *person, struct strbuf *buf, + struct string_list *mailmap)
Line-folding is a good idea, but why do we use such a deep indentation? In this project, tab-width is 8.
+static void commit_rewrite_person(struct strbuf *buf, const char **header, + struct string_list *mailmap)
Likewise.
+{
+ size_t buf_offset = 0;
+
+ if (!mailmap)
+ return;
+
+ for (;;) {
+ const char *person, *line;
+ size_t i;
+
+ line = buf->buf + buf_offset;
+ if (!*line || *line == '\n')
+ return; /* End of header */
+
+ for (i = 0; header[i]; i++)
+ if (skip_prefix(line, header[i], &person)) {
+ rewrite_ident_line(person, buf, mailmap);If the return value of rewrite_ident_line() is never used, perhaps stop computing the return value in that function and make it return "void". I personally thought it was clever to return "how much does the ident part grew/shrunk?" from the helper and use it to adjust, but I do not mind to scrap the clever-ness if some folks may find it harder to understand.
+ break; + } + + buf_offset = strchrnul(buf->buf + buf_offset, '\n') - buf->buf;
And this is a "easier-to-understand but need to scan the buffer once again, only to figure out what we ought to already know" version.
+ if (buf->buf[buf_offset] == '\n') + ++buf_offset;
Prefer post-increment when there is no reason to favor pre-increment over post-increment, i.e. write this as "buf_offset++".
quoted hunk
+ } +} + static int commit_match(struct commit *commit, struct rev_info *opt) { int retval;@@ -3832,11 +3862,12 @@ static int commit_match(struct commit *commit, struct rev_info *opt) strbuf_addstr(&buf, message); if (opt->grep_filter.header_list && opt->mailmap) { + const char *commit_headers[] = { "author ", "committer ", NULL }; + if (!buf.len) strbuf_addstr(&buf, message); - commit_rewrite_person(&buf, "\nauthor ", opt->mailmap); - commit_rewrite_person(&buf, "\ncommitter ", opt->mailmap); + commit_rewrite_person(&buf, commit_headers, opt->mailmap); } /* Append "fake" message parts as needed */