path: root/builtin/log.c
diff options
authorDenton Liu <>2019-12-12 00:49:50 (GMT)
committerJunio C Hamano <>2019-12-13 19:07:15 (GMT)
commit1d7297513df66873e68af4b254804151b8ba5359 (patch)
treeb13311d7572e5488bef01283449d2ce06d0323d0 /builtin/log.c
parent66f79ee23d51ac11784aeb0ef4f4119af9fbb984 (diff)
notes: break set_display_notes() into smaller functions
In 8164c961e1 (format-patch: use --notes behavior for format.notes, 2019-12-09), we introduced set_display_notes() which was a monolithic function with three mutually exclusive branches. Break the function up into three small and simple functions that each are only responsible for one task. This family of functions accepts an `int *show_notes` instead of returning a value suitable for assignment to `show_notes`. This is for two reasons. First of all, this guarantees that the external `show_notes` variable changes in lockstep with the `struct display_notes_opt`. Second, this prompts future developers to be careful about doing something meaningful with this value. In fact, a NULL check is intentionally omitted because causing a segfault here would tell the future developer that they are meant to use the value for something meaningful. One alternative was making the family of functions accept a `struct rev_info *` instead of the `struct display_notes_opt *`, since the former contains the `show_notes` field as well. This does not work because we have to call git_config() before repo_init_revisions(). However, if we had a `struct rev_info`, we'd need to initialize it before it gets assigned values from git_config(). As a result, we break the circular dependency by having standalone `int show_notes` and `struct display_notes_opt notes_opt` variables which temporarily hold values from git_config() until the values are copied over to `rev`. To implement this change, we need to get a pointer to `rev_info::show_notes`. Unfortunately, this is not possible with bitfields and only direct-assignment is possible. Change `rev_info::show_notes` to a non-bitfield int so that we can get its address. Signed-off-by: Denton Liu <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'builtin/log.c')
1 files changed, 6 insertions, 1 deletions
diff --git a/builtin/log.c b/builtin/log.c
index 4225615..b6d43a4 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -868,7 +868,12 @@ static int git_format_config(const char *var, const char *value, void *cb)
if (!strcmp(var, "format.notes")) {
int b = git_parse_maybe_bool(value);
- show_notes = set_display_notes(&notes_opt, b, b < 0 ? value : NULL);
+ if (b < 0)
+ enable_ref_display_notes(&notes_opt, &show_notes, value);
+ else if (b)
+ enable_default_display_notes(&notes_opt, &show_notes);
+ else
+ disable_display_notes(&notes_opt, &show_notes);
return 0;