Thread (73 messages) 73 messages, 9 authors, 2017-05-31

Re: [PATCH v4 05/10] rebase -i: also expand/collapse the SHA-1s via the rebase--helper

From: Johannes Schindelin <hidden>
Date: 2017-05-29 11:20:55

Hi Liam,

On Thu, 25 May 2017, Liam Beguin wrote:
Johannes Schindelin [off-list ref] writes:
quoted
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 821058d452d..9444c8d6c60 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -24,6 +24,10 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 				ABORT),
 		OPT_CMDMODE(0, "make-script", &command,
 			N_("make rebase script"), MAKE_SCRIPT),
+		OPT_CMDMODE(0, "shorten-sha1s", &command,
+			N_("shorten SHA-1s in the todo list"), SHORTEN_SHA1S),
+		OPT_CMDMODE(0, "expand-sha1s", &command,
+			N_("expand SHA-1s in the todo list"), EXPAND_SHA1S),
Since work is being done to convert to `struct object_id` would it
not be best to use a more generic name instead of 'sha1'?
maybe something like {shorten,expand}-hashs
Good point. You suggest the use of "ids" later, and I think that is an
even better name: what we try to do here is to expand/reduce the commit
*identifiers*. The fact that they are hexadecimal representations of
hashes is an implementation detail.
quoted
diff --git a/sequencer.c b/sequencer.c
index 88819a1a2a9..201d45b1677 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2437,3 +2437,60 @@ int sequencer_make_script(int keep_empty, FILE *out,
 	strbuf_release(&buf);
 	return 0;
 }
+
+
+int transform_todo_ids(int shorten_sha1s)
+{
+	const char *todo_file = rebase_path_todo();
+	struct todo_list todo_list = TODO_LIST_INIT;
+	int fd, res, i;
+	FILE *out;
+
+	strbuf_reset(&todo_list.buf);
+	fd = open(todo_file, O_RDONLY);
+	if (fd < 0)
+		return error_errno(_("could not open '%s'"), todo_file);
+	if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
+		close(fd);
+		return error(_("could not read '%s'."), todo_file);
+	}
+	close(fd);
+
+	res = parse_insn_buffer(todo_list.buf.buf, &todo_list);
+	if (res) {
+		todo_list_release(&todo_list);
+		return error(_("unusable instruction sheet: '%s'"), todo_file);
As you pointed out last time, the name of the "todo script" can be a
source of confusion. The migration to C could be a good opportunity to
clarify this.
True. This was simply a copy-edited part, and I should have caught that.
I don't know which is the preferred name but we could go with
"todo list" as it is the most common across the code base.
Yep, my next iteration will use that term.
quoted
+	}
+
+	out = fopen(todo_file, "w");
+	if (!out) {
+		todo_list_release(&todo_list);
+		return error(_("unable to open '%s' for writing"), todo_file);
+	}
+	for (i = 0; i < todo_list.nr; i++) {
+		struct todo_item *item = todo_list.items + i;
+		int bol = item->offset_in_buf;
+		const char *p = todo_list.buf.buf + bol;
+		int eol = i + 1 < todo_list.nr ?
+			todo_list.items[i + 1].offset_in_buf :
+			todo_list.buf.len;
+
+		if (item->command >= TODO_EXEC && item->command != TODO_DROP)
+			fwrite(p, eol - bol, 1, out);
+		else {
+			const char *sha1 = shorten_sha1s ?
+				short_commit_name(item->commit) :
+				oid_to_hex(&item->commit->object.oid);
We could also use 'hash' or 'ids' here instead of 'sha1'.
Absolutely!

Thank you,
Johannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help