From patchwork Tue Mar 12 09:17:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589650 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E578E78285 for ; Tue, 12 Mar 2024 09:17:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235029; cv=none; b=BFm/mD1uVQvPertbDBQKVKees82kr0Bf4/AsT06m8FNWE2Herj9yMxKxTkcQSSoKAfbA99S2aBMYXaHZF3HKoYxYyNul+MjpHrnFPhaM0Tneb+j4sIVvuSO9knm2jf2nqcNZE70koIvnNwgYy90swJ20IRnjDOeglFchl+osBzU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235029; c=relaxed/simple; bh=8/gaBe020pkfhuTohNqkA909omd7GHlXa6WsvM41NfE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=iYG/aKbChYPeixoJjK9MXe2q7UWgdpY7pdaxWpdvtq+rCJQ1//w/izrdgKVDwjbS5uiGu1pMqPqbBMgo4MNCohD9SNIus+ap6U7MZETjfqoux/nPkMi6xbIEHXzR8xBuG5hiHCqrflnGa3apz70wWQajyA1ZJt1JO/t5hRdXQOk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17425 invoked by uid 109); 12 Mar 2024 09:17:07 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:07 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 27995 invoked by uid 111); 12 Mar 2024 09:17:11 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:11 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:06 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 01/16] config: forbid newline as core.commentChar Message-ID: <20240312091706.GA95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> Since we usually look for a comment char while parsing line-oriented files, setting core.commentChar to a single newline can confuse our code quite a bit. For example, using it with "git commit" causes us to fail to recognize any of the template as comments, including it in the config message. Which kind of makes sense, since the template content is on its own line (so no line can "start" with a newline). In other spots I would not be surprised if you can create more mischief (e.g., violating loop assumptions) but I didn't dig into it. Since comment characters are a local preference, to some degree this is a case of "if it hurts, don't do it". But given that this would be a silly and pointless thing to do, and that it makes it harder to reason about code parsing comment lines, let's just forbid it. There are other cases that are perhaps questionable (e.g., setting the comment char to a single space), but they seem to behave reasonably (at least a simple "git commit" will correctly identify and strip the template lines). So I haven't worried about going on a hunt for every stupid thing a user might do to themselves, and just focused on the most confusing case. Signed-off-by: Jeff King --- In the string version I suppose you could set it to "\nexec rm -rf /" if you really wanted to treat yourself to a fun "git rebase". Again, this is all local, but it's perhaps nice to know that core.commentChar is not a vector for arbitrary code execution. (That of course made me wonder if setting it to just "exec rm -rf / " would work, as the rest of the template line would be ignored by "rm"; but that is self-defeating as we'd recognize the line as a comment and remove it). config.c | 2 ++ t/t0030-stripspace.sh | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/config.c b/config.c index 3cfeb3d8bd..f561631374 100644 --- a/config.c +++ b/config.c @@ -1566,6 +1566,8 @@ static int git_default_core_config(const char *var, const char *value, else if (!strcasecmp(value, "auto")) auto_comment_line_char = 1; else if (value[0] && !value[1]) { + if (value[0] == '\n') + return error(_("core.commentChar cannot be newline")); comment_line_char = value[0]; auto_comment_line_char = 0; } else diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh index d1b3be8725..e399dd9189 100755 --- a/t/t0030-stripspace.sh +++ b/t/t0030-stripspace.sh @@ -401,6 +401,11 @@ test_expect_success 'strip comments with changed comment char' ' test -z "$(echo "; comment" | git -c core.commentchar=";" stripspace -s)" ' +test_expect_success 'newline as commentchar is forbidden' ' + test_must_fail git -c core.commentChar="$LF" stripspace -s 2>err && + grep "core.commentChar cannot be newline" err +' + test_expect_success '-c with single line' ' printf "# foo\n" >expect && printf "foo" | git stripspace -c >actual && From patchwork Tue Mar 12 09:17:10 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589651 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2C8717867B for ; Tue, 12 Mar 2024 09:17:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235034; cv=none; b=oHADLSdX1TQtl8l4rFiDVTkkY1PPtToSiaCIRlUB5RA3pCWTA22uV/ryN7WpEQ7WOK/Si/17ZYXcCZXE3CqBMVkNQXxpb7pocxacoz47MLaqEg/+qiOW5dELxdWT6P2IRqnaeUVUPwSSyrsg9Pis+al9IgmgIr7k7pwpn83XyuI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235034; c=relaxed/simple; bh=QRk+GyfzVuwn8CzF1WApDjtkqt5eHgp93hlCiP3Li3Q=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=eUV8ejyEg9S0Temq0mkYjWZD5RT6ekOxsaX6Eps/4eN6HZJrsxyRxDF0MzZGbFSzzlZn9YkqXCTLVoG047XmHKECtq6sHZhogXITBaPVIoEJm3KdOReA5y9ab4sEUiBSi/XZyNJVkmBABYx0eDk09scE3AUpT1+jAUFDHUQFLsw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17445 invoked by uid 109); 12 Mar 2024 09:17:11 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:11 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28007 invoked by uid 111); 12 Mar 2024 09:17:15 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:15 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:10 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 02/16] strbuf: simplify comment-handling in add_lines() helper Message-ID: <20240312091710.GB95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> In strbuf_add_commented_lines(), we prepare two strings with potential prefixes: one with just the comment char, and one with an additional space. In the add_lines() helper, we use the one without the extra space for blank lines or lines starting with a tab. While passing in two separate prefixes to the helper is very flexible, it's more flexibility than we actually use (or are likely to use, since the rules inside add_lines() only make sense if "prefix2" is a variant of "prefix1" without the extra space). And setting up the two strings makes refactoring in strbuf_add_commented_lines() awkward. Instead, let's pass in a single string, and just let add_lines() add the extra space to the result as appropriate. We do still need to pass in a flag to trigger this behavior. The helper is shared by strbuf_add_lines(), which passes in a NULL "prefix2" to inhibit this extra handling. Signed-off-by: Jeff King --- strbuf.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/strbuf.c b/strbuf.c index 7827178d8e..689d8acd5e 100644 --- a/strbuf.c +++ b/strbuf.c @@ -340,18 +340,17 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...) } static void add_lines(struct strbuf *out, - const char *prefix1, - const char *prefix2, - const char *buf, size_t size) + const char *prefix, + const char *buf, size_t size, + int space_after_prefix) { while (size) { - const char *prefix; const char *next = memchr(buf, '\n', size); next = next ? (next + 1) : (buf + size); - prefix = ((prefix2 && (buf[0] == '\n' || buf[0] == '\t')) - ? prefix2 : prefix1); strbuf_addstr(out, prefix); + if (space_after_prefix && buf[0] != '\n' && buf[0] != '\t') + strbuf_addch(out, ' '); strbuf_add(out, buf, next - buf); size -= next - buf; buf = next; @@ -362,14 +361,11 @@ static void add_lines(struct strbuf *out, 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]; + static char prefix[2]; - if (prefix1[0] != comment_line_char) { - xsnprintf(prefix1, sizeof(prefix1), "%c ", comment_line_char); - xsnprintf(prefix2, sizeof(prefix2), "%c", comment_line_char); - } - add_lines(out, prefix1, prefix2, buf, size); + if (prefix[0] != comment_line_char) + xsnprintf(prefix, sizeof(prefix), "%c", comment_line_char); + add_lines(out, prefix, buf, size, 1); } void strbuf_commented_addf(struct strbuf *sb, char comment_line_char, @@ -750,7 +746,7 @@ ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint) void strbuf_add_lines(struct strbuf *out, const char *prefix, const char *buf, size_t size) { - add_lines(out, prefix, NULL, buf, size); + add_lines(out, prefix, buf, size, 0); } void strbuf_addstr_xml_quoted(struct strbuf *buf, const char *s) From patchwork Tue Mar 12 09:17:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589652 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C3C8478276 for ; Tue, 12 Mar 2024 09:17:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235038; cv=none; b=n2sDhO0EfiPp3A4HtILos60iG2z2PI4tdWj9lNizRHv8InvtJggAKOxjzqyAkzx81Qr9nolsfkUK26IMWdkpXkF/MbfoLttYyCgAqbEnL2fIgDMP6T48BnD5SXfM8eIQgZzgqm3GNIxhvQoQwPNzgABSjnJWNmKPIt71UhQ22lI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235038; c=relaxed/simple; bh=FKN+Df4IAP5QHRwYjTepM+9uJmbJvW45BxarBARaUeg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=jhUgErbu1mh7GrTz6XQ8/ZoXhjZqXCmzXNqwGztZPZhcV/QmtNbDqDfF9VdQNx7aadSppMkcuS/t5KDpPoA7ok0jYGfcdYTOHS8V1JK1rSAMnozVws2RmG8CsTTPs5pvle4aQ30Z/cizh32jOzOtMWB9JiUxcSHJ3T5cExlCXZs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17469 invoked by uid 109); 12 Mar 2024 09:17:16 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:16 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28016 invoked by uid 111); 12 Mar 2024 09:17:20 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:20 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:15 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 03/16] strbuf: avoid static variables in strbuf_add_commented_lines() Message-ID: <20240312091715.GC95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> In strbuf_add_commented_lines(), we have to convert the single-byte comment_line_char into a string to pass to add_lines(). We cache the created string using a static-local variable. But this makes the function non-reentrant, and it's doubtful that this provides any real performance benefit given that we know the string always contains a single character. So let's just create it from scratch each time, and to give the compiler the maximal opportunity to make it fast we'll ditch the over-complicated xsnprintf() and just assign directly into the array. Signed-off-by: Jeff King --- strbuf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/strbuf.c b/strbuf.c index 689d8acd5e..ca80a2c77e 100644 --- a/strbuf.c +++ b/strbuf.c @@ -361,10 +361,10 @@ static void add_lines(struct strbuf *out, void strbuf_add_commented_lines(struct strbuf *out, const char *buf, size_t size, char comment_line_char) { - static char prefix[2]; + char prefix[2]; - if (prefix[0] != comment_line_char) - xsnprintf(prefix, sizeof(prefix), "%c", comment_line_char); + prefix[0] = comment_line_char; + prefix[1] = '\0'; add_lines(out, prefix, buf, size, 1); } From patchwork Tue Mar 12 09:17:19 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589653 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 020B679B84 for ; Tue, 12 Mar 2024 09:17:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235042; cv=none; b=rOSbRbBFcEGKSOb1Arpd+r9KcfB4BoQUnssVFPn+h1Ios1v+50lDxs+NHq5fbPKfVJl+h1HI3rcGHFDkk8HIkHNFBdN0FmkUVVZYPgCz/bqZ1wdjgGpDYaxcH+1Ru2E02UZtacYPYBZ7LVz1baK7CIOuX8ZfsIrERgxvnhXztXo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235042; c=relaxed/simple; bh=5dvQ8FE98wHs8EPZ9vnT6GWkPYY46GLGx+3nN3Y5H+8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=bPvnnZoRSLpcezRTaTjXB/BNT8J5MUsjIcCSJ69dGBJtdRMzIVY+Ex5gTepae+JbRriGGcFhLO+1oJaAW6+mzhHVKLgiNTdJsXETIieac+htaqt26m40U3WQOb2fQ1R15cz6fwFxnVoljtTd5AEiKvMxkIhOnz49NnKYz/jlJe8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17497 invoked by uid 109); 12 Mar 2024 09:17:20 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:20 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28025 invoked by uid 111); 12 Mar 2024 09:17:24 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:24 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:19 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 04/16] commit: refactor base-case of adjust_comment_line_char() Message-ID: <20240312091719.GD95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> When core.commentChar is set to "auto", we check a set of candidate characters against the proposed buffer to see which if any can be used without ambiguity. But before we do that, we optimize for the common case that the default "#" is fine by just seeing if it is present in the buffer at all. The way we do this is a bit subtle, though: we assign the candidate character to comment_line_char preemptively, then check if it works, and return if it does. The subtle part is that sometimes setting comment_line_char is important (after we return, the important outcome is the fact that we have set the variable) and sometimes it is useless (if our optimization fails, we go on to do the more careful checks and eventually assign something else instead). To make it more clear what is happening (and to make further refactoring of comment_line_char easier), let's check our candidate character directly, and then assign as part of returning if it worked out. Signed-off-by: Jeff King --- builtin/commit.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index a91197245f..b2d05c0cc9 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -685,9 +685,10 @@ static void adjust_comment_line_char(const struct strbuf *sb) char *candidate; const char *p; - comment_line_char = candidates[0]; - if (!memchr(sb->buf, comment_line_char, sb->len)) + if (!memchr(sb->buf, candidates[0], sb->len)) { + comment_line_char = candidates[0]; return; + } p = sb->buf; candidate = strchr(candidates, *p); From patchwork Tue Mar 12 09:17:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589654 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DD9C178284 for ; Tue, 12 Mar 2024 09:17:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235045; cv=none; b=sYTNi0+iTDBwZlDiwZaiNfIyRQEIjJkYqZis66Jukjm0cY7YAdcwt2eHhG7P877yMutlrky0/HWNMLFV1HKB/EgOnYpIXtYQgw0lquZf4mUyqgjrzdv4w4kmj0xR48s5jBjl/Doi8I36yt6kigOLfE4SwT6kIS8bqtYKUAq9OyM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235045; c=relaxed/simple; bh=0nwPY3jAi48v7qoe/p/RIhb6d6n73g+DuQdh/UbNxys=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=JfEC8LLwXF0IZrl3FCDPf2u1vZklx8KbQ15oIXXAMkDNEXuJLtjSid692iJrMgHFFbaniQe+IcyyzRYYw7NfVbsgrR54DQb5p3BWeGBgdapb+rAC5sTuhVoUM0cM6T9zFOqSQHFSvuCT1HxfTEcm8n59eX347ckV/mxDskfH9o8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17513 invoked by uid 109); 12 Mar 2024 09:17:23 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:23 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28034 invoked by uid 111); 12 Mar 2024 09:17:27 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:27 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:22 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 05/16] strbuf: avoid shadowing global comment_line_char name Message-ID: <20240312091722.GE95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> Several comment-related strbuf functions take a comment_line_char parameter. There's also a global comment_line_char variable, which is closely related (most callers pass it in as this parameter). Let's avoid shadowing the global name. This makes it more obvious that we're not using the global value, and it will be especially helpful as we refactor the global in future patches (in particular, any macro trickery wouldn't work because the preprocessor doesn't respect scope). We'll use "comment_prefix". That should be descriptive enough, and as a bonus is more neutral with respect to the "char" type (since we'll eventually swap it out for a string). Signed-off-by: Jeff King --- strbuf.c | 16 ++++++++-------- strbuf.h | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/strbuf.c b/strbuf.c index ca80a2c77e..a33aed6c07 100644 --- a/strbuf.c +++ b/strbuf.c @@ -359,16 +359,16 @@ static void add_lines(struct strbuf *out, } void strbuf_add_commented_lines(struct strbuf *out, const char *buf, - size_t size, char comment_line_char) + size_t size, char comment_prefix) { char prefix[2]; - prefix[0] = comment_line_char; + prefix[0] = comment_prefix; prefix[1] = '\0'; add_lines(out, prefix, buf, size, 1); } -void strbuf_commented_addf(struct strbuf *sb, char comment_line_char, +void strbuf_commented_addf(struct strbuf *sb, char comment_prefix, const char *fmt, ...) { va_list params; @@ -379,7 +379,7 @@ void strbuf_commented_addf(struct strbuf *sb, char comment_line_char, strbuf_vaddf(&buf, fmt, params); va_end(params); - strbuf_add_commented_lines(sb, buf.buf, buf.len, comment_line_char); + strbuf_add_commented_lines(sb, buf.buf, buf.len, comment_prefix); if (incomplete_line) sb->buf[--sb->len] = '\0'; @@ -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_line_char to skip every line starting + * Pass a non-NUL comment_prefix to skip every line starting * with it. */ -void strbuf_stripspace(struct strbuf *sb, char comment_line_char) +void strbuf_stripspace(struct strbuf *sb, char comment_prefix) { size_t empties = 0; size_t i, j, len, newlen; @@ -1017,8 +1017,8 @@ void strbuf_stripspace(struct strbuf *sb, char comment_line_char) eol = memchr(sb->buf + i, '\n', sb->len - i); len = eol ? eol - (sb->buf + i) + 1 : sb->len - i; - if (comment_line_char && len && - sb->buf[i] == comment_line_char) { + if (comment_prefix && len && + sb->buf[i] == comment_prefix) { newlen = 0; continue; } diff --git a/strbuf.h b/strbuf.h index e959caca87..860fcec5fb 100644 --- a/strbuf.h +++ b/strbuf.h @@ -288,7 +288,7 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t len, */ void strbuf_add_commented_lines(struct strbuf *out, const char *buf, size_t size, - char comment_line_char); + char comment_prefix); /** @@ -379,7 +379,7 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...); * blank to the buffer. */ __attribute__((format (printf, 3, 4))) -void strbuf_commented_addf(struct strbuf *sb, char comment_line_char, const char *fmt, ...); +void strbuf_commented_addf(struct strbuf *sb, char comment_prefix, const char *fmt, ...); __attribute__((format (printf,2,0))) void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap); @@ -513,11 +513,11 @@ int strbuf_getcwd(struct strbuf *sb); int strbuf_normalize_path(struct strbuf *sb); /** - * Strip whitespace from a buffer. If comment_line_char is non-NUL, + * Strip whitespace from a buffer. If comment_prefix is non-NUL, * then lines beginning with that character are considered comments, * thus removed. */ -void strbuf_stripspace(struct strbuf *buf, char comment_line_char); +void strbuf_stripspace(struct strbuf *buf, char comment_prefix); static inline int strbuf_strip_suffix(struct strbuf *sb, const char *suffix) { From patchwork Tue Mar 12 09:17:24 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589655 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AF5B479B8D for ; Tue, 12 Mar 2024 09:17:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235049; cv=none; b=L4M0eVElUmvwcMXRjzSRotTfxLCFfLdcta6gc3RU0e3dDUT7+rRDL2nMjiIhsR0Wn5D4deEKOFg/WEC2klYO5WvpH2+TYYdAFyXnqUQbcjuk+9VhoRgnrzwLN2+agFz6/vVkuUGoPtYDcg2D8+V+giYGZK+MQyNeFCmtZ8x0wE0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235049; c=relaxed/simple; bh=oK3fsY6r3C9akxd2KQJRp3QDpeiUExHCUbsm4tomJXQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Lu2hwmEO5bEkj+oVGRZ1cONfCozL1Zv4LllWp99Ey5Vj/dq2TfaW3IvPuuD8+twNkAHsQQ2ZNHCD3yEPk5dYUEIuDCcamlIKk7oPuMNQHO7tH4YS7XsHd5un0VAMAhR6QBrYAoTAyA2PEj3z1M+8dbEG6v7dEwFbQ5oqXpf+Spc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17534 invoked by uid 109); 12 Mar 2024 09:17:26 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:26 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28043 invoked by uid 111); 12 Mar 2024 09:17:30 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:30 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:24 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 06/16] environment: store comment_line_char as a string Message-ID: <20240312091724.GF95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> We'd like to eventually support multi-byte comment prefixes, but the comment_line_char variable is referenced in many spots, making the transition difficult. Let's start by storing the character in a NUL-terminated string. That will let us switch code over incrementally to the string format, and we can easily support the existing code with a macro wrapper (since we'll continue to allow only a single-byte prefix, this will behave identically). Once all references to the "char" variable have been converted, we can drop it and enable longer strings. We'll still have to touch all of the spots that create or set the variable in this patch, but there are only a few (reading the config, and the "auto" character selector). Signed-off-by: Jeff King --- builtin/commit.c | 4 ++-- config.c | 2 +- environment.c | 2 +- environment.h | 3 ++- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index b2d05c0cc9..82229c3100 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -686,7 +686,7 @@ static void adjust_comment_line_char(const struct strbuf *sb) const char *p; if (!memchr(sb->buf, candidates[0], sb->len)) { - comment_line_char = candidates[0]; + comment_line_str = xstrfmt("%c", candidates[0]); return; } @@ -707,7 +707,7 @@ static void adjust_comment_line_char(const struct strbuf *sb) if (!*p) die(_("unable to select a comment character that is not used\n" "in the current commit message")); - comment_line_char = *p; + comment_line_str = xstrfmt("%c", *p); } static void prepare_amend_commit(struct commit *commit, struct strbuf *sb, diff --git a/config.c b/config.c index f561631374..7e5dbca4bd 100644 --- a/config.c +++ b/config.c @@ -1568,7 +1568,7 @@ static int git_default_core_config(const char *var, const char *value, else if (value[0] && !value[1]) { if (value[0] == '\n') return error(_("core.commentChar cannot be newline")); - comment_line_char = value[0]; + comment_line_str = xstrfmt("%c", value[0]); auto_comment_line_char = 0; } else return error(_("core.commentChar should only be one ASCII character")); diff --git a/environment.c b/environment.c index 60706ea398..a73ba9c12c 100644 --- a/environment.c +++ b/environment.c @@ -110,7 +110,7 @@ int protect_ntfs = PROTECT_NTFS_DEFAULT; * The character that begins a commented line in user-editable file * that is subject to stripspace. */ -char comment_line_char = '#'; +const char *comment_line_str = "#"; int auto_comment_line_char; /* Parallel index stat data preload? */ diff --git a/environment.h b/environment.h index 5cec19cecc..1c7d0c2f74 100644 --- a/environment.h +++ b/environment.h @@ -8,7 +8,8 @@ struct strvec; * The character that begins a commented line in user-editable file * that is subject to stripspace. */ -extern char comment_line_char; +#define comment_line_char (comment_line_str[0]) +extern const char *comment_line_str; extern int auto_comment_line_char; /* From patchwork Tue Mar 12 09:17:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589656 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F20F279DA3 for ; Tue, 12 Mar 2024 09:17:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235050; cv=none; b=rkIrULzFf9tumeVG6DfBxW8zFmAmzSF1CXSA/0yQ9RvONDLm4pIeweIQwTIqDNwznYmAZMypCp4YZ+ttKGMg4MSIZtjYaRQrI16w6UlEtjCc1WTwNEh+8urCFefaRdEqMlQqwHzcE/WJVtyL1iLnBbE+lRAjFZTI33yFtXb/tZM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235050; c=relaxed/simple; bh=i4RBKFibyeaPcqnpS8i/HU2tB/foFKjZ12ZrAlNbb2Y=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=u52RV5E1VpQW27f7B9yt3ea2DwZ6rv3+cR5IuXZsGHPr7Qx/YKoRJ8z5/0XCiDhKirV+jDKOZbsuMU4IrpS3tQyX/cNB6s1agBdlrnOBu1wiGX7BhCvNGPQITFU5z6DeksyoHm2zvf1f0Z9jAFoLssrPuJnQ4tNBMycBGrfIujU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17555 invoked by uid 109); 12 Mar 2024 09:17:28 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:28 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28054 invoked by uid 111); 12 Mar 2024 09:17:32 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:32 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:27 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 07/16] strbuf: accept a comment string for strbuf_stripspace() Message-ID: <20240312091727.GG95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> 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 --- builtin/am.c | 2 +- builtin/branch.c | 2 +- builtin/commit.c | 2 +- builtin/notes.c | 4 ++-- builtin/rebase.c | 2 +- builtin/stripspace.c | 2 +- builtin/tag.c | 2 +- builtin/worktree.c | 2 +- gpg-interface.c | 4 ++-- rebase-interactive.c | 2 +- sequencer.c | 6 +++--- strbuf.c | 6 +++--- strbuf.h | 4 ++-- 13 files changed, 20 insertions(+), 20 deletions(-) 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/branch.c b/builtin/branch.c index b3cbb7fd44..f6091f3438 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -684,7 +684,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/commit.c b/builtin/commit.c index 82229c3100..9b139fc795 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 caf20fd5bd..ae981085ea 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); } } @@ -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/rebase.c b/builtin/rebase.c index be787690bd..dc17c4727f 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/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/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 5c6f541126..4819265bf1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1154,7 +1154,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); } /* @@ -1186,7 +1186,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); @@ -1559,7 +1559,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) { From patchwork Tue Mar 12 09:17:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589657 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5740B7A136 for ; Tue, 12 Mar 2024 09:17:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235053; cv=none; b=MxSk3SKSyqgSNEHkc5f7C9bnJQmcenRiO/KHX4lRzNVJkcemoY9A1NMZjh7k82s4R35477s558JT2CgOyw5uq2PjhPvScyRruhcocD4WPQwA8MWY04tD0b5MkrprypP0aFF0drOQB/Ly0Hh/MkZS9L3FjngFSJjzIQtA2Nxp3jY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235053; c=relaxed/simple; bh=z+iLir8QHzqA0DiGFu4DAzWjfVXx1lLy93MDzqtVGuk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Hp3NxwJ1S0Kmh5VoNWeTKhxB/Wsm8bUtoG+vxU0InhoOI/WiLoTJ1hLibZnlAyOkVmtW3T//dmX+A7dnmv85Vlwhsr7/Nust6i0KZHj9C9mwHE9JQkmLSPHjIUiZIpZZR1AlBswhLP4iJQnj9kWVdCjrEqciw+kQo3+aOORwYTU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17576 invoked by uid 109); 12 Mar 2024 09:17:31 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:31 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28063 invoked by uid 111); 12 Mar 2024 09:17:35 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:35 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:29 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 08/16] strbuf: accept a comment string for strbuf_commented_addf() Message-ID: <20240312091729.GH95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> As part of our transition to multi-byte comment characters, let's take a NUL-terminated string pointer for strbuf_commented_addf() rather than a single character. All of the callers have to be adjusted, but they can just pass comment_line_str rather than comment_line_char. Note that we rely on strbuf_add_commented_lines() under the hood, so we'll cheat a bit to squeeze our string into a single character (for now the two are equivalent, and we'll address this TODO in the next patch). Signed-off-by: Jeff King --- add-patch.c | 8 ++++---- builtin/branch.c | 2 +- builtin/merge.c | 8 ++++---- builtin/tag.c | 4 ++-- rebase-interactive.c | 2 +- sequencer.c | 4 ++-- strbuf.c | 10 ++++++++-- strbuf.h | 2 +- wt-status.c | 2 +- 9 files changed, 24 insertions(+), 18 deletions(-) diff --git a/add-patch.c b/add-patch.c index 68f525b35c..7390677795 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1105,11 +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, comment_line_char, + strbuf_commented_addf(&s->buf, comment_line_str, _("Manual hunk edit mode -- see bottom for " "a quick guide.\n")); render_hunk(s, hunk, 0, 0, &s->buf); - strbuf_commented_addf(&s->buf, comment_line_char, + strbuf_commented_addf(&s->buf, comment_line_str, _("---\n" "To remove '%c' lines, make them ' ' lines " "(context).\n" @@ -1118,13 +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, comment_line_char, "%s", + strbuf_commented_addf(&s->buf, comment_line_str, "%s", _(s->mode->edit_hunk_hint)); /* * TRANSLATORS: 'it' refers to the patch mentioned in the previous * messages. */ - strbuf_commented_addf(&s->buf, comment_line_char, + strbuf_commented_addf(&s->buf, comment_line_str, _("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/branch.c b/builtin/branch.c index f6091f3438..2d8c89e9ac 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -673,7 +673,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, comment_line_char, + strbuf_commented_addf(&buf, comment_line_str, _("Please edit the description for the branch\n" " %s\n" "Lines starting with '%c' will be stripped.\n"), diff --git a/builtin/merge.c b/builtin/merge.c index a0ba1f9815..4e47434708 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -852,15 +852,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, comment_line_char, "\n"); + strbuf_commented_addf(&msg, comment_line_str, "\n"); } - strbuf_commented_addf(&msg, comment_line_char, + strbuf_commented_addf(&msg, comment_line_str, _(merge_editor_comment)); if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) - strbuf_commented_addf(&msg, comment_line_char, + strbuf_commented_addf(&msg, comment_line_str, _(scissors_editor_comment)); else - strbuf_commented_addf(&msg, comment_line_char, + strbuf_commented_addf(&msg, comment_line_str, _(no_scissors_editor_comment), comment_line_char); } if (signoff) diff --git a/builtin/tag.c b/builtin/tag.c index 07327d3c04..1c708785bf 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -291,10 +291,10 @@ 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, comment_line_char, + strbuf_commented_addf(&buf, comment_line_str, _(tag_template), tag, comment_line_char); else - strbuf_commented_addf(&buf, comment_line_char, + strbuf_commented_addf(&buf, comment_line_str, _(tag_template_nocleanup), tag, comment_line_char); write_or_die(fd, buf.buf, buf.len); strbuf_release(&buf); diff --git a/rebase-interactive.c b/rebase-interactive.c index 6dfc33e4e3..affc93a8e4 100644 --- a/rebase-interactive.c +++ b/rebase-interactive.c @@ -71,7 +71,7 @@ void append_todo_help(int command_count, if (!edit_todo) { strbuf_addch(buf, '\n'); - strbuf_commented_addf(buf, comment_line_char, + strbuf_commented_addf(buf, comment_line_str, Q_("Rebase %s onto %s (%d command)", "Rebase %s onto %s (%d commands)", command_count), diff --git a/sequencer.c b/sequencer.c index 4819265bf1..051929c9f1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -667,11 +667,11 @@ void append_conflicts_hint(struct index_state *istate, } strbuf_addch(msgbuf, '\n'); - strbuf_commented_addf(msgbuf, comment_line_char, "Conflicts:\n"); + strbuf_commented_addf(msgbuf, comment_line_str, "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, comment_line_char, + strbuf_commented_addf(msgbuf, comment_line_str, "\t%s\n", ce->name); while (i < istate->cache_nr && !strcmp(ce->name, istate->cache[i]->name)) diff --git a/strbuf.c b/strbuf.c index e9b6127e76..76d02e0920 100644 --- a/strbuf.c +++ b/strbuf.c @@ -368,7 +368,7 @@ void strbuf_add_commented_lines(struct strbuf *out, const char *buf, add_lines(out, prefix, buf, size, 1); } -void strbuf_commented_addf(struct strbuf *sb, char comment_prefix, +void strbuf_commented_addf(struct strbuf *sb, const char *comment_prefix, const char *fmt, ...) { va_list params; @@ -379,7 +379,13 @@ void strbuf_commented_addf(struct strbuf *sb, char comment_prefix, strbuf_vaddf(&buf, fmt, params); va_end(params); - strbuf_add_commented_lines(sb, buf.buf, buf.len, comment_prefix); + /* + * TODO Our commented_lines helper does not yet understand + * comment strings. But since we know that the strings are + * always single-char, we can cheat for the moment, and + * fix this later. + */ + strbuf_add_commented_lines(sb, buf.buf, buf.len, comment_prefix[0]); if (incomplete_line) sb->buf[--sb->len] = '\0'; diff --git a/strbuf.h b/strbuf.h index dc4710adbb..b128ca539a 100644 --- a/strbuf.h +++ b/strbuf.h @@ -379,7 +379,7 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...); * blank to the buffer. */ __attribute__((format (printf, 3, 4))) -void strbuf_commented_addf(struct strbuf *sb, char comment_prefix, const char *fmt, ...); +void strbuf_commented_addf(struct strbuf *sb, const char *comment_prefix, const char *fmt, ...); __attribute__((format (printf,2,0))) void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap); diff --git a/wt-status.c b/wt-status.c index 7108a92b52..3845e1d383 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1103,7 +1103,7 @@ 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, comment_line_char, "%s", cut_line); + strbuf_commented_addf(buf, comment_line_str, "%s", cut_line); strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char); } From patchwork Tue Mar 12 09:17:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589658 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C474B7A708 for ; Tue, 12 Mar 2024 09:17:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235055; cv=none; b=pqGPuIb64vINzQWzwXPpGusAqZhGvY79W4JMRRFvX2BplM86PykX+Q0epJdvrEHLTwcOrWNGFCM+gEJYq2Httg5rj7aLiY92UPN1gM/8Pt7ThCcjAVKywO7pJjcbymUBHQcXVhbIz2KkO0fB2LbL5OlJSQyj2ECnSld8lOPxXKE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235055; c=relaxed/simple; bh=XKWG8SylCDu/gK/bDOp/I8Z5kb4I+7RptMC+l7rtG3I=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=WvB1m0upuisXZHodP8SF1WjJ3lN4l+2FbVxWX1EaM2s/ApKeYMk5ohmfVNqUFrjtMre+3Md41xhj5Mo/7DQOJXulTucdJ3NyIWG5lgbmsNyGP3olxttWDVFU4i9l8fTS79tU/NAcL5Rfze9WIOWFajCqyP8OF2q9iS4rONmbtz8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17597 invoked by uid 109); 12 Mar 2024 09:17:33 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:33 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28072 invoked by uid 111); 12 Mar 2024 09:17:37 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:37 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:32 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 09/16] strbuf: accept a comment string for strbuf_add_commented_lines() Message-ID: <20240312091732.GI95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> As part of our transition to multi-byte comment characters, let's take a NUL-terminated string pointer for strbuf_add_commented_lines() rather than a single character. All of the callers have to be adjusted; most can just pass comment_line_str rather than comment_line_char. And now our "cheat" in strbuf_commented_addf() can go away, as we can take the full string from it. Signed-off-by: Jeff King --- builtin/notes.c | 8 ++++---- builtin/stripspace.c | 2 +- fmt-merge-msg.c | 6 +++--- rebase-interactive.c | 6 +++--- sequencer.c | 8 ++++---- strbuf.c | 16 +++------------- strbuf.h | 2 +- wt-status.c | 4 ++-- 8 files changed, 21 insertions(+), 31 deletions(-) diff --git a/builtin/notes.c b/builtin/notes.c index ae981085ea..cb011303e6 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -179,7 +179,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, comment_line_char); + strbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str); write_or_die(fd, cbuf.buf, cbuf.len); strbuf_release(&cbuf); @@ -207,10 +207,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"), comment_line_char); + strbuf_add_commented_lines(&buf, "\n", strlen("\n"), comment_line_str); strbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)), - comment_line_char); - strbuf_add_commented_lines(&buf, "\n", strlen("\n"), comment_line_char); + comment_line_str); + strbuf_add_commented_lines(&buf, "\n", strlen("\n"), comment_line_str); write_or_die(fd, buf.buf, buf.len); write_commented_object(fd, object); diff --git a/builtin/stripspace.c b/builtin/stripspace.c index 434ac490cb..e5626e5126 100644 --- a/builtin/stripspace.c +++ b/builtin/stripspace.c @@ -13,7 +13,7 @@ static void comment_lines(struct strbuf *buf) size_t len; msg = strbuf_detach(buf, &len); - strbuf_add_commented_lines(buf, msg, len, comment_line_char); + strbuf_add_commented_lines(buf, msg, len, comment_line_str); free(msg); } diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c index 66e47449a0..79e8aad086 100644 --- a/fmt-merge-msg.c +++ b/fmt-merge-msg.c @@ -510,7 +510,7 @@ static void fmt_tag_signature(struct strbuf *tagbuf, if (sig->len) { strbuf_addch(tagbuf, '\n'); strbuf_add_commented_lines(tagbuf, sig->buf, sig->len, - comment_line_char); + comment_line_str); } } @@ -557,7 +557,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out) strbuf_add_commented_lines(&tagline, origins.items[first_tag].string, strlen(origins.items[first_tag].string), - comment_line_char); + comment_line_str); strbuf_insert(&tagbuf, 0, tagline.buf, tagline.len); strbuf_release(&tagline); @@ -566,7 +566,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out) strbuf_add_commented_lines(&tagbuf, origins.items[i].string, strlen(origins.items[i].string), - comment_line_char); + comment_line_str); fmt_tag_signature(&tagbuf, &sig, buf, len); } strbuf_release(&payload); diff --git a/rebase-interactive.c b/rebase-interactive.c index affc93a8e4..c343e16fcd 100644 --- a/rebase-interactive.c +++ b/rebase-interactive.c @@ -78,7 +78,7 @@ void append_todo_help(int command_count, shortrevisions, shortonto, command_count); } - strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char); + strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_str); if (get_missing_commit_check_level() == MISSING_COMMIT_CHECK_ERROR) msg = _("\nDo not remove any line. Use 'drop' " @@ -87,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), comment_line_char); + strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_str); if (edit_todo) msg = _("\nYou are editing the todo file " @@ -98,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), comment_line_char); + strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_str); } int edit_todo_list(struct repository *r, struct todo_list *todo_list, diff --git a/sequencer.c b/sequencer.c index 051929c9f1..d12c5a8a03 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1853,7 +1853,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, comment_line_char); + strbuf_add_commented_lines(buf, s, len, comment_line_str); } /* Does the current fixup chain contain a squash command? */ @@ -1952,7 +1952,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, comment_line_char); + strbuf_add_commented_lines(buf, body, commented_len, comment_line_str); /* buf->buf may be reallocated so store an offset into the buffer */ fixup_off = buf->len; strbuf_addstr(buf, body + commented_len); @@ -2043,7 +2043,7 @@ static int update_squash_messages(struct repository *r, strbuf_addstr(&buf, "\n\n"); if (is_fixup_flag(command, flag)) strbuf_add_commented_lines(&buf, body, strlen(body), - comment_line_char); + comment_line_str); else strbuf_addstr(&buf, body); @@ -2063,7 +2063,7 @@ static int update_squash_messages(struct repository *r, ++opts->current_fixup_count + 1); strbuf_addstr(&buf, "\n\n"); strbuf_add_commented_lines(&buf, body, strlen(body), - comment_line_char); + comment_line_str); } else return error(_("unknown command: %d"), command); repo_unuse_commit_buffer(r, commit, message); diff --git a/strbuf.c b/strbuf.c index 76d02e0920..7c8f582127 100644 --- a/strbuf.c +++ b/strbuf.c @@ -359,13 +359,9 @@ static void add_lines(struct strbuf *out, } void strbuf_add_commented_lines(struct strbuf *out, const char *buf, - size_t size, char comment_prefix) + size_t size, const char *comment_prefix) { - char prefix[2]; - - prefix[0] = comment_prefix; - prefix[1] = '\0'; - add_lines(out, prefix, buf, size, 1); + add_lines(out, comment_prefix, buf, size, 1); } void strbuf_commented_addf(struct strbuf *sb, const char *comment_prefix, @@ -379,13 +375,7 @@ void strbuf_commented_addf(struct strbuf *sb, const char *comment_prefix, strbuf_vaddf(&buf, fmt, params); va_end(params); - /* - * TODO Our commented_lines helper does not yet understand - * comment strings. But since we know that the strings are - * always single-char, we can cheat for the moment, and - * fix this later. - */ - strbuf_add_commented_lines(sb, buf.buf, buf.len, comment_prefix[0]); + strbuf_add_commented_lines(sb, buf.buf, buf.len, comment_prefix); if (incomplete_line) sb->buf[--sb->len] = '\0'; diff --git a/strbuf.h b/strbuf.h index b128ca539a..58dddf2777 100644 --- a/strbuf.h +++ b/strbuf.h @@ -288,7 +288,7 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t len, */ void strbuf_add_commented_lines(struct strbuf *out, const char *buf, size_t size, - char comment_prefix); + const char *comment_prefix); /** diff --git a/wt-status.c b/wt-status.c index 3845e1d383..ae623e760e 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1028,7 +1028,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, comment_line_char); + strbuf_add_commented_lines(&summary, summary_content, len, comment_line_str); free(summary_content); } @@ -1104,7 +1104,7 @@ 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, comment_line_str, "%s", cut_line); - strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char); + strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_str); } void wt_status_add_cut_line(struct wt_status *s) From patchwork Tue Mar 12 09:17:34 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589659 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7A29A7A72C for ; Tue, 12 Mar 2024 09:17:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235058; cv=none; b=l2QwoTfs+a9gByW6elG37CFa6XUggFffwyqrh8b/ufYdtYMc1Ww+xywTz9ezZQHK/zSCUj4RMaeAhByv0Ql3CEE0uzqyMLyaPD/v8VJlGHkve1cR/agVwDTKQ5eJULJNJFKTkAY6faXSc6+sx5Wr3Ih1ZoxR9sVLYBhlZ+27lUo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235058; c=relaxed/simple; bh=O+BHieVyAXs5r0C0/pxSXWQLUqvOKBXsOr/243eGkwk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ObaybjKIDjnKpJOCI94TGkgdIXKBvVLuc046ht3qhoGACDFbBSeJ6sYtzQwCVhNBNRGTI0QaB1L7ttxM9Ydu4NmSSciyz8xnvCs3ZwLlGGTSVVqsTvT9crtMP9A07TRIkhgqLkRebVg7+OfNro2riPPYtchVVxlF4ARbEfhnES8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17620 invoked by uid 109); 12 Mar 2024 09:17:36 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:36 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28081 invoked by uid 111); 12 Mar 2024 09:17:40 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:40 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:34 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 10/16] prefer comment_line_str to comment_line_char for printing Message-ID: <20240312091734.GJ95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> As part of our transition to multi-byte comment characters, we should use the string variable rather than the historical character variable. All of the sites adjusted here are just swapping out "%c" for "%s" in format strings, or strbuf_addch() for strbuf_addstr(). The type system and printf-attribute give the compiler enough information to make sure our formats and variable changes all match (especially important for cases where the format string is defined far away from its use, like prepare_to_commit() in commit.c). Signed-off-by: Jeff King --- add-patch.c | 4 ++-- builtin/branch.c | 4 ++-- builtin/commit.c | 12 ++++++------ builtin/merge.c | 4 ++-- builtin/tag.c | 8 ++++---- fmt-merge-msg.c | 2 +- sequencer.c | 20 ++++++++++---------- wt-status.c | 10 +++++----- 8 files changed, 32 insertions(+), 32 deletions(-) diff --git a/add-patch.c b/add-patch.c index 7390677795..4a10237d50 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1114,10 +1114,10 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk) "To remove '%c' lines, make them ' ' lines " "(context).\n" "To remove '%c' lines, delete them.\n" - "Lines starting with %c will be removed.\n"), + "Lines starting with %s will be removed.\n"), s->mode->is_reverse ? '+' : '-', s->mode->is_reverse ? '-' : '+', - comment_line_char); + comment_line_str); strbuf_commented_addf(&s->buf, comment_line_str, "%s", _(s->mode->edit_hunk_hint)); /* diff --git a/builtin/branch.c b/builtin/branch.c index 2d8c89e9ac..faf6ea1b7b 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -676,8 +676,8 @@ static int edit_branch_description(const char *branch_name) strbuf_commented_addf(&buf, comment_line_str, _("Please edit the description for the branch\n" " %s\n" - "Lines starting with '%c' will be stripped.\n"), - branch_name, comment_line_char); + "Lines starting with '%s' will be stripped.\n"), + branch_name, comment_line_str); write_file_buf(edit_description(), buf.buf, buf.len); strbuf_reset(&buf); if (launch_editor(edit_description(), &buf, NULL)) { diff --git a/builtin/commit.c b/builtin/commit.c index 9b139fc795..066dc42a3d 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -910,18 +910,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix, struct ident_split ci, ai; const char *hint_cleanup_all = allow_empty_message ? _("Please enter the commit message for your changes." - " Lines starting\nwith '%c' will be ignored.\n") : + " Lines starting\nwith '%s' will be ignored.\n") : _("Please enter the commit message for your changes." - " Lines starting\nwith '%c' will be ignored, and an empty" + " Lines starting\nwith '%s' will be ignored, and an empty" " message aborts the commit.\n"); const char *hint_cleanup_space = allow_empty_message ? _("Please enter the commit message for your changes." " Lines starting\n" - "with '%c' will be kept; you may remove them" + "with '%s' will be kept; you may remove them" " yourself if you want to.\n") : _("Please enter the commit message for your changes." " Lines starting\n" - "with '%c' will be kept; you may remove them" + "with '%s' will be kept; you may remove them" " yourself if you want to.\n" "An empty message aborts the commit.\n"); if (whence != FROM_COMMIT) { @@ -944,12 +944,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix, fprintf(s->fp, "\n"); if (cleanup_mode == COMMIT_MSG_CLEANUP_ALL) - status_printf(s, GIT_COLOR_NORMAL, hint_cleanup_all, comment_line_char); + status_printf(s, GIT_COLOR_NORMAL, hint_cleanup_all, comment_line_str); else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) { if (whence == FROM_COMMIT) wt_status_add_cut_line(s); } else /* COMMIT_MSG_CLEANUP_SPACE, that is. */ - status_printf(s, GIT_COLOR_NORMAL, hint_cleanup_space, comment_line_char); + status_printf(s, GIT_COLOR_NORMAL, hint_cleanup_space, comment_line_str); /* * These should never fail because they come from our own diff --git a/builtin/merge.c b/builtin/merge.c index 4e47434708..1e33aa49a0 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -821,7 +821,7 @@ static const char scissors_editor_comment[] = N_("An empty message aborts the commit.\n"); static const char no_scissors_editor_comment[] = -N_("Lines starting with '%c' will be ignored, and an empty message aborts\n" +N_("Lines starting with '%s' will be ignored, and an empty message aborts\n" "the commit.\n"); static void write_merge_heads(struct commit_list *); @@ -861,7 +861,7 @@ static void prepare_to_commit(struct commit_list *remoteheads) _(scissors_editor_comment)); else strbuf_commented_addf(&msg, comment_line_str, - _(no_scissors_editor_comment), comment_line_char); + _(no_scissors_editor_comment), comment_line_str); } if (signoff) append_signoff(&msg, ignored_log_message_bytes(msg.buf, msg.len), 0); diff --git a/builtin/tag.c b/builtin/tag.c index 1c708785bf..721d07a589 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -158,11 +158,11 @@ static int do_sign(struct strbuf *buffer) static const char tag_template[] = N_("\nWrite a message for tag:\n %s\n" - "Lines starting with '%c' will be ignored.\n"); + "Lines starting with '%s' will be ignored.\n"); static const char tag_template_nocleanup[] = N_("\nWrite a message for tag:\n %s\n" - "Lines starting with '%c' will be kept; you may remove them" + "Lines starting with '%s' will be kept; you may remove them" " yourself if you want to.\n"); static int git_tag_config(const char *var, const char *value, @@ -292,10 +292,10 @@ static void create_tag(const struct object_id *object, const char *object_ref, strbuf_addch(&buf, '\n'); if (opt->cleanup_mode == CLEANUP_ALL) strbuf_commented_addf(&buf, comment_line_str, - _(tag_template), tag, comment_line_char); + _(tag_template), tag, comment_line_str); else strbuf_commented_addf(&buf, comment_line_str, - _(tag_template_nocleanup), tag, comment_line_char); + _(tag_template_nocleanup), tag, comment_line_str); write_or_die(fd, buf.buf, buf.len); strbuf_release(&buf); } diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c index 79e8aad086..ae201e21db 100644 --- a/fmt-merge-msg.c +++ b/fmt-merge-msg.c @@ -321,7 +321,7 @@ static void credit_people(struct strbuf *out, skip_prefix(me, them->items->string, &me) && starts_with(me, " <"))) return; - strbuf_addf(out, "\n%c %s ", comment_line_char, label); + strbuf_addf(out, "\n%s %s ", comment_line_str, label); add_people_count(out, them); } diff --git a/sequencer.c b/sequencer.c index d12c5a8a03..b75d0c098d 100644 --- a/sequencer.c +++ b/sequencer.c @@ -663,7 +663,7 @@ void append_conflicts_hint(struct index_state *istate, if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) { strbuf_addch(msgbuf, '\n'); wt_status_append_cut_line(msgbuf); - strbuf_addch(msgbuf, comment_line_char); + strbuf_addstr(msgbuf, comment_line_str); } strbuf_addch(msgbuf, '\n'); @@ -1948,7 +1948,7 @@ static int append_squash_message(struct strbuf *buf, const char *body, (starts_with(body, "squash!") || starts_with(body, "fixup!")))) commented_len = commit_subject_length(body); - strbuf_addf(buf, "\n%c ", comment_line_char); + strbuf_addf(buf, "\n%s ", comment_line_str); strbuf_addf(buf, _(nth_commit_msg_fmt), ++opts->current_fixup_count + 1); strbuf_addstr(buf, "\n\n"); @@ -2008,7 +2008,7 @@ static int update_squash_messages(struct repository *r, eol = buf.buf[0] != comment_line_char ? buf.buf : strchrnul(buf.buf, '\n'); - strbuf_addf(&header, "%c ", comment_line_char); + strbuf_addf(&header, "%s ", comment_line_str); strbuf_addf(&header, _(combined_commit_msg_fmt), opts->current_fixup_count + 2); strbuf_splice(&buf, 0, eol - buf.buf, header.buf, header.len); @@ -2034,9 +2034,9 @@ static int update_squash_messages(struct repository *r, repo_unuse_commit_buffer(r, head_commit, head_message); return error(_("cannot write '%s'"), rebase_path_fixup_msg()); } - strbuf_addf(&buf, "%c ", comment_line_char); + strbuf_addf(&buf, "%s ", comment_line_str); strbuf_addf(&buf, _(combined_commit_msg_fmt), 2); - strbuf_addf(&buf, "\n%c ", comment_line_char); + strbuf_addf(&buf, "\n%s ", comment_line_str); strbuf_addstr(&buf, is_fixup_flag(command, flag) ? _(skip_first_commit_msg_str) : _(first_commit_msg_str)); @@ -2058,7 +2058,7 @@ static int update_squash_messages(struct repository *r, if (command == TODO_SQUASH || is_fixup_flag(command, flag)) { res = append_squash_message(&buf, body, command, opts, flag); } else if (command == TODO_FIXUP) { - strbuf_addf(&buf, "\n%c ", comment_line_char); + strbuf_addf(&buf, "\n%s ", comment_line_str); strbuf_addf(&buf, _(skip_nth_commit_msg_fmt), ++opts->current_fixup_count + 1); strbuf_addstr(&buf, "\n\n"); @@ -5667,8 +5667,8 @@ static int make_script_with_merges(struct pretty_print_context *pp, oid_to_hex(&commit->object.oid), oneline.buf); if (is_empty) - strbuf_addf(&buf, " %c empty", - comment_line_char); + strbuf_addf(&buf, " %s empty", + comment_line_str); FLEX_ALLOC_STR(entry, string, buf.buf); oidcpy(&entry->entry.oid, &commit->object.oid); @@ -5758,7 +5758,7 @@ static int make_script_with_merges(struct pretty_print_context *pp, entry = oidmap_get(&state.commit2label, &commit->object.oid); if (entry) - strbuf_addf(out, "\n%c Branch %s\n", comment_line_char, entry->string); + strbuf_addf(out, "\n%s Branch %s\n", comment_line_str, entry->string); else strbuf_addch(out, '\n'); @@ -5895,7 +5895,7 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc, oid_to_hex(&commit->object.oid)); pretty_print_commit(&pp, commit, out); if (is_empty) - strbuf_addf(out, " %c empty", comment_line_char); + strbuf_addf(out, " %s empty", comment_line_str); strbuf_addch(out, '\n'); } if (skipped_commit) diff --git a/wt-status.c b/wt-status.c index ae623e760e..6201a97de0 100644 --- a/wt-status.c +++ b/wt-status.c @@ -70,7 +70,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color, strbuf_vaddf(&sb, fmt, ap); if (!sb.len) { if (s->display_comment_prefix) { - strbuf_addch(&sb, comment_line_char); + strbuf_addstr(&sb, comment_line_str); if (!trail) strbuf_addch(&sb, ' '); } @@ -85,7 +85,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color, strbuf_reset(&linebuf); if (at_bol && s->display_comment_prefix) { - strbuf_addch(&linebuf, comment_line_char); + strbuf_addstr(&linebuf, comment_line_str); if (*line != '\n' && *line != '\t') strbuf_addch(&linebuf, ' '); } @@ -1090,7 +1090,7 @@ size_t wt_status_locate_end(const char *s, size_t len) const char *p; struct strbuf pattern = STRBUF_INIT; - strbuf_addf(&pattern, "\n%c %s", comment_line_char, cut_line); + strbuf_addf(&pattern, "\n%s %s", comment_line_str, cut_line); if (starts_with(s, pattern.buf + 1)) len = 0; else if ((p = strstr(s, pattern.buf))) @@ -1218,8 +1218,8 @@ static void wt_longstatus_print_tracking(struct wt_status *s) "%s%.*s", comment_line_string, (int)(ep - cp), cp); if (s->display_comment_prefix) - color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%c", - comment_line_char); + color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%s", + comment_line_str); else fputs("\n", s->fp); strbuf_release(&sb); From patchwork Tue Mar 12 09:17:37 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589660 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 981FF7AE76 for ; Tue, 12 Mar 2024 09:17:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235060; cv=none; b=QnznyjhHkWxhvmGhto0XOU2wY5J3isuQ4dBGoiqmTyfk5eXJ7rK66/DxOrjfefQjhEju/253OoZPa7bXVov27a9c7QxPEXboSpMCV7DsbhfEXWaZ47mIbrZjFam3NgZWFn3dvruhx6jNh0XsxXJ0ezgjvTj3u4xCpgD482NroJA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235060; c=relaxed/simple; bh=VfYsPqlVt9M/XLybc6igCh1+5bzSNCscr4iDDx93KQ0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=S0EeN9RI7sWlXXVDBTC6vQsP8ajccQX8Ho8zYnRObbm7LFE6HHVRx8v5RXqBFMB73tbTVveYWy63IaDBF/Y1nmC8AP7rzttDsXIMoxL3OgB/JemWvl2lVN/G75nTwCTRpxDrDyvltM18iJe6eyS+tBB4I5gFnzPGVT/yu1t/Xt8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17639 invoked by uid 109); 12 Mar 2024 09:17:38 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:38 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28090 invoked by uid 111); 12 Mar 2024 09:17:42 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:42 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:37 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 11/16] find multi-byte comment chars in NUL-terminated strings Message-ID: <20240312091737.GK95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> Several parts of the code need to identify lines that begin with the comment character, and do so with a simple byte equality check. As part of the transition to handling multi-byte characters, we need to match all of the bytes. For cases where we are looking in a NUL-terminated string, we can just use starts_with(), which checks all of the characters in comment_line_str. Note that we can drop the "line.len" check in wt-status.c's read_rebase_todolist(). The starts_with() function handles the case of an empty haystack buffer (it will always return false for a non-empty prefix). Signed-off-by: Jeff King --- add-patch.c | 2 +- sequencer.c | 2 +- trailer.c | 2 +- wt-status.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/add-patch.c b/add-patch.c index 4a10237d50..d599ca53e1 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1139,7 +1139,7 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk) for (i = 0; i < s->buf.len; ) { size_t next = find_next_line(&s->buf, i); - if (s->buf.buf[i] != comment_line_char) + if (!starts_with(s->buf.buf + i, comment_line_str)) strbuf_add(&s->plain, s->buf.buf + i, next - i); i = next; } diff --git a/sequencer.c b/sequencer.c index b75d0c098d..42125e57a4 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2005,7 +2005,7 @@ static int update_squash_messages(struct repository *r, return error(_("could not read '%s'"), rebase_path_squash_msg()); - eol = buf.buf[0] != comment_line_char ? + eol = !starts_with(buf.buf, comment_line_str) ? buf.buf : strchrnul(buf.buf, '\n'); strbuf_addf(&header, "%s ", comment_line_str); diff --git a/trailer.c b/trailer.c index ef9df4af55..fe18faf6c5 100644 --- a/trailer.c +++ b/trailer.c @@ -1013,7 +1013,7 @@ static void parse_trailers(struct trailer_info *info, for (i = 0; i < info->trailer_nr; i++) { int separator_pos; char *trailer = info->trailers[i]; - if (trailer[0] == comment_line_char) + if (starts_with(trailer, comment_line_str)) continue; separator_pos = find_separator(trailer, separators); if (separator_pos >= 1) { diff --git a/wt-status.c b/wt-status.c index 6201a97de0..8753d59f90 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1386,7 +1386,7 @@ static int read_rebase_todolist(const char *fname, struct string_list *lines) git_path("%s", fname)); } while (!strbuf_getline_lf(&line, f)) { - if (line.len && line.buf[0] == comment_line_char) + if (starts_with(line.buf, comment_line_str)) continue; strbuf_trim(&line); if (!line.len) From patchwork Tue Mar 12 09:17:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589661 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 94BB378266 for ; Tue, 12 Mar 2024 09:17:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235063; cv=none; b=KInAui4g7lx6yrEhhwXuAz9vQ/W/tuN3qUXRulXvwBw+dQBQEHQb0bR7uzwC7cgiFEzDYNxorvVcMDdQ1svuFUNyilPZNuT7HXT/7lJUC0Q6KVdDZ9doT8oZcyqHkuZ08wIF2OByos/iVCydNpH8HieT7Kj//bJYbn1cHX/luQw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235063; c=relaxed/simple; bh=c87M49c+2VzVU0Zz4IkcYfQbsjD8YDglBPqfQItAgEg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=J3WvY4AKToseMCYqVB68f7peGHJ+JIr9DMqA/jd3TakxO/nhm8LRiW8vvM8p5XMZCQXbG14RLb2zuoSphMzgiFpLzha3QKDKDOhcbERD2k1g7ebxK+yO1u5EB08n6CKzuj01ARUJRpHmK4TvYVPspH6JTQfio+oJfvnz6hGIFsE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17665 invoked by uid 109); 12 Mar 2024 09:17:41 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:41 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28099 invoked by uid 111); 12 Mar 2024 09:17:45 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:45 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:39 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 12/16] find multi-byte comment chars in unterminated buffers Message-ID: <20240312091739.GL95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> As with the previous patch, we need to swap out single-byte matching for something like starts_with() to match all bytes of a multi-byte comment character. But for cases where the buffer is not NUL-terminated (and we instead have an explicit size or end pointer), it's not safe to use starts_with(), as it might walk off the end of the buffer. Let's introduce a new starts_with_mem() that does the same thing but also accepts the length of the "haystack" str and makes sure not to walk past it. Note that in most cases the existing code did not need a length check at all, since it was written in a way that knew we had at least one byte available (and that was all we checked). So I had to read each one to find the appropriate bounds. The one exception is sequencer.c's add_commented_lines(), where we can actually get rid of the length check. Just like starts_with(), our starts_with_mem() handles an empty haystack variable by not matching (assuming a non-empty prefix). A few notes on the implementation of starts_with_mem(): - it would be equally correct to take an "end" pointer (and indeed, many of the callers have this and have to subtract to come up with the length). I think taking a ptr/size combo is a more usual interface for our codebase, though, and has the added benefit that the function signature makes it harder to mix up the three parameters. - we could obviously build starts_with() on top of this by passing strlen(str) as the length. But it's possible that starts_with() is a relatively hot code path, and it should not pay that penalty (it can generally return an answer proportional to the size of the prefix, not the whole string). - it naively feels like xstrncmpz() should be able to do the same thing, but that's not quite true. If you pass the length of the haystack buffer, then strncmp() finds that a shorter prefix string is "less than" than the haystack, even if the haystack starts with the prefix. If you pass the length of the prefix, then you risk reading past the end of the haystack if it is shorter than the prefix. So I think we really do need a new function. Signed-off-by: Jeff King --- commit.c | 3 ++- sequencer.c | 4 ++-- strbuf.c | 11 +++++++++++ strbuf.h | 1 + trailer.c | 4 ++-- 5 files changed, 18 insertions(+), 5 deletions(-) diff --git a/commit.c b/commit.c index 467be9f7f9..9cfbe9d657 100644 --- a/commit.c +++ b/commit.c @@ -1797,7 +1797,8 @@ size_t ignored_log_message_bytes(const char *buf, size_t len) else next_line++; - if (buf[bol] == comment_line_char || buf[bol] == '\n') { + if (starts_with_mem(buf + bol, cutoff - bol, comment_line_str) || + buf[bol] == '\n') { /* is this the first of the run of comments? */ if (!boc) boc = bol; diff --git a/sequencer.c b/sequencer.c index 42125e57a4..ef84832855 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1842,7 +1842,7 @@ static int is_fixup_flag(enum todo_command command, unsigned flag) static void add_commented_lines(struct strbuf *buf, const void *str, size_t len) { const char *s = str; - while (len > 0 && s[0] == comment_line_char) { + while (starts_with_mem(s, len, comment_line_str)) { size_t count; const char *n = memchr(s, '\n', len); if (!n) @@ -2564,7 +2564,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item, /* left-trim */ bol += strspn(bol, " \t"); - if (bol == eol || *bol == '\r' || *bol == comment_line_char) { + if (bol == eol || *bol == '\r' || starts_with_mem(bol, eol - bol, comment_line_str)) { item->command = TODO_COMMENT; item->commit = NULL; item->arg_offset = bol - buf; diff --git a/strbuf.c b/strbuf.c index 7c8f582127..291bdc2a65 100644 --- a/strbuf.c +++ b/strbuf.c @@ -24,6 +24,17 @@ int istarts_with(const char *str, const char *prefix) return 0; } +int starts_with_mem(const char *str, size_t len, const char *prefix) +{ + const char *end = str + len; + for (; ; str++, prefix++) { + if (!*prefix) + return 1; + else if (str == end || *str != *prefix) + return 0; + } +} + int skip_to_optional_arg_default(const char *str, const char *prefix, const char **arg, const char *def) { diff --git a/strbuf.h b/strbuf.h index 58dddf2777..3156d6ea8c 100644 --- a/strbuf.h +++ b/strbuf.h @@ -673,6 +673,7 @@ char *xstrfmt(const char *fmt, ...); int starts_with(const char *str, const char *prefix); int istarts_with(const char *str, const char *prefix); +int starts_with_mem(const char *str, size_t len, const char *prefix); /* * If the string "str" is the same as the string in "prefix", then the "arg" diff --git a/trailer.c b/trailer.c index fe18faf6c5..fdb0b8137e 100644 --- a/trailer.c +++ b/trailer.c @@ -882,7 +882,7 @@ static size_t find_trailer_block_start(const char *buf, size_t len) /* The first paragraph is the title and cannot be trailers */ for (s = buf; s < buf + len; s = next_line(s)) { - if (s[0] == comment_line_char) + if (starts_with_mem(s, buf + len - s, comment_line_str)) continue; if (is_blank_line(s)) break; @@ -902,7 +902,7 @@ static size_t find_trailer_block_start(const char *buf, size_t len) const char **p; ssize_t separator_pos; - if (bol[0] == comment_line_char) { + if (starts_with_mem(bol, buf + len - bol, comment_line_str)) { non_trailer_lines += possible_continuation_lines; possible_continuation_lines = 0; continue; From patchwork Tue Mar 12 09:17:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589662 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 193157BAEC for ; Tue, 12 Mar 2024 09:17:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235065; cv=none; b=GA5zRX7xdNCwAyomq+OiGniAy0f+e60eU+mYGSiE/Yi9gmlFmZNe57Dw2b056M1QWuwV0SLTnP/bHB8FkaZvToYJZTH2xPvnhx5hbtZXQNsxNOyf3Iow+0zeM3jyfkTf4rQhpN1IgaZylXWSx2NXNBafq9QSbJSoWEYYxgAET1M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235065; c=relaxed/simple; bh=x8G6de8A1MvFh7EaXh/f/JvvcJFzjg2wa/KZxYri87o=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=pB09uIbYbojsigVrA/5S3HwcmWIbsmf9y5i/19AaK9p7CS+sydWolVPPxe3eCAwMuVzN6AWPSfSzykJvLb+Fimtnf/hvQOVQ6FFRt/fJRVkZj/kWmwQoORi4y0IMH+HTct1o8KmFMWE7Gvwat9vPG7OcZ7sHOIYQJlv7t0FKaXU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17681 invoked by uid 109); 12 Mar 2024 09:17:44 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:44 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28108 invoked by uid 111); 12 Mar 2024 09:17:48 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:48 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:42 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 13/16] sequencer: handle multi-byte comment characters when writing todo list Message-ID: <20240312091742.GM95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> We already match multi-byte comment characters in parse_insn_line(), thanks to the previous commit, yielding a TODO_COMMENT entry. But in todo_list_to_strbuf(), we may call command_to_char() to convert that back into something we can output. We can't just return comment_line_char anymore, since it may require multiple bytes. Instead, we'll return "0" for this case, which is the same thing we'd return for a command which does not have a single-letter abbreviation (e.g., "revert" or "noop"). There is only a single caller of command_to_char(), and upon seeing "0" it falls back to outputting the full name via command_to_string(). So we can handle TODO_COMMENT there, returning the full string. Note that there are many other callers of command_to_string(), which will now behave differently if they pass TODO_COMMENT. But we would not expect that to happen; prior to this commit, the function just calls die() in this case. And looking at those callers, that makes sense; e.g., do_pick_commit() will only be called when servicing a pick command, and should never be called for a comment in the first place. Signed-off-by: Jeff King --- sequencer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index ef84832855..a8fdf00e89 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1781,14 +1781,16 @@ static const char *command_to_string(const enum todo_command command) { if (command < TODO_COMMENT) return todo_command_info[command].str; + if (command == TODO_COMMENT) + return comment_line_str; die(_("unknown command: %d"), command); } static char command_to_char(const enum todo_command command) { if (command < TODO_COMMENT) return todo_command_info[command].c; - return comment_line_char; + return 0; } static int is_noop(const enum todo_command command) From patchwork Tue Mar 12 09:17:45 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589663 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D21D47C091 for ; Tue, 12 Mar 2024 09:17:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235068; cv=none; b=JXieBKz3uBQPpDOkYYqyme0K1lDOFrqPbWsqyaX2YqqPVTejdR0TzUh9kJbfCGr2lBIwacQmzAs5AKFGTHjcPCipDG/jOW9WEd4MTebdF99N59zrpyY+WTDpivi8TBP0m/ntMz0lvofP6wl3EvWyPcI4nlhiKdca/Sb7NzDeUVs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235068; c=relaxed/simple; bh=KrC1XlpYbdGd2krGdt6YFQBmnXkjOs4kms8aFvw/Xlw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Fh7jFWLbbApdZ9LRfQr+N0Afu6NJa5M6rlFrFKvHfX2X3WcACCifWrAKQDhQGtTYvZm9dlzwOKK4ifpdPxHAm+/0UDt10H7EW9KGaYFbAgUjyjqFmm9/MucDoEx+FDKuZ0EMWYJJLTzZL2gEXtoyOHdA5+NCqqNV75j+L8OUcDM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17703 invoked by uid 109); 12 Mar 2024 09:17:46 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:46 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28117 invoked by uid 111); 12 Mar 2024 09:17:50 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:50 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:45 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 14/16] wt-status: drop custom comment-char stringification Message-ID: <20240312091745.GN95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> In wt_longstatus_print_tracking() we may conditionally show a comment prefix based on the wt_status->display_comment_prefix flag. We handle that by creating a local "comment_line_string" that is either the empty string or the comment character followed by a space. For a single-byte comment, the maximum length of this string is 2 (plus a NUL byte). But to handle multi-byte comment characters, it can be arbitrarily large. One way to handle this is to just call xstrfmt("%s ", comment_line_str), and then free it when we're done. But we can simplify things further by just conditionally switching between our prefix string and an empty string when formatting. We couldn't just do that with the previous code, because the comment character was a single byte. There's no way to have a "%c" format switch between some character and "no character at all". Whereas with "%s" you can switch between some string and the empty string. So now that we have a comment string and not a comment char, we can just use it directly when formatting. Do note that we have to also conditionally add the trailing space at the same time. Signed-off-by: Jeff King --- wt-status.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/wt-status.c b/wt-status.c index 8753d59f90..7217ff30c5 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1180,8 +1180,6 @@ static void wt_longstatus_print_tracking(struct wt_status *s) struct strbuf sb = STRBUF_INIT; const char *cp, *ep, *branch_name; struct branch *branch; - char comment_line_string[3]; - int i; uint64_t t_begin = 0; assert(s->branch && !s->is_initial); @@ -1206,16 +1204,11 @@ static void wt_longstatus_print_tracking(struct wt_status *s) } } - i = 0; - if (s->display_comment_prefix) { - comment_line_string[i++] = comment_line_char; - comment_line_string[i++] = ' '; - } - comment_line_string[i] = '\0'; - for (cp = sb.buf; (ep = strchr(cp, '\n')) != NULL; cp = ep + 1) color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), - "%s%.*s", comment_line_string, + "%s%s%.*s", + s->display_comment_prefix ? comment_line_str : "", + s->display_comment_prefix ? " " : "", (int)(ep - cp), cp); if (s->display_comment_prefix) color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%s", From patchwork Tue Mar 12 09:17:47 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589664 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1C7DF7C0B3 for ; Tue, 12 Mar 2024 09:17:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235070; cv=none; b=GkRxmRkLfm6oJauG34qfwiaBaDNLiUhg7yznXrdg/loJ88xusyUjHKOHX80mrHpRwNLEn0QBqWQJSGT9kMQLNBjNFn4DBs+tr+ZiY3uY8wj8eTOenauV/wIGh3uM2wytxe7X3zyJGs+zKgUusN2+gpwkFWEvip52WSOFCQDe/u0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235070; c=relaxed/simple; bh=dE+OgkC6uDx74Un+L8u26xa5OYAByAmeNrMfhV/nnAI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=FutwrmqmdFzcyOD2Jm12/semRSY2i1UtDxRkgyYQDoWOArZymNLKWAZ5i3VByXg9XbB0T5sQ/gqzvxW4W4iO0QZUTh1oND6qhNfL6J+uX5SQCnnTCb7U7z5UpALOLTImXbUFEAzqYK68YBpLUkfqysdYdtR1leGgz7fpHlwoW/k= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17725 invoked by uid 109); 12 Mar 2024 09:17:49 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:49 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28126 invoked by uid 111); 12 Mar 2024 09:17:53 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:53 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:47 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 15/16] environment: drop comment_line_char compatibility macro Message-ID: <20240312091747.GO95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> There is no longer any code which references the single-byte comment_line_char. Let's drop it, clearing the way for true multi-byte entries in comment_line_str. It's possible there are topics in flight that have added new references to comment_line_char. But we would prefer to fail compilation (and then fix it) upon merging with this, rather than have them quietly ignore the bytes after the first. Signed-off-by: Jeff King --- environment.h | 1 - 1 file changed, 1 deletion(-) diff --git a/environment.h b/environment.h index 1c7d0c2f74..05fd94d7be 100644 --- a/environment.h +++ b/environment.h @@ -8,7 +8,6 @@ struct strvec; * The character that begins a commented line in user-editable file * that is subject to stripspace. */ -#define comment_line_char (comment_line_str[0]) extern const char *comment_line_str; extern int auto_comment_line_char; From patchwork Tue Mar 12 09:17:50 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13589665 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D86317CF0D for ; Tue, 12 Mar 2024 09:17:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235073; cv=none; b=sz9sNYDtbBHL8wQknAqShM5KI2kxpRXYaPAb5ezwyJMIEEI6LLvx6fOBfK6KZl8tstvKhaLuNK02ARofOU7xUws9nrldbMUp5USZrWRcjnlksKA7CEZez87boTl8Z9VsqSySaFHozEIJKeSK3GLbEBmZkLxP9hnpwgxSg3Ps8qQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710235073; c=relaxed/simple; bh=W2X2wQciIpkIp04Cl9A9H8wLP75DFSRAktUZyFd8CTU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=nMOjcXUwgjeW3J1UtG9sJrmdUoL6h6ilEFN8a5KqgK/+Iy8oizGrLkX4frLItRC8O9GeJjOAVeBhGflknhOi7zr+Map5/iR4bpRaV7sZNr2obw1jYZQHhRH1UqPXpzydZjzS0XxA4EmvwT45KUH6nA18Pi4o3MFpbBSTN1IEKLg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 17747 invoked by uid 109); 12 Mar 2024 09:17:51 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 12 Mar 2024 09:17:51 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 28135 invoked by uid 111); 12 Mar 2024 09:17:55 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 12 Mar 2024 05:17:55 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 12 Mar 2024 05:17:50 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Dragan Simic , Kristoffer Haugsbakk , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood Subject: [PATCH v2 16/16] config: allow multi-byte core.commentChar Message-ID: <20240312091750.GP95609@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312091013.GA95442@coredump.intra.peff.net> Now that all of the code handles multi-byte comment characters, it's safe to allow users to set them. There is one special case I kept: we still will not allow an empty string for the commentChar. While it might make sense in some contexts (e.g., output where you don't want any comment prefix), there are plenty where it will behave badly (e.g., all of our starts_with() checks will indicate that every line is a comment!). It might be reasonable to assign some meaningful semantics, but it would probably involve checking how each site behaves. In the interim let's forbid it and we can loosen things later. Likewise, the "commentChar cannot be a newline" rule is now extended to "it cannot contain a newline" (for the same reason: it can confuse our parsing loops). Since comment_line_str is used in many parts of the code, it's hard to cover all possibilities with tests. We can convert the existing double-semicolon prefix test to show that "git status" works. And we'll give it a more challenging case in t7507, where we confirm that git-commit strips out the commit template along with any --verbose text when reading the edited commit message back in. That covers the basics, though it's possible there could be issues in more exotic spots (e.g., the sequencer todo list uses its own code). Signed-off-by: Jeff King --- Documentation/config/core.txt | 4 +++- config.c | 10 +++++----- t/t0030-stripspace.sh | 7 ++++++- t/t7507-commit-verbose.sh | 10 ++++++++++ t/t7508-status.sh | 4 +++- 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt index 0e8c2832bf..c86b8c8408 100644 --- a/Documentation/config/core.txt +++ b/Documentation/config/core.txt @@ -523,7 +523,9 @@ core.commentChar:: Commands such as `commit` and `tag` that let you edit messages consider a line that begins with this character commented, and removes them after the editor returns - (default '#'). + (default '#'). Note that this option can take values larger than + a byte (whether a single multi-byte character, or you + could even go wild with a multi-character sequence). + If set to "auto", `git-commit` would select a character that is not the beginning character of any line in existing commit messages. diff --git a/config.c b/config.c index 7e5dbca4bd..92c752ed9f 100644 --- a/config.c +++ b/config.c @@ -1565,13 +1565,13 @@ static int git_default_core_config(const char *var, const char *value, return config_error_nonbool(var); else if (!strcasecmp(value, "auto")) auto_comment_line_char = 1; - else if (value[0] && !value[1]) { - if (value[0] == '\n') - return error(_("core.commentChar cannot be newline")); - comment_line_str = xstrfmt("%c", value[0]); + else if (value[0]) { + if (strchr(value, '\n')) + return error(_("core.commentChar cannot contain newline")); + comment_line_str = xstrdup(value); auto_comment_line_char = 0; } else - return error(_("core.commentChar should only be one ASCII character")); + return error(_("core.commentChar must have at least one character")); return 0; } diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh index e399dd9189..a161faf702 100755 --- a/t/t0030-stripspace.sh +++ b/t/t0030-stripspace.sh @@ -403,7 +403,12 @@ test_expect_success 'strip comments with changed comment char' ' test_expect_success 'newline as commentchar is forbidden' ' test_must_fail git -c core.commentChar="$LF" stripspace -s 2>err && - grep "core.commentChar cannot be newline" err + grep "core.commentChar cannot contain newline" err +' + +test_expect_success 'empty commentchar is forbidden' ' + test_must_fail git -c core.commentchar= stripspace -s 2>err && + grep "core.commentChar must have at least one character" err ' test_expect_success '-c with single line' ' diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh index c3281b192e..4c7db19ce7 100755 --- a/t/t7507-commit-verbose.sh +++ b/t/t7507-commit-verbose.sh @@ -101,6 +101,16 @@ test_expect_success 'verbose diff is stripped out with set core.commentChar' ' test_grep "Aborting commit due to empty commit message." err ' +test_expect_success 'verbose diff is stripped with multi-byte comment char' ' + ( + GIT_EDITOR=cat && + export GIT_EDITOR && + test_must_fail git -c core.commentchar="foo>" commit -a -v >out 2>err + ) && + grep "^foo> " out && + test_grep "Aborting commit due to empty commit message." err +' + test_expect_success 'status does not verbose without --verbose' ' git status >actual && ! grep "^diff --git" actual diff --git a/t/t7508-status.sh b/t/t7508-status.sh index a3c18a4fc2..10ed8b32bc 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -1403,7 +1403,9 @@ test_expect_success "status (core.commentchar with submodule summary)" ' test_expect_success "status (core.commentchar with two chars with submodule summary)" ' test_config core.commentchar ";;" && - test_must_fail git -c status.displayCommentPrefix=true status + sed "s/^/;/" expect.double && + git -c status.displayCommentPrefix=true status >output && + test_cmp expect.double output ' test_expect_success "--ignore-submodules=all suppresses submodule summary" ' From patchwork Wed Mar 27 08:19:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13605869 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7D3CA24A08 for ; Wed, 27 Mar 2024 08:19:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711527566; cv=none; b=I9n+N4SdEMFqlEY0+l4+/ulmld3qjGIvZYyFJeAq+DlavFpRLH5ByU2nGP64lR23eTI2kqJOPM4dHk5wgGTbqisk9jAo4gppXEyoBZxS6HvBpWkAWUJelkNLf1sbk0mbSIpt1McCf+Mict72qFFDpdLgFXOPjgAQQLgKcgQ981I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711527566; c=relaxed/simple; bh=R6FTFSsv9T4PmyZmvGWbL2gSqtJ/PNQ13tfvnCKBYIM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=GLq84whmd3TouFjR69cofHJdM+fVRSHmEImD0ygwWYHilgKMYGaoBVFq1fv5tZFt6JDLCTi/HsfhvA7167Zq3f5x0rawHC6PwKCro+2eli8IPFIz4fUEZWeO5FXK3OwolFQhQBeOSvYZxHUl/rHA497GgDH2/7ThotdyZP6lRcU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21198 invoked by uid 109); 27 Mar 2024 08:19:23 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Wed, 27 Mar 2024 08:19:23 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 24289 invoked by uid 111); 27 Mar 2024 08:19:27 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Wed, 27 Mar 2024 04:19:27 -0400 Authentication-Results: peff.net; auth=none Date: Wed, 27 Mar 2024 04:19:22 -0400 From: Jeff King To: Junio C Hamano Cc: Kristoffer Haugsbakk , Dragan Simic , Manlio Perillo , =?utf-8?b?UmVuw6k=?= Scharfe , Phillip Wood , git@vger.kernel.org Subject: [PATCH 17/16] config: add core.commentString Message-ID: <20240327081922.GA830163@coredump.intra.peff.net> References: <20240312091013.GA95442@coredump.intra.peff.net> <20240312091750.GP95609@coredump.intra.peff.net> <0426f7bf-6032-4fc7-886c-c4278c6e105b@app.fastmail.com> <20240315055944.GB1741107@coredump.intra.peff.net> <6be335ed-8598-406c-b535-2e58554b00e9@app.fastmail.com> <20240315081041.GA1753560@coredump.intra.peff.net> <20240327074655.GA831122@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240327074655.GA831122@coredump.intra.peff.net> On Wed, Mar 27, 2024 at 03:46:55AM -0400, Jeff King wrote: > > My preference is to introduce core.commentString to avoid confusion > > coming from an older Git using the first-byte of a multi-byte > > string, or dying upon reading a configuration file meant for a newer > > Git, and then let core.commentString override core.commentChar, but > > I would prefer to see the discussion participants to raise their > > opinions and reach a conclusion. > > OK. I don't have a strong opinion. Are you OK with core.commentString as > a strict synonym (so last-one-wins and either name overwrites previous)? > Or do you want an override (i.e., commentString always overrides > commentChar, regardless of order). I think it's mostly academic, and the > strict synonym version is much easier to implement. Like this, on top of what you have queued in jk/core-comment-string. Note that you graduated kh/doc-commentchar-is-a-byte, which says "this ASCII character" early in the description, which will be incorrect if my series is merged. This would need to be fixed (possibly as part of merging my topic, though I don't think it actually triggers a conflict, so you'll have to remember to do so manually). Or mine could be rebased on top of master and then remove it as part of the series. -- >8 -- Subject: [PATCH] config: add core.commentString The core.commentChar code recently learned to accept more than a single ASCII character. But using it is annoying with multiple versions of Git, since older ones will reject it outright: $ git.v2.44.0 -c core.commentchar=foo stripspace -s error: core.commentChar should only be one ASCII character fatal: unable to parse 'core.commentchar' from command-line config Let's add an alias core.commentString. That's arguably a better name anyway, since we now can handle strings, and it makes it possible to have a config that works reasonably with both old and new versions of Git (see the example in the documentation). This is strictly an alias, so there's not much point in adding duplicate tests; I added a single one to t0030 that exercises the alias code. Note also that the error messages for invalid values will now show the variable the config parser handed us, and thus will be normalized to lowercase (rather than camelcase). A few tests in t0030 are adjusted to match. Signed-off-by: Jeff King --- An alternative to using "$var cannot ..." in the error messages (if we don't like the all-lowercase variable name) is to just say "comment strings cannot ...". That vaguely covers both cases, and the message printed by the config code itself does mention the actual variable name that triggered the error. Documentation/config/core.txt | 19 ++++++++++++++++--- config.c | 7 ++++--- t/t0030-stripspace.sh | 9 +++++++-- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt index c86b8c8408..bbe869c497 100644 --- a/Documentation/config/core.txt +++ b/Documentation/config/core.txt @@ -520,15 +520,28 @@ core.editor:: `GIT_EDITOR` is not set. See linkgit:git-var[1]. core.commentChar:: +core.commentString:: Commands such as `commit` and `tag` that let you edit messages consider a line that begins with this character commented, and removes them after the editor returns - (default '#'). Note that this option can take values larger than - a byte (whether a single multi-byte character, or you - could even go wild with a multi-character sequence). + (default '#'). + If set to "auto", `git-commit` would select a character that is not the beginning character of any line in existing commit messages. ++ +Note that these two variables are aliases of each other, and in modern +versions of Git you are free to use a string (e.g., `//` or `⁑⁕⁑`) with +`commentChar`. Versions of Git prior to v2.45.0 will ignore +`commentString` but will reject a value of `commentChar` that consists +of more than a single ASCII byte. If you plan to use your config with +older and newer versions of Git, you may want to specify both: ++ + [core] + # single character for older versions + commentChar = "#" + # string for newer versions (which will override commentChar + # because it comes later in the file) + commentString = "//" core.filesRefLockTimeout:: The length of time, in milliseconds, to retry when trying to diff --git a/config.c b/config.c index 92c752ed9f..d12e0f34f1 100644 --- a/config.c +++ b/config.c @@ -1560,18 +1560,19 @@ static int git_default_core_config(const char *var, const char *value, if (!strcmp(var, "core.editor")) return git_config_string(&editor_program, var, value); - if (!strcmp(var, "core.commentchar")) { + if (!strcmp(var, "core.commentchar") || + !strcmp(var, "core.commentstring")) { if (!value) return config_error_nonbool(var); else if (!strcasecmp(value, "auto")) auto_comment_line_char = 1; else if (value[0]) { if (strchr(value, '\n')) - return error(_("core.commentChar cannot contain newline")); + return error(_("%s cannot contain newline"), var); comment_line_str = xstrdup(value); auto_comment_line_char = 0; } else - return error(_("core.commentChar must have at least one character")); + return error(_("%s must have at least one character"), var); return 0; } diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh index a161faf702..f10f42ff1e 100755 --- a/t/t0030-stripspace.sh +++ b/t/t0030-stripspace.sh @@ -401,14 +401,19 @@ test_expect_success 'strip comments with changed comment char' ' test -z "$(echo "; comment" | git -c core.commentchar=";" stripspace -s)" ' +test_expect_success 'strip comments with changed comment string' ' + test ! -z "$(echo "// comment" | git -c core.commentchar=// stripspace)" && + test -z "$(echo "// comment" | git -c core.commentchar="//" stripspace -s)" +' + test_expect_success 'newline as commentchar is forbidden' ' test_must_fail git -c core.commentChar="$LF" stripspace -s 2>err && - grep "core.commentChar cannot contain newline" err + grep "core.commentchar cannot contain newline" err ' test_expect_success 'empty commentchar is forbidden' ' test_must_fail git -c core.commentchar= stripspace -s 2>err && - grep "core.commentChar must have at least one character" err + grep "core.commentchar must have at least one character" err ' test_expect_success '-c with single line' '