| Message ID | 99c88c74ceab751c0540b51d98bf339bffc098ef.1685904424.git.code@khaugsbakk.name (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | notes: update documentation for `use_default_notes` | expand |
On Sun, Jun 04, 2023 at 08:54:00PM +0200, Kristoffer Haugsbakk wrote: > Its better to document the struct members on the struct definition > instead of on a function that takes a pointer to such a struct. This > will also make it easier to update the documentation in the future. This is better, I think, but... > +/* > + * - use_default_notes: less than `0` is "unset", which means that the > + * default notes are shown iff no other notes are given. Otherwise, > + * treat it like a boolean. > + * > + * - extra_notes_refs may contain a list of globs (in the same style > + * as notes.displayRef) where notes should be loaded from. > + */ > struct display_notes_opt { > int use_default_notes; > struct string_list extra_notes_refs; ...what I had meant to suggest was putting the documentation next to each field, like: struct foo { /* when set, enable "bar" for all frotz's */ int use_bar; /* etc... */ }; For an example, try "struct bloom_filter_settings" in bloom.h (though there are many others, too). -Peff > @@ -283,15 +291,7 @@ void disable_display_notes(struct display_notes_opt *opt, int *show_notes); > /* > * Load the notes machinery for displaying several notes trees. > * > - * If 'opt' is not NULL, then it specifies additional settings for the > - * displaying: > - * > - * - use_default_notes: less than `0` is "unset", which means that the > - * default notes are shown iff no other notes are given. Otherwise, > - * treat it like a boolean. > - * > - * - extra_notes_refs may contain a list of globs (in the same style > - * as notes.displayRef) where notes should be loaded from. > + * 'opt' may be NULL. > */ > void load_display_notes(struct display_notes_opt *opt); > > -- > 2.41.0 >
On Mon, Jun 5, 2023, at 06:50, Jeff King wrote: > This is better, I think, but... > > [snip] > > ...what I had meant to suggest was putting the documentation next to > each field, like: That make even more sense. :)
diff --git a/notes.h b/notes.h index a823840e49..c63b275d7a 100644 --- a/notes.h +++ b/notes.h @@ -255,6 +255,14 @@ void free_notes(struct notes_tree *t); struct string_list; +/* + * - use_default_notes: less than `0` is "unset", which means that the + * default notes are shown iff no other notes are given. Otherwise, + * treat it like a boolean. + * + * - extra_notes_refs may contain a list of globs (in the same style + * as notes.displayRef) where notes should be loaded from. + */ struct display_notes_opt { int use_default_notes; struct string_list extra_notes_refs; @@ -283,15 +291,7 @@ void disable_display_notes(struct display_notes_opt *opt, int *show_notes); /* * Load the notes machinery for displaying several notes trees. * - * If 'opt' is not NULL, then it specifies additional settings for the - * displaying: - * - * - use_default_notes: less than `0` is "unset", which means that the - * default notes are shown iff no other notes are given. Otherwise, - * treat it like a boolean. - * - * - extra_notes_refs may contain a list of globs (in the same style - * as notes.displayRef) where notes should be loaded from. + * 'opt' may be NULL. */ void load_display_notes(struct display_notes_opt *opt);
Its better to document the struct members on the struct definition instead of on a function that takes a pointer to such a struct. This will also make it easier to update the documentation in the future. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> --- Notes (series): Suggested in: https://lore.kernel.org/git/20230601175218.GB4165405@coredump.intra.peff.net/ notes.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)