Re: [PATCH 2/2] [GSOC] interpret-trailer: easy parse trailer value

"ZheNing Hu via GitGitGadget" [off-list ref] writes:

As I already said, we do not want to pretend that this is a
generally reusable helper function.  We should at least have a
comment to tell people to never add new callers to this function,
with explanation of the reason.

Junio C Hamano [off-list ref] 于2021年3月21日周日 上午5:06写道：

"ZheNing Hu via GitGitGadget" [off-list ref] writes:

quoted

diff --git a/commit.h b/commit.h
index 49c0f503964e..970a73ccd5be 100644
--- a/commit.h
+++ b/commit.h

@@ -371,4 +371,6 @@ int parse_buffer_signed_by_header(const char *buffer,
                                struct strbuf *signature,
                                const struct git_hash_algo *algop);

+const char *find_author_by_nickname(const char *name);
+
 #endif /* COMMIT_H */

As I already said, we do not want to pretend that this is a
generally reusable helper function.  We should at least have a
comment to tell people to never add new callers to this function,
with explanation of the reason.

Do you think this is appropriate?

@@ -370,5 +370,15 @@ int parse_buffer_signed_by_header(const char *buffer,
                                  struct strbuf *payload,
                                  struct strbuf *signature,
                                  const struct git_hash_algo *algop);
+/*
+ * Calling `find_author_by_nickname` to find the "author <email>" pair
+ * in the most recent commit which matches "--author=name".
+ *
+ * Note that `find_author_by_nickname` is not reusable, because it haven't
+ * reset flags for parsed objects. The only safe way to use

`find_author_by_nickname`
+ * (without rewriting the revision traversal machinery) is to spawn a
+ * subprocess and do find_author_by_nickname() in it.
+ */
+const char *find_author_by_nickname(const char *name);

Thanks.

ZheNing Hu [off-list ref] writes:

Telling people not to add any new caller is good, but everything
after "because" does not make sense to me.

I do not think calling find_author_by_nickname() in a subprocess
alone would not help somebody who wants to do this, either.  We'd be
doing a moral equivalent of that call, but the result has to be
communicated back to the parent process,

In the longer term, we'd probably want to have a pre-computed table
of contributors, like we have precomputed files for reachability
bitmaps, commit DAG topology, and such, but that is obviously far
outside of the scope of this series.

+const char *find_author_by_nickname(const char *name);

Junio C Hamano [off-list ref] 于2021年3月21日周日 下午9:57写道：

ZheNing Hu [off-list ref] writes:

quoted

Do you think this is appropriate?

@@ -370,5 +370,15 @@ int parse_buffer_signed_by_header(const char *buffer,
                                  struct strbuf *payload,
                                  struct strbuf *signature,
                                  const struct git_hash_algo *algop);
+/*
+ * Calling `find_author_by_nickname` to find the "author <email>" pair
+ * in the most recent commit which matches "--author=name".
+ *
+ * Note that `find_author_by_nickname` is not reusable, because it haven't
+ * reset flags for parsed objects. The only safe way to use

`find_author_by_nickname`
+ * (without rewriting the revision traversal machinery) is to spawn a
+ * subprocess and do find_author_by_nickname() in it.
+ */

Telling people not to add any new caller is good, but everything
after "because" does not make sense to me.

I do not think calling find_author_by_nickname() in a subprocess
alone would not help somebody who wants to do this, either.  We'd be
doing a moral equivalent of that call, but the result has to be
communicated back to the parent process,

What I am thinking about here is that `commit --trailer` itself jumps to a
sub-process to do this, but this does depend on the fact that
`interpret-trailers`
 itself does not have a traversal, and indeed should not be arbitrarily call it.

In the longer term, we'd probably want to have a pre-computed table
of contributors, like we have precomputed files for reachability
bitmaps, commit DAG topology, and such, but that is obviously far
outside of the scope of this series.

Indeed this will be a very big project. But `.mailmap` always makes me
feel similar.

quoted

+const char *find_author_by_nickname(const char *name);