From d0ca92a7fe205195850f9d4b8cf774c0687c3789 Mon Sep 17 00:00:00 2001 From: Calvin Wan Date: Fri, 12 May 2023 17:15:10 +0000 Subject: [PATCH 1/7] strbuf: clarify API boundary strbuf, as a generic and widely used structure across the codebase, should be limited as a library to only interact with primitives. Add documentation so future functions can appropriately be placed. Older functions that do not follow this boundary should eventually be moved or refactored. Signed-off-by: Calvin Wan Signed-off-by: Junio C Hamano --- strbuf.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/strbuf.h b/strbuf.h index 3dfeadb44c2..70778c6e105 100644 --- a/strbuf.h +++ b/strbuf.h @@ -1,6 +1,14 @@ #ifndef STRBUF_H #define STRBUF_H +/* + * NOTE FOR STRBUF DEVELOPERS + * + * 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; /** From 689ece6602634742f4d21d2a96abf0a2801963dd Mon Sep 17 00:00:00 2001 From: Calvin Wan Date: Fri, 12 May 2023 17:15:11 +0000 Subject: [PATCH 2/7] abspath: move related functions to abspath Move abspath-related functions from strbuf.[ch] to abspath.[ch] so that strbuf is focused on string manipulation routines with minimal dependencies. Signed-off-by: Calvin Wan Signed-off-by: Junio C Hamano --- abspath.c | 36 ++++++++++++++++++++++++++++++++++++ abspath.h | 21 +++++++++++++++++++++ hook.c | 1 + strbuf.c | 37 ------------------------------------- strbuf.h | 22 ---------------------- tempfile.c | 1 + 6 files changed, 59 insertions(+), 59 deletions(-) diff --git a/abspath.c b/abspath.c index d032f5dce51..1202cde23db 100644 --- a/abspath.c +++ b/abspath.c @@ -289,3 +289,39 @@ char *prefix_filename_except_for_dash(const char *pfx, const char *arg) return xstrdup(arg); return prefix_filename(pfx, arg); } + +void strbuf_add_absolute_path(struct strbuf *sb, const char *path) +{ + if (!*path) + die("The empty string is not a valid path"); + if (!is_absolute_path(path)) { + struct stat cwd_stat, pwd_stat; + size_t orig_len = sb->len; + char *cwd = xgetcwd(); + char *pwd = getenv("PWD"); + if (pwd && strcmp(pwd, cwd) && + !stat(cwd, &cwd_stat) && + (cwd_stat.st_dev || cwd_stat.st_ino) && + !stat(pwd, &pwd_stat) && + pwd_stat.st_dev == cwd_stat.st_dev && + pwd_stat.st_ino == cwd_stat.st_ino) + strbuf_addstr(sb, pwd); + else + strbuf_addstr(sb, cwd); + if (sb->len > orig_len && !is_dir_sep(sb->buf[sb->len - 1])) + strbuf_addch(sb, '/'); + free(cwd); + } + strbuf_addstr(sb, path); +} + +void strbuf_add_real_path(struct strbuf *sb, const char *path) +{ + if (sb->len) { + struct strbuf resolved = STRBUF_INIT; + strbuf_realpath(&resolved, path, 1); + strbuf_addbuf(sb, &resolved); + strbuf_release(&resolved); + } else + strbuf_realpath(sb, path, 1); +} diff --git a/abspath.h b/abspath.h index 7cd3de5e9de..4653080d5e4 100644 --- a/abspath.h +++ b/abspath.h @@ -30,4 +30,25 @@ static inline int is_absolute_path(const char *path) return is_dir_sep(path[0]) || has_dos_drive_prefix(path); } +/** + * Add a path to a buffer, converting a relative path to an + * absolute one in the process. Symbolic links are not + * resolved. + */ +void strbuf_add_absolute_path(struct strbuf *sb, const char *path); + +/** + * Canonize `path` (make it absolute, resolve symlinks, remove extra + * slashes) and append it to `sb`. Die with an informative error + * message if there is a problem. + * + * The directory part of `path` (i.e., everything up to the last + * dir_sep) must denote a valid, existing directory, but the last + * component need not exist. + * + * Callers that don't mind links should use the more lightweight + * strbuf_add_absolute_path() instead. + */ +void strbuf_add_real_path(struct strbuf *sb, const char *path); + #endif /* ABSPATH_H */ diff --git a/hook.c b/hook.c index 76e322f5804..2d8706371eb 100644 --- a/hook.c +++ b/hook.c @@ -1,4 +1,5 @@ #include "git-compat-util.h" +#include "abspath.h" #include "advice.h" #include "gettext.h" #include "hook.h" diff --git a/strbuf.c b/strbuf.c index 729378ec824..c3b6d487976 100644 --- a/strbuf.c +++ b/strbuf.c @@ -1,5 +1,4 @@ #include "git-compat-util.h" -#include "abspath.h" #include "alloc.h" #include "environment.h" #include "gettext.h" @@ -899,42 +898,6 @@ void strbuf_humanise_rate(struct strbuf *buf, off_t bytes) strbuf_humanise(buf, bytes, 1); } -void strbuf_add_absolute_path(struct strbuf *sb, const char *path) -{ - if (!*path) - die("The empty string is not a valid path"); - if (!is_absolute_path(path)) { - struct stat cwd_stat, pwd_stat; - size_t orig_len = sb->len; - char *cwd = xgetcwd(); - char *pwd = getenv("PWD"); - if (pwd && strcmp(pwd, cwd) && - !stat(cwd, &cwd_stat) && - (cwd_stat.st_dev || cwd_stat.st_ino) && - !stat(pwd, &pwd_stat) && - pwd_stat.st_dev == cwd_stat.st_dev && - pwd_stat.st_ino == cwd_stat.st_ino) - strbuf_addstr(sb, pwd); - else - strbuf_addstr(sb, cwd); - if (sb->len > orig_len && !is_dir_sep(sb->buf[sb->len - 1])) - strbuf_addch(sb, '/'); - free(cwd); - } - strbuf_addstr(sb, path); -} - -void strbuf_add_real_path(struct strbuf *sb, const char *path) -{ - if (sb->len) { - struct strbuf resolved = STRBUF_INIT; - strbuf_realpath(&resolved, path, 1); - strbuf_addbuf(sb, &resolved); - strbuf_release(&resolved); - } else - strbuf_realpath(sb, path, 1); -} - int printf_ln(const char *fmt, ...) { int ret; diff --git a/strbuf.h b/strbuf.h index 70778c6e105..207efb4f98c 100644 --- a/strbuf.h +++ b/strbuf.h @@ -535,28 +535,6 @@ int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term); */ int strbuf_getcwd(struct strbuf *sb); -/** - * Add a path to a buffer, converting a relative path to an - * absolute one in the process. Symbolic links are not - * resolved. - */ -void strbuf_add_absolute_path(struct strbuf *sb, const char *path); - -/** - * Canonize `path` (make it absolute, resolve symlinks, remove extra - * slashes) and append it to `sb`. Die with an informative error - * message if there is a problem. - * - * The directory part of `path` (i.e., everything up to the last - * dir_sep) must denote a valid, existing directory, but the last - * component need not exist. - * - * Callers that don't mind links should use the more lightweight - * strbuf_add_absolute_path() instead. - */ -void strbuf_add_real_path(struct strbuf *sb, const char *path); - - /** * Normalize in-place the path contained in the strbuf. See * normalize_path_copy() for details. If an error occurs, the contents of "sb" diff --git a/tempfile.c b/tempfile.c index 50c377134ce..6c88a63b422 100644 --- a/tempfile.c +++ b/tempfile.c @@ -43,6 +43,7 @@ */ #include "git-compat-util.h" +#include "abspath.h" #include "path.h" #include "tempfile.h" #include "sigchain.h" From eaec86fb92f9ce18d0072329d8de8d4410755cb1 Mon Sep 17 00:00:00 2001 From: Calvin Wan Date: Fri, 12 May 2023 17:15:12 +0000 Subject: [PATCH 3/7] credential-store: move related functions to credential-store file is_rfc3986_unreserved() and is_rfc3986_reserved_or_unreserved() are only called from builtin/credential-store.c and they are only relevant to that file so move those functions and make them static. Signed-off-by: Calvin Wan Signed-off-by: Junio C Hamano --- builtin/credential-store.c | 19 +++++++++++++++++++ strbuf.c | 19 ------------------- strbuf.h | 3 --- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/builtin/credential-store.c b/builtin/credential-store.c index 8977604eb9d..47761183317 100644 --- a/builtin/credential-store.c +++ b/builtin/credential-store.c @@ -73,6 +73,25 @@ static void rewrite_credential_file(const char *fn, struct credential *c, die_errno("unable to write credential store"); } +static int is_rfc3986_unreserved(char ch) +{ + return isalnum(ch) || + ch == '-' || ch == '_' || ch == '.' || ch == '~'; +} + +static int is_rfc3986_reserved_or_unreserved(char ch) +{ + if (is_rfc3986_unreserved(ch)) + return 1; + switch (ch) { + case '!': case '*': case '\'': case '(': case ')': case ';': + case ':': case '@': case '&': case '=': case '+': case '$': + case ',': case '/': case '?': case '#': case '[': case ']': + return 1; + } + return 0; +} + static void store_credential_file(const char *fn, struct credential *c) { struct strbuf buf = STRBUF_INIT; diff --git a/strbuf.c b/strbuf.c index c3b6d487976..da2693b21f8 100644 --- a/strbuf.c +++ b/strbuf.c @@ -809,25 +809,6 @@ void strbuf_addstr_xml_quoted(struct strbuf *buf, const char *s) } } -int is_rfc3986_reserved_or_unreserved(char ch) -{ - if (is_rfc3986_unreserved(ch)) - return 1; - switch (ch) { - case '!': case '*': case '\'': case '(': case ')': case ';': - case ':': case '@': case '&': case '=': case '+': case '$': - case ',': case '/': case '?': case '#': case '[': case ']': - return 1; - } - return 0; -} - -int is_rfc3986_unreserved(char ch) -{ - return isalnum(ch) || - ch == '-' || ch == '_' || ch == '.' || ch == '~'; -} - static void strbuf_add_urlencode(struct strbuf *sb, const char *s, size_t len, char_predicate allow_unencoded_fn) { diff --git a/strbuf.h b/strbuf.h index 207efb4f98c..114ad0c0244 100644 --- a/strbuf.h +++ b/strbuf.h @@ -690,9 +690,6 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name); typedef int (*char_predicate)(char ch); -int is_rfc3986_unreserved(char ch); -int is_rfc3986_reserved_or_unreserved(char ch); - void strbuf_addstr_urlencode(struct strbuf *sb, const char *name, char_predicate allow_unencoded_fn); From d1272d146978b846bb195aea62753d96a2ccce8a Mon Sep 17 00:00:00 2001 From: Calvin Wan Date: Fri, 12 May 2023 17:15:13 +0000 Subject: [PATCH 4/7] object-name: move related functions to object-name Move object-name-related functions from strbuf.[ch] to object-name.[ch] so that strbuf is focused on string manipulation routines with minimal dependencies. Signed-off-by: Calvin Wan Signed-off-by: Junio C Hamano --- object-name.c | 15 +++++++++++++++ object-name.h | 9 +++++++++ pretty.c | 1 + strbuf.c | 16 ---------------- strbuf.h | 10 ---------- 5 files changed, 25 insertions(+), 26 deletions(-) diff --git a/object-name.c b/object-name.c index 538e8a8f62e..c2e82aceeaf 100644 --- a/object-name.c +++ b/object-name.c @@ -766,6 +766,21 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad) find_abbrev_len_for_pack(p, mad); } +void strbuf_repo_add_unique_abbrev(struct strbuf *sb, struct repository *repo, + const struct object_id *oid, int abbrev_len) +{ + int r; + strbuf_grow(sb, GIT_MAX_HEXSZ + 1); + r = repo_find_unique_abbrev_r(repo, sb->buf + sb->len, oid, abbrev_len); + strbuf_setlen(sb, sb->len + r); +} + +void strbuf_add_unique_abbrev(struct strbuf *sb, const struct object_id *oid, + int abbrev_len) +{ + strbuf_repo_add_unique_abbrev(sb, the_repository, oid, abbrev_len); +} + int repo_find_unique_abbrev_r(struct repository *r, char *hex, const struct object_id *oid, int len) { diff --git a/object-name.h b/object-name.h index 1d63698f42b..9ae52230714 100644 --- a/object-name.h +++ b/object-name.h @@ -40,6 +40,15 @@ struct object_context { const char *repo_find_unique_abbrev(struct repository *r, const struct object_id *oid, int len); int repo_find_unique_abbrev_r(struct repository *r, char *hex, const struct object_id *oid, int len); +/** + * Add the abbreviation, as generated by repo_find_unique_abbrev(), of `sha1` to + * the strbuf `sb`. + */ +void strbuf_repo_add_unique_abbrev(struct strbuf *sb, struct repository *repo, + const struct object_id *oid, int abbrev_len); +void strbuf_add_unique_abbrev(struct strbuf *sb, const struct object_id *oid, + int abbrev_len); + int repo_get_oid(struct repository *r, const char *str, struct object_id *oid); __attribute__((format (printf, 2, 3))) int get_oidf(struct object_id *oid, const char *fmt, ...); diff --git a/pretty.c b/pretty.c index 0bb938021ba..78bac2d8189 100644 --- a/pretty.c +++ b/pretty.c @@ -18,6 +18,7 @@ #include "gpg-interface.h" #include "trailer.h" #include "run-command.h" +#include "object-name.h" /* * The limit for formatting directives, which enable the caller to append diff --git a/strbuf.c b/strbuf.c index da2693b21f8..6533559e956 100644 --- a/strbuf.c +++ b/strbuf.c @@ -3,7 +3,6 @@ #include "environment.h" #include "gettext.h" #include "hex.h" -#include "object-name.h" #include "refs.h" #include "string-list.h" #include "utf8.h" @@ -1023,21 +1022,6 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm, strbuf_setlen(sb, sb->len + len); } -void strbuf_repo_add_unique_abbrev(struct strbuf *sb, struct repository *repo, - const struct object_id *oid, int abbrev_len) -{ - int r; - strbuf_grow(sb, GIT_MAX_HEXSZ + 1); - r = repo_find_unique_abbrev_r(repo, sb->buf + sb->len, oid, abbrev_len); - strbuf_setlen(sb, sb->len + r); -} - -void strbuf_add_unique_abbrev(struct strbuf *sb, const struct object_id *oid, - int abbrev_len) -{ - strbuf_repo_add_unique_abbrev(sb, the_repository, oid, abbrev_len); -} - /* * Returns the length of a line, without trailing spaces. * diff --git a/strbuf.h b/strbuf.h index 114ad0c0244..5d97e27b991 100644 --- a/strbuf.h +++ b/strbuf.h @@ -616,16 +616,6 @@ void strbuf_add_separated_string_list(struct strbuf *str, */ void strbuf_list_free(struct strbuf **list); -/** - * Add the abbreviation, as generated by repo_find_unique_abbrev(), of `sha1` to - * the strbuf `sb`. - */ -struct repository; -void strbuf_repo_add_unique_abbrev(struct strbuf *sb, struct repository *repo, - const struct object_id *oid, int abbrev_len); -void strbuf_add_unique_abbrev(struct strbuf *sb, const struct object_id *oid, - int abbrev_len); - /* * Remove the filename from the provided path string. If the path * contains a trailing separator, then the path is considered a directory From 9a5bb4efa0c8c76a0438f5a6d6fdc4df2667e366 Mon Sep 17 00:00:00 2001 From: Calvin Wan Date: Fri, 12 May 2023 17:15:14 +0000 Subject: [PATCH 5/7] path: move related function to path Move path-related function from strbuf.[ch] to path.[ch] so that strbuf is focused on string manipulation routines with minimal dependencies. Signed-off-by: Calvin Wan Signed-off-by: Junio C Hamano --- path.c | 20 ++++++++++++++++++++ path.h | 5 +++++ strbuf.c | 20 -------------------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/path.c b/path.c index 7c1cd8182a8..e17a2613c5f 100644 --- a/path.c +++ b/path.c @@ -1213,6 +1213,26 @@ int normalize_path_copy(char *dst, const char *src) return normalize_path_copy_len(dst, src, NULL); } +int strbuf_normalize_path(struct strbuf *src) +{ + struct strbuf dst = STRBUF_INIT; + + strbuf_grow(&dst, src->len); + if (normalize_path_copy(dst.buf, src->buf) < 0) { + strbuf_release(&dst); + return -1; + } + + /* + * normalize_path does not tell us the new length, so we have to + * compute it by looking for the new NUL it placed + */ + strbuf_setlen(&dst, strlen(dst.buf)); + strbuf_swap(src, &dst); + strbuf_release(&dst); + return 0; +} + /* * path = Canonical absolute path * prefixes = string_list containing normalized, absolute paths without diff --git a/path.h b/path.h index 60e83a49a98..639372edd9e 100644 --- a/path.h +++ b/path.h @@ -191,6 +191,11 @@ const char *remove_leading_path(const char *in, const char *prefix); const char *relative_path(const char *in, const char *prefix, struct strbuf *sb); int normalize_path_copy_len(char *dst, const char *src, int *prefix_len); int normalize_path_copy(char *dst, const char *src); +/** + * Normalize in-place the path contained in the strbuf. If an error occurs, + * the contents of "sb" are left untouched, and -1 is returned. + */ +int strbuf_normalize_path(struct strbuf *src); int longest_ancestor_length(const char *path, struct string_list *prefixes); char *strip_path_suffix(const char *path, const char *suffix); int daemon_avoid_alias(const char *path); diff --git a/strbuf.c b/strbuf.c index 6533559e956..178d75f250d 100644 --- a/strbuf.c +++ b/strbuf.c @@ -1088,26 +1088,6 @@ void strbuf_stripspace(struct strbuf *sb, int skip_comments) strbuf_setlen(sb, j); } -int strbuf_normalize_path(struct strbuf *src) -{ - struct strbuf dst = STRBUF_INIT; - - strbuf_grow(&dst, src->len); - if (normalize_path_copy(dst.buf, src->buf) < 0) { - strbuf_release(&dst); - return -1; - } - - /* - * normalize_path does not tell us the new length, so we have to - * compute it by looking for the new NUL it placed - */ - strbuf_setlen(&dst, strlen(dst.buf)); - strbuf_swap(src, &dst); - strbuf_release(&dst); - return 0; -} - void strbuf_strip_file_from_path(struct strbuf *sb) { char *path_sep = find_last_dir_sep(sb->buf); From 88da41d9ff10cdda01830778b620a95eafe5a9eb Mon Sep 17 00:00:00 2001 From: Calvin Wan Date: Fri, 12 May 2023 17:15:15 +0000 Subject: [PATCH 6/7] strbuf: clarify dependency refs.h was once needed but is no longer so as of 6bab74e7fb8 ("strbuf: move strbuf_branchname to sha1_name.c", 2010-11-06). strbuf.h was included thru refs.h, so removing refs.h requires strbuf.h to be added back. Signed-off-by: Calvin Wan Signed-off-by: Junio C Hamano --- strbuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/strbuf.c b/strbuf.c index 178d75f250d..d5978fee4e4 100644 --- a/strbuf.c +++ b/strbuf.c @@ -3,7 +3,7 @@ #include "environment.h" #include "gettext.h" #include "hex.h" -#include "refs.h" +#include "strbuf.h" #include "string-list.h" #include "utf8.h" #include "date.h" From 6897be8f14fb97e7ee4f9d5543af4320bb8039ee Mon Sep 17 00:00:00 2001 From: Calvin Wan Date: Fri, 12 May 2023 17:15:16 +0000 Subject: [PATCH 7/7] strbuf: remove global variable As a library that only interacts with other primitives, strbuf should not utilize the comment_line_char global variable within its 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-NUL character to determine whether to skip comments or not. Signed-off-by: Calvin Wan Signed-off-by: Junio C Hamano --- add-patch.c | 12 +++++++----- builtin/am.c | 2 +- builtin/branch.c | 4 ++-- builtin/commit.c | 2 +- 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 +++-- rebase-interactive.c | 15 ++++++++------- sequencer.c | 24 +++++++++++++++--------- strbuf.c | 18 ++++++++++-------- strbuf.h | 14 ++++++++------ wt-status.c | 6 +++--- 16 files changed, 90 insertions(+), 64 deletions(-) diff --git a/add-patch.c b/add-patch.c index 8d770d203ff..9702c1aabdb 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1105,10 +1105,11 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk) size_t i; strbuf_reset(&s->buf); - strbuf_commented_addf(&s->buf, _("Manual hunk edit mode -- see bottom for " - "a quick guide.\n")); + strbuf_commented_addf(&s->buf, comment_line_char, + _("Manual hunk edit mode -- see bottom for " + "a quick guide.\n")); render_hunk(s, hunk, 0, 0, &s->buf); - strbuf_commented_addf(&s->buf, + strbuf_commented_addf(&s->buf, comment_line_char, _("---\n" "To remove '%c' lines, make them ' ' lines " "(context).\n" @@ -1117,12 +1118,13 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk) s->mode->is_reverse ? '+' : '-', s->mode->is_reverse ? '-' : '+', comment_line_char); - strbuf_commented_addf(&s->buf, "%s", _(s->mode->edit_hunk_hint)); + strbuf_commented_addf(&s->buf, comment_line_char, "%s", + _(s->mode->edit_hunk_hint)); /* * TRANSLATORS: 'it' refers to the patch mentioned in the previous * messages. */ - strbuf_commented_addf(&s->buf, + strbuf_commented_addf(&s->buf, comment_line_char, _("If it does not apply cleanly, you will be " "given an opportunity to\n" "edit again. If all lines of the hunk are " diff --git a/builtin/am.c b/builtin/am.c index 5c83f2e003f..9ece2e30667 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1283,7 +1283,7 @@ static int parse_mail(struct am_state *state, const char *mail) strbuf_addstr(&msg, "\n\n"); strbuf_addbuf(&msg, &mi.log_message); - strbuf_stripspace(&msg, 0); + strbuf_stripspace(&msg, '\0'); assert(!state->author_name); state->author_name = strbuf_detach(&author_name, NULL); diff --git a/builtin/branch.c b/builtin/branch.c index 501c47657c9..4560610d09c 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -629,7 +629,7 @@ static int edit_branch_description(const char *branch_name) exists = !read_branch_desc(&buf, branch_name); if (!buf.len || buf.buf[buf.len-1] != '\n') strbuf_addch(&buf, '\n'); - strbuf_commented_addf(&buf, + strbuf_commented_addf(&buf, comment_line_char, _("Please edit the description for the branch\n" " %s\n" "Lines starting with '%c' will be stripped.\n"), @@ -640,7 +640,7 @@ static int edit_branch_description(const char *branch_name) strbuf_release(&buf); return -1; } - strbuf_stripspace(&buf, 1); + strbuf_stripspace(&buf, comment_line_char); strbuf_addf(&name, "branch.%s.description", branch_name); if (buf.len || exists) diff --git a/builtin/commit.c b/builtin/commit.c index e67c4be2211..e002ebf070f 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -893,7 +893,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, s->hints = 0; if (clean_message_contents) - strbuf_stripspace(&sb, 0); + strbuf_stripspace(&sb, '\0'); if (signoff) append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0); diff --git a/builtin/merge.c b/builtin/merge.c index 8da3e46abb0..1d14767c0c3 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -879,13 +879,15 @@ static void prepare_to_commit(struct commit_list *remoteheads) strbuf_addch(&msg, '\n'); if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) { wt_status_append_cut_line(&msg); - strbuf_commented_addf(&msg, "\n"); + strbuf_commented_addf(&msg, comment_line_char, "\n"); } - strbuf_commented_addf(&msg, _(merge_editor_comment)); + strbuf_commented_addf(&msg, comment_line_char, + _(merge_editor_comment)); if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) - strbuf_commented_addf(&msg, _(scissors_editor_comment)); + strbuf_commented_addf(&msg, comment_line_char, + _(scissors_editor_comment)); else - strbuf_commented_addf(&msg, + strbuf_commented_addf(&msg, comment_line_char, _(no_scissors_editor_comment), comment_line_char); } if (signoff) diff --git a/builtin/notes.c b/builtin/notes.c index d5788352b6e..3bad5b458bb 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -11,6 +11,7 @@ #include "config.h" #include "builtin.h" #include "editor.h" +#include "environment.h" #include "gettext.h" #include "hex.h" #include "notes.h" @@ -157,7 +158,7 @@ static void write_commented_object(int fd, const struct object_id *object) if (strbuf_read(&buf, show.out, 0) < 0) die_errno(_("could not read 'show' output")); - strbuf_add_commented_lines(&cbuf, buf.buf, buf.len); + strbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_char); write_or_die(fd, cbuf.buf, cbuf.len); strbuf_release(&cbuf); @@ -185,9 +186,10 @@ static void prepare_note_data(const struct object_id *object, struct note_data * copy_obj_to_fd(fd, old_note); strbuf_addch(&buf, '\n'); - strbuf_add_commented_lines(&buf, "\n", strlen("\n")); - strbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template))); - strbuf_add_commented_lines(&buf, "\n", strlen("\n")); + strbuf_add_commented_lines(&buf, "\n", strlen("\n"), comment_line_char); + strbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)), + comment_line_char); + strbuf_add_commented_lines(&buf, "\n", strlen("\n"), comment_line_char); write_or_die(fd, buf.buf, buf.len); write_commented_object(fd, object); @@ -199,7 +201,7 @@ static void prepare_note_data(const struct object_id *object, struct note_data * if (launch_editor(d->edit_path, &d->buf, NULL)) { die(_("please supply the note contents using either -m or -F option")); } - strbuf_stripspace(&d->buf, 1); + strbuf_stripspace(&d->buf, comment_line_char); } } @@ -225,7 +227,7 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset) if (d->buf.len) strbuf_addch(&d->buf, '\n'); strbuf_addstr(&d->buf, arg); - strbuf_stripspace(&d->buf, 0); + strbuf_stripspace(&d->buf, '\0'); d->given = 1; return 0; @@ -244,7 +246,7 @@ static int parse_file_arg(const struct option *opt, const char *arg, int unset) die_errno(_("cannot read '%s'"), arg); } else if (strbuf_read_file(&d->buf, arg, 1024) < 0) die_errno(_("could not open or read '%s'"), arg); - strbuf_stripspace(&d->buf, 0); + strbuf_stripspace(&d->buf, '\0'); d->given = 1; return 0; diff --git a/builtin/rebase.c b/builtin/rebase.c index ace1d5e8d11..b1ba9fb05dd 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -209,7 +209,7 @@ static int edit_todo_file(unsigned flags) if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) return error_errno(_("could not read '%s'."), todo_file); - strbuf_stripspace(&todo_list.buf, 1); + strbuf_stripspace(&todo_list.buf, comment_line_char); res = edit_todo_list(the_repository, &todo_list, &new_todo, NULL, NULL, flags); if (!res && todo_list_write_to_file(the_repository, &new_todo, todo_file, NULL, NULL, -1, flags & ~(TODO_LIST_SHORTEN_IDS))) diff --git a/builtin/stripspace.c b/builtin/stripspace.c index 9451eb69ff4..1987752359c 100644 --- a/builtin/stripspace.c +++ b/builtin/stripspace.c @@ -1,6 +1,7 @@ #include "builtin.h" #include "cache.h" #include "config.h" +#include "environment.h" #include "gettext.h" #include "parse-options.h" #include "setup.h" @@ -13,7 +14,7 @@ static void comment_lines(struct strbuf *buf) size_t len; msg = strbuf_detach(buf, &len); - strbuf_add_commented_lines(buf, msg, len); + strbuf_add_commented_lines(buf, msg, len, comment_line_char); free(msg); } @@ -58,7 +59,8 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix) die_errno("could not read the input"); if (mode == STRIP_DEFAULT || mode == STRIP_COMMENTS) - strbuf_stripspace(&buf, mode == STRIP_COMMENTS); + strbuf_stripspace(&buf, + mode == STRIP_COMMENTS ? comment_line_char : '\0'); else comment_lines(&buf); diff --git a/builtin/tag.c b/builtin/tag.c index 1850a6a6fdc..b79e0a88e68 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -311,9 +311,11 @@ static void create_tag(const struct object_id *object, const char *object_ref, struct strbuf buf = STRBUF_INIT; strbuf_addch(&buf, '\n'); if (opt->cleanup_mode == CLEANUP_ALL) - strbuf_commented_addf(&buf, _(tag_template), tag, comment_line_char); + strbuf_commented_addf(&buf, comment_line_char, + _(tag_template), tag, comment_line_char); else - strbuf_commented_addf(&buf, _(tag_template_nocleanup), tag, comment_line_char); + strbuf_commented_addf(&buf, comment_line_char, + _(tag_template_nocleanup), tag, comment_line_char); write_or_die(fd, buf.buf, buf.len); strbuf_release(&buf); } @@ -327,7 +329,8 @@ static void create_tag(const struct object_id *object, const char *object_ref, } if (opt->cleanup_mode != CLEANUP_NONE) - strbuf_stripspace(buf, opt->cleanup_mode == CLEANUP_ALL); + strbuf_stripspace(buf, + opt->cleanup_mode == CLEANUP_ALL ? comment_line_char : '\0'); if (!opt->message_given && !buf->len) die(_("no tag message?")); diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c index 5af0d4715ba..5d15e2387d8 100644 --- a/fmt-merge-msg.c +++ b/fmt-merge-msg.c @@ -508,7 +508,8 @@ static void fmt_tag_signature(struct strbuf *tagbuf, strbuf_complete_line(tagbuf); if (sig->len) { strbuf_addch(tagbuf, '\n'); - strbuf_add_commented_lines(tagbuf, sig->buf, sig->len); + strbuf_add_commented_lines(tagbuf, sig->buf, sig->len, + comment_line_char); } } @@ -554,7 +555,8 @@ static void fmt_merge_msg_sigs(struct strbuf *out) strbuf_addch(&tagline, '\n'); strbuf_add_commented_lines(&tagline, origins.items[first_tag].string, - strlen(origins.items[first_tag].string)); + strlen(origins.items[first_tag].string), + comment_line_char); strbuf_insert(&tagbuf, 0, tagline.buf, tagline.len); strbuf_release(&tagline); @@ -562,7 +564,8 @@ static void fmt_merge_msg_sigs(struct strbuf *out) strbuf_addch(&tagbuf, '\n'); strbuf_add_commented_lines(&tagbuf, origins.items[i].string, - strlen(origins.items[i].string)); + strlen(origins.items[i].string), + comment_line_char); fmt_tag_signature(&tagbuf, &sig, buf, len); } strbuf_release(&payload); diff --git a/gpg-interface.c b/gpg-interface.c index f3ac5acdd9a..c27a751b70d 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -11,6 +11,7 @@ #include "tempfile.h" #include "alias.h" #include "wrapper.h" +#include "environment.h" static int git_gpg_config(const char *, const char *, void *); @@ -584,8 +585,8 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc, } } - strbuf_stripspace(&ssh_keygen_out, 0); - strbuf_stripspace(&ssh_keygen_err, 0); + strbuf_stripspace(&ssh_keygen_out, '\0'); + strbuf_stripspace(&ssh_keygen_err, '\0'); /* Add stderr outputs to show the user actual ssh-keygen errors */ strbuf_add(&ssh_keygen_out, ssh_principals_err.buf, ssh_principals_err.len); strbuf_add(&ssh_keygen_out, ssh_keygen_err.buf, ssh_keygen_err.len); diff --git a/rebase-interactive.c b/rebase-interactive.c index 789f4073617..012defbb532 100644 --- a/rebase-interactive.c +++ b/rebase-interactive.c @@ -71,13 +71,14 @@ void append_todo_help(int command_count, if (!edit_todo) { strbuf_addch(buf, '\n'); - strbuf_commented_addf(buf, Q_("Rebase %s onto %s (%d command)", - "Rebase %s onto %s (%d commands)", - command_count), + strbuf_commented_addf(buf, comment_line_char, + Q_("Rebase %s onto %s (%d command)", + "Rebase %s onto %s (%d commands)", + command_count), shortrevisions, shortonto, command_count); } - strbuf_add_commented_lines(buf, msg, strlen(msg)); + strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char); if (get_missing_commit_check_level() == MISSING_COMMIT_CHECK_ERROR) msg = _("\nDo not remove any line. Use 'drop' " @@ -86,7 +87,7 @@ void append_todo_help(int command_count, msg = _("\nIf you remove a line here " "THAT COMMIT WILL BE LOST.\n"); - strbuf_add_commented_lines(buf, msg, strlen(msg)); + strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char); if (edit_todo) msg = _("\nYou are editing the todo file " @@ -97,7 +98,7 @@ void append_todo_help(int command_count, msg = _("\nHowever, if you remove everything, " "the rebase will be aborted.\n\n"); - strbuf_add_commented_lines(buf, msg, strlen(msg)); + strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char); } int edit_todo_list(struct repository *r, struct todo_list *todo_list, @@ -129,7 +130,7 @@ int edit_todo_list(struct repository *r, struct todo_list *todo_list, if (launch_sequence_editor(todo_file, &new_todo->buf, NULL)) return -2; - strbuf_stripspace(&new_todo->buf, 1); + strbuf_stripspace(&new_todo->buf, comment_line_char); if (initial && new_todo->buf.len == 0) return -3; diff --git a/sequencer.c b/sequencer.c index c88d1d95538..eae4563ef75 100644 --- a/sequencer.c +++ b/sequencer.c @@ -659,11 +659,12 @@ void append_conflicts_hint(struct index_state *istate, } strbuf_addch(msgbuf, '\n'); - strbuf_commented_addf(msgbuf, "Conflicts:\n"); + strbuf_commented_addf(msgbuf, comment_line_char, "Conflicts:\n"); for (i = 0; i < istate->cache_nr;) { const struct cache_entry *ce = istate->cache[i++]; if (ce_stage(ce)) { - strbuf_commented_addf(msgbuf, "\t%s\n", ce->name); + strbuf_commented_addf(msgbuf, comment_line_char, + "\t%s\n", ce->name); while (i < istate->cache_nr && !strcmp(ce->name, istate->cache[i]->name)) i++; @@ -1142,7 +1143,8 @@ void cleanup_message(struct strbuf *msgbuf, cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) strbuf_setlen(msgbuf, wt_status_locate_end(msgbuf->buf, msgbuf->len)); if (cleanup_mode != COMMIT_MSG_CLEANUP_NONE) - strbuf_stripspace(msgbuf, cleanup_mode == COMMIT_MSG_CLEANUP_ALL); + strbuf_stripspace(msgbuf, + cleanup_mode == COMMIT_MSG_CLEANUP_ALL ? comment_line_char : '\0'); } /* @@ -1173,7 +1175,8 @@ int template_untouched(const struct strbuf *sb, const char *template_file, if (!template_file || strbuf_read_file(&tmpl, template_file, 0) <= 0) return 0; - strbuf_stripspace(&tmpl, cleanup_mode == COMMIT_MSG_CLEANUP_ALL); + strbuf_stripspace(&tmpl, + cleanup_mode == COMMIT_MSG_CLEANUP_ALL ? comment_line_char : '\0'); if (!skip_prefix(sb->buf, tmpl.buf, &start)) start = sb->buf; strbuf_release(&tmpl); @@ -1545,7 +1548,8 @@ static int try_to_commit(struct repository *r, cleanup = opts->default_msg_cleanup; if (cleanup != COMMIT_MSG_CLEANUP_NONE) - strbuf_stripspace(msg, cleanup == COMMIT_MSG_CLEANUP_ALL); + strbuf_stripspace(msg, + cleanup == COMMIT_MSG_CLEANUP_ALL ? comment_line_char : '\0'); if ((flags & EDIT_MSG) && message_is_empty(msg, cleanup)) { res = 1; /* run 'git commit' to display error message */ goto out; @@ -1839,7 +1843,7 @@ static void add_commented_lines(struct strbuf *buf, const void *str, size_t len) s += count; len -= count; } - strbuf_add_commented_lines(buf, s, len); + strbuf_add_commented_lines(buf, s, len, comment_line_char); } /* Does the current fixup chain contain a squash command? */ @@ -1938,7 +1942,7 @@ static int append_squash_message(struct strbuf *buf, const char *body, strbuf_addf(buf, _(nth_commit_msg_fmt), ++opts->current_fixup_count + 1); strbuf_addstr(buf, "\n\n"); - strbuf_add_commented_lines(buf, body, commented_len); + strbuf_add_commented_lines(buf, body, commented_len, comment_line_char); /* buf->buf may be reallocated so store an offset into the buffer */ fixup_off = buf->len; strbuf_addstr(buf, body + commented_len); @@ -2028,7 +2032,8 @@ static int update_squash_messages(struct repository *r, _(first_commit_msg_str)); strbuf_addstr(&buf, "\n\n"); if (is_fixup_flag(command, flag)) - strbuf_add_commented_lines(&buf, body, strlen(body)); + strbuf_add_commented_lines(&buf, body, strlen(body), + comment_line_char); else strbuf_addstr(&buf, body); @@ -2047,7 +2052,8 @@ static int update_squash_messages(struct repository *r, strbuf_addf(&buf, _(skip_nth_commit_msg_fmt), ++opts->current_fixup_count + 1); strbuf_addstr(&buf, "\n\n"); - strbuf_add_commented_lines(&buf, body, strlen(body)); + strbuf_add_commented_lines(&buf, body, strlen(body), + comment_line_char); } else return error(_("unknown command: %d"), command); repo_unuse_commit_buffer(r, commit, message); diff --git a/strbuf.c b/strbuf.c index d5978fee4e4..67e399b60a6 100644 --- a/strbuf.c +++ b/strbuf.c @@ -1,6 +1,5 @@ #include "git-compat-util.h" #include "alloc.h" -#include "environment.h" #include "gettext.h" #include "hex.h" #include "strbuf.h" @@ -362,7 +361,8 @@ static void add_lines(struct strbuf *out, strbuf_complete_line(out); } -void strbuf_add_commented_lines(struct strbuf *out, const char *buf, size_t size) +void strbuf_add_commented_lines(struct strbuf *out, const char *buf, + size_t size, char comment_line_char) { static char prefix1[3]; static char prefix2[2]; @@ -374,7 +374,8 @@ void strbuf_add_commented_lines(struct strbuf *out, const char *buf, size_t size add_lines(out, prefix1, prefix2, buf, size); } -void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...) +void strbuf_commented_addf(struct strbuf *sb, char comment_line_char, + const char *fmt, ...) { va_list params; struct strbuf buf = STRBUF_INIT; @@ -384,7 +385,7 @@ void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...) strbuf_vaddf(&buf, fmt, params); va_end(params); - strbuf_add_commented_lines(sb, buf.buf, buf.len); + strbuf_add_commented_lines(sb, buf.buf, buf.len, comment_line_char); if (incomplete_line) sb->buf[--sb->len] = '\0'; @@ -1051,10 +1052,10 @@ static size_t cleanup(char *line, size_t len) * * If last line does not have a newline at the end, one is added. * - * Enable skip_comments to skip every line starting with comment - * character. + * 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) { size_t empties = 0; size_t i, j, len, newlen; @@ -1067,7 +1068,8 @@ void strbuf_stripspace(struct strbuf *sb, int skip_comments) eol = memchr(sb->buf + i, '\n', sb->len - i); len = eol ? eol - (sb->buf + i) + 1 : sb->len - i; - if (skip_comments && len && sb->buf[i] == comment_line_char) { + if (comment_line_char && len && + sb->buf[i] == comment_line_char) { newlen = 0; continue; } diff --git a/strbuf.h b/strbuf.h index 5d97e27b991..da91c172578 100644 --- a/strbuf.h +++ b/strbuf.h @@ -291,7 +291,8 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t len, * by a comment character and a blank. */ void strbuf_add_commented_lines(struct strbuf *out, - const char *buf, size_t size); + const char *buf, size_t size, + char comment_line_char); /** @@ -420,8 +421,8 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...); * Add a formatted string prepended by a comment character and a * blank to the buffer. */ -__attribute__((format (printf, 2, 3))) -void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...); +__attribute__((format (printf, 3, 4))) +void strbuf_commented_addf(struct strbuf *sb, char comment_line_char, const char *fmt, ...); __attribute__((format (printf,2,0))) void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap); @@ -543,10 +544,11 @@ int strbuf_getcwd(struct strbuf *sb); int strbuf_normalize_path(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. 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); static inline int strbuf_strip_suffix(struct strbuf *sb, const char *suffix) { diff --git a/wt-status.c b/wt-status.c index 97b9c1c0359..da7734866fd 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1023,7 +1023,7 @@ static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncom if (s->display_comment_prefix) { size_t len; summary_content = strbuf_detach(&summary, &len); - strbuf_add_commented_lines(&summary, summary_content, len); + strbuf_add_commented_lines(&summary, summary_content, len, comment_line_char); free(summary_content); } @@ -1098,8 +1098,8 @@ void wt_status_append_cut_line(struct strbuf *buf) { const char *explanation = _("Do not modify or remove the line above.\nEverything below it will be ignored."); - strbuf_commented_addf(buf, "%s", cut_line); - strbuf_add_commented_lines(buf, explanation, strlen(explanation)); + strbuf_commented_addf(buf, comment_line_char, "%s", cut_line); + strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char); } void wt_status_add_cut_line(FILE *fp)