| Message ID | 20240307092126.GF2080210@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | allow multi-byte core.commentChar | expand |
On Thu, Mar 07, 2024 at 04:21:26AM -0500, Jeff King wrote: > As part of our transition to multi-byte comment characters, let's take a > NUL-terminated string pointer for strbuf_stripspace(), rather than a > single character. We can continue to support its feature of ignoring > comments by accepting a NULL pointer (as opposed to the current behavior > of a NUL byte). > > All of the callers have to be adjusted, but they can all just pass > comment_line_str (or NULL). Bah. I relied on the compiler to tell me the call-sites that needed to be adjusted. But interestingly gcc is quite happy to allow '\0' to be passed in place of a pointer, but clang complains: gpg-interface.c:589:37: error: expression which evaluates to zero treated as a null pointer constant of type 'const char *' [-Werror,-Wnon-literal-null-conversion] strbuf_stripspace(&ssh_keygen_out, '\0'); ^~~~ Likewise there are a few bare "0"'s which do not cause a warning, but which violate our style standards. So I think we'd want to squash the patch below in to this step. The other functions don't need the same treatment because they never treated NUL specially. --- diff --git a/builtin/am.c b/builtin/am.c index d1990d7edc..5bc72d7822 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1286,7 +1286,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, NULL); assert(!state->author_name); state->author_name = strbuf_detach(&author_name, NULL); diff --git a/builtin/commit.c b/builtin/commit.c index 8519a004d0..e04f1236e8 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -890,7 +890,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, NULL); if (signoff) append_signoff(&sb, ignored_log_message_bytes(sb.buf, sb.len), 0); diff --git a/builtin/notes.c b/builtin/notes.c index 1a67f01d00..cb011303e6 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -264,7 +264,7 @@ static void concat_messages(struct note_data *d) if ((d->stripspace == UNSPECIFIED && d->messages[i]->stripspace == STRIPSPACE) || d->stripspace == STRIPSPACE) - strbuf_stripspace(&d->buf, 0); + strbuf_stripspace(&d->buf, NULL); strbuf_reset(&msg); } strbuf_release(&msg); diff --git a/builtin/worktree.c b/builtin/worktree.c index 9c76b62b02..f0aa962cf8 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -657,7 +657,7 @@ static int can_use_local_refs(const struct add_opts *opts) strbuf_add_real_path(&path, get_worktree_git_dir(NULL)); strbuf_addstr(&path, "/HEAD"); strbuf_read_file(&contents, path.buf, 64); - strbuf_stripspace(&contents, 0); + strbuf_stripspace(&contents, NULL); strbuf_strip_suffix(&contents, "\n"); warning(_("HEAD points to an invalid (or orphaned) reference.\n" diff --git a/gpg-interface.c b/gpg-interface.c index 95e764acb1..b5993385ff 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -586,8 +586,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, NULL); + strbuf_stripspace(&ssh_keygen_err, NULL); /* 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/builtin/branch.c b/builtin/branch.c index cfb63cce5f..c03c0407d1 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -678,7 +678,7 @@ static int edit_branch_description(const char *branch_name) strbuf_release(&buf); return -1; } - strbuf_stripspace(&buf, comment_line_char); + strbuf_stripspace(&buf, comment_line_str); strbuf_addf(&name, "branch.%s.description", branch_name); if (buf.len || exists) diff --git a/builtin/notes.c b/builtin/notes.c index caf20fd5bd..5223a3f350 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -223,7 +223,7 @@ static void prepare_note_data(const struct object_id *object, struct note_data * die(_("please supply the note contents using either -m or -F option")); } if (d->stripspace) - strbuf_stripspace(&d->buf, comment_line_char); + strbuf_stripspace(&d->buf, comment_line_str); } } diff --git a/builtin/rebase.c b/builtin/rebase.c index 6ead9465a4..bf78402129 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -204,7 +204,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, comment_line_char); + strbuf_stripspace(&todo_list.buf, comment_line_str); 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 7b700a9fb1..434ac490cb 100644 --- a/builtin/stripspace.c +++ b/builtin/stripspace.c @@ -59,7 +59,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix) if (mode == STRIP_DEFAULT || mode == STRIP_COMMENTS) strbuf_stripspace(&buf, - mode == STRIP_COMMENTS ? comment_line_char : '\0'); + mode == STRIP_COMMENTS ? comment_line_str : NULL); else comment_lines(&buf); diff --git a/builtin/tag.c b/builtin/tag.c index 19a7e06bf4..07327d3c04 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -310,7 +310,7 @@ 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 ? comment_line_char : '\0'); + opt->cleanup_mode == CLEANUP_ALL ? comment_line_str : NULL); if (!opt->message_given && !buf->len) die(_("no tag message?")); diff --git a/rebase-interactive.c b/rebase-interactive.c index d9718409b3..6dfc33e4e3 100644 --- a/rebase-interactive.c +++ b/rebase-interactive.c @@ -130,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, comment_line_char); + strbuf_stripspace(&new_todo->buf, comment_line_str); if (initial && new_todo->buf.len == 0) return -3; diff --git a/sequencer.c b/sequencer.c index f49a871ac0..6a1b7b200e 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1152,7 +1152,7 @@ void cleanup_message(struct strbuf *msgbuf, 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 ? comment_line_char : '\0'); + cleanup_mode == COMMIT_MSG_CLEANUP_ALL ? comment_line_str : NULL); } /* @@ -1184,7 +1184,7 @@ int template_untouched(const struct strbuf *sb, const char *template_file, return 0; strbuf_stripspace(&tmpl, - cleanup_mode == COMMIT_MSG_CLEANUP_ALL ? comment_line_char : '\0'); + cleanup_mode == COMMIT_MSG_CLEANUP_ALL ? comment_line_str : NULL); if (!skip_prefix(sb->buf, tmpl.buf, &start)) start = sb->buf; strbuf_release(&tmpl); @@ -1557,7 +1557,7 @@ static int try_to_commit(struct repository *r, if (cleanup != COMMIT_MSG_CLEANUP_NONE) strbuf_stripspace(msg, - cleanup == COMMIT_MSG_CLEANUP_ALL ? comment_line_char : '\0'); + cleanup == COMMIT_MSG_CLEANUP_ALL ? comment_line_str : NULL); if ((flags & EDIT_MSG) && message_is_empty(msg, cleanup)) { res = 1; /* run 'git commit' to display error message */ goto out; diff --git a/strbuf.c b/strbuf.c index a33aed6c07..e9b6127e76 100644 --- a/strbuf.c +++ b/strbuf.c @@ -1001,10 +1001,10 @@ static size_t cleanup(char *line, size_t len) * * If last line does not have a newline at the end, one is added. * - * Pass a non-NUL comment_prefix to skip every line starting + * Pass a non-NULL comment_prefix to skip every line starting * with it. */ -void strbuf_stripspace(struct strbuf *sb, char comment_prefix) +void strbuf_stripspace(struct strbuf *sb, const char *comment_prefix) { size_t empties = 0; size_t i, j, len, newlen; @@ -1018,7 +1018,7 @@ void strbuf_stripspace(struct strbuf *sb, char comment_prefix) len = eol ? eol - (sb->buf + i) + 1 : sb->len - i; if (comment_prefix && len && - sb->buf[i] == comment_prefix) { + starts_with(sb->buf + i, comment_prefix)) { newlen = 0; continue; } diff --git a/strbuf.h b/strbuf.h index 860fcec5fb..dc4710adbb 100644 --- a/strbuf.h +++ b/strbuf.h @@ -513,11 +513,11 @@ int strbuf_getcwd(struct strbuf *sb); int strbuf_normalize_path(struct strbuf *sb); /** - * Strip whitespace from a buffer. If comment_prefix is non-NUL, + * Strip whitespace from a buffer. If comment_prefix is non-NULL, * then lines beginning with that character are considered comments, * thus removed. */ -void strbuf_stripspace(struct strbuf *buf, char comment_prefix); +void strbuf_stripspace(struct strbuf *buf, const char *comment_prefix); static inline int strbuf_strip_suffix(struct strbuf *sb, const char *suffix) {
As part of our transition to multi-byte comment characters, let's take a NUL-terminated string pointer for strbuf_stripspace(), rather than a single character. We can continue to support its feature of ignoring comments by accepting a NULL pointer (as opposed to the current behavior of a NUL byte). All of the callers have to be adjusted, but they can all just pass comment_line_str (or NULL). Inside the function we detect comments by comparing the first byte of a line to the comment character. We'll adjust that to use starts_with(), which will match multiple bytes (though for now, of course, we still only allow a single byte, so it's academic). Signed-off-by: Jeff King <peff@peff.net> --- builtin/branch.c | 2 +- builtin/notes.c | 2 +- builtin/rebase.c | 2 +- builtin/stripspace.c | 2 +- builtin/tag.c | 2 +- rebase-interactive.c | 2 +- sequencer.c | 6 +++--- strbuf.c | 6 +++--- strbuf.h | 4 ++-- 9 files changed, 14 insertions(+), 14 deletions(-)