Junio C Hamano <gitster@xxxxxxxxx> 于2021年3月20日周六 下午2:53写道： > > ZheNing Hu <adlternative@xxxxxxxxx> writes: > > > In actually , > > > > @@ -1071,6 +1071,7 @@ static const char *find_author_by_nickname(const > > char *name) > > strbuf_release(&buf); > > format_commit_message(commit, "%aN <%aE>", &buf, &ctx); > > clear_mailmap(&mailmap); > > + reset_revision_walk(); > > return strbuf_detach(&buf, NULL); > > } > > > > then we can reuse this function. > > > > But I think I can give find_author_by_nickname another arg for choice if we want > > `reset_revision_walk()`. > > That is half fixing and half breaking. It would allow us to call > the helper number of times as long as there is no other revision > traversal is in progress; calling reset_revision_walk() would mean > any and all revision traversal in progress will be broken. So we > cannot use the helper to tweak each and every commit we encounter > while running "git log", for example. Imagine adding an option to > "git format-patch" to allow each commit it formats to be tweaked by > adding "--trailers=foo:@ZheNing" and the like. There is one primary > traversal that is used to list which commit to format in what order, > and every time that primary traversal yields a commit, if we run > find_author_by_nickname(), we end up initiating another traversal, > and then by calling reset, we clear all object flags that are used > for revision traversal, thereby breaking the primary traversal. > I understand what you meaning. I may not be very thorough in considering the potential problems. This is as dangerous as calling `list_del` in the middle of `list_for_each`. > The only safe way to introduce a generally usable helper (without > rewriting the revision traversal machinery) is to spawn a subprocess > and do an equivalent of find_author_by_nickname() in it. > If I didn't notice what you mentioned later, I might think about it for a long time. Execute a child process specifically to execute `find_author_by_nickname()`, which feels a bit wasteful. > A standalone "interpret-trailers" command, as long as it won't do > any other revision traversal, would not have such a problem, and > calling reset every time find_author_by_nickname() is called may be > sufficient. The only thing I care about is *not* to pretend that > find_author_by_nickname() plus reset() is the generally reusable > helper function and advertise it as such, which will mislead future > developers into misusing the function in a context they shouldn't > (i.e. while they are performing their own revision traversal). > > Thanks. After listening to your previous explanation, I agree with this view. `interpret-trailers` alone as a process, as long as it does not traverse the revision externally, It is safe for us to call `reset_revision_walk()` after `find_author_by_nickname()` inside interpret-trailers. Thank you for your thoughts and answers.
