From 1e6ed5441a61b5085978e0429691e2e2425f6846 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 9 Dec 2019 05:10:39 -0800 Subject: notes: rename to load_display_notes() According to the function comment, init_display_notes() was supposed to "Load the notes machinery for displaying several notes trees." Rename this function to load_display_notes() so that its use is more accurately represented. This is done because, in a future commit, we will reuse the name init_display_notes(). Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- notes.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'notes.h') diff --git a/notes.h b/notes.h index 414bc6855a..1ce528442a 100644 --- a/notes.h +++ b/notes.h @@ -272,18 +272,18 @@ struct display_notes_opt { * - extra_notes_refs may contain a list of globs (in the same style * as notes.displayRef) where notes should be loaded from. */ -void init_display_notes(struct display_notes_opt *opt); +void load_display_notes(struct display_notes_opt *opt); /* * Append notes for the given 'object_sha1' from all trees set up by - * init_display_notes() to 'sb'. The 'flags' are a bitwise + * load_display_notes() to 'sb'. The 'flags' are a bitwise * combination of * * - NOTES_SHOW_HEADER: add a 'Notes (refname):' header * * - NOTES_INDENT: indent the notes by 4 places * - * You *must* call init_display_notes() before using this function. + * You *must* call load_display_notes() before using this function. */ void format_display_notes(const struct object_id *object_oid, struct strbuf *sb, const char *output_encoding, int raw); -- cgit v1.2.3 From e6e230eeae0f3cb46c4c356e6cd0a0f1119a2a83 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 9 Dec 2019 05:10:41 -0800 Subject: notes: create init_display_notes() helper We currently open code the initialization for revs->notes_opt. Abstract this away into a helper function so that the logic can be reused in a future commit. This is slightly wasteful as we memset the struct twice but this is only run once so it shouldn't have any major effect. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- notes.h | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'notes.h') diff --git a/notes.h b/notes.h index 1ce528442a..c0b712371c 100644 --- a/notes.h +++ b/notes.h @@ -260,6 +260,11 @@ struct display_notes_opt { struct string_list extra_notes_refs; }; +/* + * Initialize a display_notes_opt to its default value. + */ +void init_display_notes(struct display_notes_opt *opt); + /* * Load the notes machinery for displaying several notes trees. * -- cgit v1.2.3 From 452538c3586a76939faf43019fb7c21b3147309b Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 9 Dec 2019 05:10:44 -0800 Subject: notes: extract logic into set_display_notes() Instead of open coding the logic that tweaks the variables in `struct display_notes_opt` within handle_revision_opt(), abstract away the logic into set_display_notes() so that it can be reused. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- notes.h | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'notes.h') diff --git a/notes.h b/notes.h index c0b712371c..a476bfa066 100644 --- a/notes.h +++ b/notes.h @@ -265,6 +265,16 @@ struct display_notes_opt { */ void init_display_notes(struct display_notes_opt *opt); +/* + * Set a display_notes_opt to a given state. 'show_notes' is a boolean + * representing whether or not to show notes. 'opt_ref' points to a + * string for the notes ref, or is NULL if the default notes should be + * used. + * + * Return 'show_notes' normalized to 1 or 0. + */ +int set_display_notes(struct display_notes_opt *opt, int show_notes, const char *opt_ref); + /* * Load the notes machinery for displaying several notes trees. * -- cgit v1.2.3 From 1d7297513df66873e68af4b254804151b8ba5359 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 11 Dec 2019 16:49:50 -0800 Subject: 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 --- notes.h | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 'notes.h') diff --git a/notes.h b/notes.h index a476bfa066..3e78448181 100644 --- a/notes.h +++ b/notes.h @@ -266,14 +266,19 @@ struct display_notes_opt { void init_display_notes(struct display_notes_opt *opt); /* - * Set a display_notes_opt to a given state. 'show_notes' is a boolean - * representing whether or not to show notes. 'opt_ref' points to a - * string for the notes ref, or is NULL if the default notes should be - * used. - * - * Return 'show_notes' normalized to 1 or 0. + * This family of functions enables or disables the display of notes. In + * particular, 'enable_default_display_notes' will display the default notes, + * 'enable_default_display_notes' will display the notes ref 'ref' and + * 'disable_display_notes' will disable notes, including those added by previous + * invocations of the 'enable_*_display_notes' functions. + * + * 'show_notes' is a points to a boolean which will be set to 1 if notes are + * displayed, else 0. It must not be NULL. */ -int set_display_notes(struct display_notes_opt *opt, int show_notes, const char *opt_ref); +void enable_default_display_notes(struct display_notes_opt *opt, int *show_notes); +void enable_ref_display_notes(struct display_notes_opt *opt, int *show_notes, + const char *ref); +void disable_display_notes(struct display_notes_opt *opt, int *show_notes); /* * Load the notes machinery for displaying several notes trees. -- cgit v1.2.3 From e0f9095aaa56f5c731faced2e61ca48df5caedfb Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 18 Dec 2019 10:17:43 -0800 Subject: notes.h: fix typos in comment In 1d7297513d (notes: break set_display_notes() into smaller functions, 2019-12-11), we introduced a comment which had a couple of typos. In the first typo, we referenced 'enable_default_display_notes' instead of 'enable_ref_display_notes'. In the second typo, we wrote "is a points to" instead of "is a pointer to". Correct both of these typos. Reported-by: Eric Sunshine Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- notes.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'notes.h') diff --git a/notes.h b/notes.h index 3e78448181..bbab1961ca 100644 --- a/notes.h +++ b/notes.h @@ -268,11 +268,11 @@ void init_display_notes(struct display_notes_opt *opt); /* * This family of functions enables or disables the display of notes. In * particular, 'enable_default_display_notes' will display the default notes, - * 'enable_default_display_notes' will display the notes ref 'ref' and + * 'enable_ref_display_notes' will display the notes ref 'ref' and * 'disable_display_notes' will disable notes, including those added by previous * invocations of the 'enable_*_display_notes' functions. * - * 'show_notes' is a points to a boolean which will be set to 1 if notes are + * 'show_notes' is a pointer to a boolean which will be set to 1 if notes are * displayed, else 0. It must not be NULL. */ void enable_default_display_notes(struct display_notes_opt *opt, int *show_notes); -- cgit v1.2.3