git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH v2 1/2] rebase-merges: move labels' whitespace mangling into `label_oid()`
Date: Sun, 17 Nov 2019 23:16:09 +0000	[thread overview]
Message-ID: <3f4d086e805b69cf1cbf8058f382cc154239f703.1574032570.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.327.v2.git.1574032570.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

One of the trickier aspects of the design of `git rebase
--rebase-merges` is the way labels are generated for the initial todo
list: those labels are supposed to be intuitive and first and foremost
unique.

To that end, `label_oid()` appends a unique suffix when necessary.

Those labels not only need to be unique, but they also need to be valid
refs. To make sure of that, `make_script_with_merges()` replaces
whitespace by dashes.

That would appear to be the wrong layer for that sanitizing step,
though: all callers of `label_oid()` should get that same benefit.

Even if it does not make a difference currently (the only called of
`label_oid()` that passes a label that might need to be sanitized _is_
`make_script_with_merges()`), let's move the responsibility for
sanitizing labels into the `label_oid()` function.

This commit is best viewed with `-w` because it unfortunately needs to
change the indentation of a large block of code in `label_oid()`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 56 ++++++++++++++++++++++++++---------------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 8952cfa89b..85c66f489f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4423,7 +4423,6 @@ static const char *label_oid(struct object_id *oid, const char *label,
 	struct labels_entry *labels_entry;
 	struct string_entry *string_entry;
 	struct object_id dummy;
-	size_t len;
 	int i;
 
 	string_entry = oidmap_get(&state->commit2label, oid);
@@ -4443,10 +4442,10 @@ static const char *label_oid(struct object_id *oid, const char *label,
 	 * abbreviation for any uninteresting commit's names that does not
 	 * clash with any other label.
 	 */
+	strbuf_reset(&state->buf);
 	if (!label) {
 		char *p;
 
-		strbuf_reset(&state->buf);
 		strbuf_grow(&state->buf, GIT_MAX_HEXSZ);
 		label = p = state->buf.buf;
 
@@ -4469,32 +4468,37 @@ static const char *label_oid(struct object_id *oid, const char *label,
 				p[i] = save;
 			}
 		}
-	} else if (((len = strlen(label)) == the_hash_algo->hexsz &&
-		    !get_oid_hex(label, &dummy)) ||
-		   (len == 1 && *label == '#') ||
-		   hashmap_get_from_hash(&state->labels,
-					 strihash(label), label)) {
-		/*
-		 * If the label already exists, or if the label is a valid full
-		 * OID, or the label is a '#' (which we use as a separator
-		 * between merge heads and oneline), we append a dash and a
-		 * number to make it unique.
-		 */
+	} else {
 		struct strbuf *buf = &state->buf;
 
-		strbuf_reset(buf);
-		strbuf_add(buf, label, len);
+		for (; *label; label++)
+			strbuf_addch(buf, isspace(*label) ? '-' : *label);
+		label = buf->buf;
 
-		for (i = 2; ; i++) {
-			strbuf_setlen(buf, len);
-			strbuf_addf(buf, "-%d", i);
-			if (!hashmap_get_from_hash(&state->labels,
-						   strihash(buf->buf),
-						   buf->buf))
-				break;
-		}
+		if ((buf->len == the_hash_algo->hexsz &&
+		     !get_oid_hex(label, &dummy)) ||
+		    (buf->len == 1 && *label == '#') ||
+		    hashmap_get_from_hash(&state->labels,
+					  strihash(label), label)) {
+			/*
+			 * If the label already exists, or if the label is a
+			 * valid full OID, or the label is a '#' (which we use
+			 * as a separator between merge heads and oneline), we
+			 * append a dash and a number to make it unique.
+			 */
+			size_t len = buf->len;
 
-		label = buf->buf;
+			for (i = 2; ; i++) {
+				strbuf_setlen(buf, len);
+				strbuf_addf(buf, "-%d", i);
+				if (!hashmap_get_from_hash(&state->labels,
+							   strihash(buf->buf),
+							   buf->buf))
+					break;
+			}
+
+			label = buf->buf;
+		}
 	}
 
 	FLEX_ALLOC_STR(labels_entry, label, label);
@@ -4596,10 +4600,6 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 		else
 			strbuf_addbuf(&label, &oneline);
 
-		for (p1 = label.buf; *p1; p1++)
-			if (isspace(*p1))
-				*(char *)p1 = '-';
-
 		strbuf_reset(&buf);
 		strbuf_addf(&buf, "%s -C %s",
 			    cmd_merge, oid_to_hex(&commit->object.oid));
-- 
gitgitgadget


  reply	other threads:[~2019-11-17 23:16 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-02 14:01 [PATCH 0/1] Make git rebase -r's label generation more resilient Johannes Schindelin via GitGitGadget
2019-09-02 14:01 ` [PATCH 1/1] rebase -r: let `label` generate safer labels Matt R via GitGitGadget
2019-09-02 17:57   ` Phillip Wood
2019-09-02 18:29     ` Junio C Hamano
2019-09-02 20:12       ` brian m. carlson
2019-09-02 21:24       ` Philip Oakley
     [not found]         ` <CAOjrSZtw+wYHxFRQCfb80xzm9OsGDh2rW8uD+AYYdmDPxk5DFQ@mail.gmail.com>
2019-09-02 22:13           ` Philip Oakley
2019-09-03 11:19       ` Johannes Schindelin
2019-09-03 19:51         ` Junio C Hamano
2019-09-03 22:40           ` Matt Rogers
2019-09-02 19:42     ` Johannes Schindelin
2019-09-03 18:10       ` Junio C Hamano
2019-11-18 20:51         ` Johannes Schindelin
2019-11-17 23:16 ` [PATCH v2 0/2] Make git rebase -r's label generation more resilient Johannes Schindelin via GitGitGadget
2019-11-17 23:16   ` Johannes Schindelin via GitGitGadget [this message]
2019-11-17 23:16   ` [PATCH v2 2/2] rebase -r: let `label` generate safer labels Matthew Rogers via GitGitGadget
2019-11-18  3:42   ` [PATCH v2 0/2] Make git rebase -r's label generation more resilient Junio C Hamano
2019-11-18 11:23     ` [PATCH] sequencer: handle rebase-merge for "onto" message Danh Doan
2019-11-18 11:57       ` [PATCH v2] sequencer: handle rebase-merges " Doan Tran Cong Danh
2019-11-18 20:15         ` Johannes Schindelin
2019-11-21  0:16       ` [PATCH] sequencer: handle rebase-merge " Junio C Hamano
2019-11-18 20:52     ` [PATCH v2 0/2] Make git rebase -r's label generation more resilient Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3f4d086e805b69cf1cbf8058f382cc154239f703.1574032570.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).