| Message ID | 20230512171429.2202982-1-calvinwan@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | strbuf cleanups | expand |
Calvin Wan <calvinwan@google.com> writes: > Reroll with last couple of nits fixed. Thanks again to everyone for > reviewing this series! > > Calvin Wan (7): > strbuf: clarify API boundary > abspath: move related functions to abspath > credential-store: move related functions to credential-store file > object-name: move related functions to object-name > path: move related function to path > strbuf: clarify dependency > strbuf: remove global variable Looking good. I see Eric's comments, to which Phillip agreed to, are all addressed in these patches. Will replace; let's mark the topic for 'next'.
On Fri, May 12, 2023 at 4:24 PM Junio C Hamano <gitster@pobox.com> wrote: > Calvin Wan <calvinwan@google.com> writes: > > Reroll with last couple of nits fixed. Thanks again to everyone for > > reviewing this series! > > Looking good. I see Eric's comments, to which Phillip agreed to, > are all addressed in these patches. Agreed. This version is looking good; I have no further comments. Thanks for taking my comments into consideration.
Reroll with last couple of nits fixed. Thanks again to everyone for reviewing this series! Calvin Wan (7): strbuf: clarify API boundary abspath: move related functions to abspath credential-store: move related functions to credential-store file object-name: move related functions to object-name path: move related function to path strbuf: clarify dependency strbuf: remove global variable abspath.c | 36 ++++++++++++ abspath.h | 21 +++++++ add-patch.c | 12 ++-- builtin/am.c | 2 +- builtin/branch.c | 4 +- builtin/commit.c | 2 +- builtin/credential-store.c | 19 +++++++ builtin/merge.c | 10 ++-- builtin/notes.c | 16 +++--- builtin/rebase.c | 2 +- builtin/stripspace.c | 6 +- builtin/tag.c | 9 ++- fmt-merge-msg.c | 9 ++- gpg-interface.c | 5 +- hook.c | 1 + object-name.c | 15 +++++ object-name.h | 9 +++ path.c | 20 +++++++ path.h | 5 ++ pretty.c | 1 + rebase-interactive.c | 15 ++--- sequencer.c | 24 +++++--- strbuf.c | 112 ++++--------------------------------- strbuf.h | 57 ++++++------------- tempfile.c | 1 + wt-status.c | 6 +- 26 files changed, 227 insertions(+), 192 deletions(-) Range-diff against v5: 1: 287e787c9c ! 1: 5ae531a1e2 strbuf: clarify API boundary @@ strbuf.h +/* + * NOTE FOR STRBUF DEVELOPERS + * -+ * The objects that this API interacts with should be limited to other -+ * primitives, however, there are older functions in here that interact -+ * with non-primitive objects which should eventually be moved out or -+ * refactored. ++ * strbuf is a low-level primitive; as such it should interact only ++ * with other low-level primitives. Do not introduce new functions ++ * which interact with higher-level APIs. + */ + struct string_list; 2: 5bd27cad7a = 2: 3bb2b9c01a abspath: move related functions to abspath 3: 08e3e40de6 = 3: ff91ca2fda credential-store: move related functions to credential-store file 4: 1a21d4fdd1 = 4: 10e61eb570 object-name: move related functions to object-name 5: 03b20f3384 = 5: c3d5db4e11 path: move related function to path 6: b55f4a39e1 = 6: 113d156195 strbuf: clarify dependency 7: 6ea36d7730 ! 7: 01c923160d strbuf: remove global variable @@ Commit message functions. Therefore, add an additional parameter for functions that use comment_line_char and refactor callers to pass it in instead. strbuf_stripspace() removes the skip_comments boolean and checks if - comment_line_char is a non-NULL character to determine whether to skip + comment_line_char is a non-NUL character to determine whether to skip comments or not. ## add-patch.c ## @@ strbuf.c: static size_t cleanup(char *line, size_t len) * - * Enable skip_comments to skip every line starting with comment - * character. -+ * Pass a non-NULL comment_line_char to skip every line starting -+ * with it ++ * Pass a non-NUL comment_line_char to skip every line starting ++ * with it. */ -void strbuf_stripspace(struct strbuf *sb, int skip_comments) +void strbuf_stripspace(struct strbuf *sb, char comment_line_char) @@ strbuf.c: void strbuf_stripspace(struct strbuf *sb, int skip_comments) len = eol ? eol - (sb->buf + i) + 1 : sb->len - i; - if (skip_comments && len && sb->buf[i] == comment_line_char) { -+ if (comment_line_char != '\0' && len && ++ if (comment_line_char && len && + sb->buf[i] == comment_line_char) { newlen = 0; continue; @@ strbuf.h: int strbuf_getcwd(struct strbuf *sb); /** - * Strip whitespace from a buffer. The second parameter controls if - * comments are considered contents to be removed or not. -+ * Strip whitespace from a buffer. The second parameter is the comment -+ * character that determines whether a line is a comment or not. If the -+ * second parameter is a non-NULL character, comments are considered -+ * contents to be removed. ++ * Strip whitespace from a buffer. If comment_line_char is non-NUL, ++ * then lines beginning with that character are considered comments, ++ * thus removed. */ -void strbuf_stripspace(struct strbuf *buf, int skip_comments); +void strbuf_stripspace(struct strbuf *buf, char comment_line_char);