git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 2/6] rebase -i: add abbreviate_commands function

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
May 2, 2017, 15:32 UTC
Message-ID
<alpine.DEB.2.20.1705021641240.3480@virtualbox>
In-Reply-To
<20170502040048.9065-3-liambeguin@gmail.com>
Hi Liam,
On Tue, 2 May 2017, Liam Beguin wrote:
Show 21 quoted lines
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 9b8a030ff045..4fa621062cdf 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -884,6 +884,20 @@ add_exec_commands () {
>  	mv "$1.new" "$1"
>  }
>  
> +abbreviate_commands () {
> +	test "$(git config --bool rebase.abbreviateCommands)" = true || return
> +
> +	while read -r command rest
> +	do
> +		case $command in
> +		x|exec) command=x ;;
> +		*)      command=${command%${command#?}} ;;
> +		esac
> +		printf "%s\n" "$command $rest"
> +	done <"$1" >"$1.new" &&
> +	mv -f "$1.new" "$1"
> +}

Neither this function nor its callers handle errors, but that is in line with how the other todo list-munging functions work.

I just say this to document that I read this code, noted this shortcoming, and think that it is okay to keep it this way for the moment.

There is one more fundamental comment I have, though: we already have a step that converts the todo list to/from the human readable format (by collapsing/expanding the commit hashes presented in the list). We could easily make this conversion a part of that conversion.

Once my `rebase-i-extra` patches get accepted, we will need to rewrite this in C, as part of sequencer.c, anyway. It will look somewhat like this:

-- snipsnap --
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index e6591f01112..5eda71307af 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -11,7 +11,7 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
 	struct replay_opts opts = REPLAY_OPTS_INIT;
-	int keep_empty = 0;
+	int keep_empty = 0, abbreviate_commands = 0;
 	enum {
 		CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,
 		CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH
@@ -39,6 +39,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 	};
 
 	git_config(git_default_config, NULL);
+	git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands);
 
 	opts.action = REPLAY_INTERACTIVE_REBASE;
 	opts.allow_ff = 1;
@@ -52,11 +53,12 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 	if (command == ABORT && argc == 1)
 		return !!sequencer_remove_state(&opts);
 	if (command == MAKE_SCRIPT && argc > 1)
-		return !!sequencer_make_script(keep_empty, stdout, argc, argv);
+		return !!sequencer_make_script(keep_empty, abbreviate_commands,
+					       stdout, argc, argv);
 	if (command == SHORTEN_SHA1S && argc == 1)
-		return !!transform_todo_ids(1);
+		return !!transform_todo_ids(1, abbreviate_commands);
 	if (command == EXPAND_SHA1S && argc == 1)
-		return !!transform_todo_ids(0);
+		return !!transform_todo_ids(0, 0);
 	if (command == CHECK_TODO_LIST && argc == 1)
 		return !!check_todo_list();
 	if (command == SKIP_UNNECESSARY_PICKS && argc == 1)
diff --git a/sequencer.c b/sequencer.c
index 63a588f0916..ca504063d16 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2390,7 +2390,7 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
 	strbuf_release(&sob);
 }
 
-int sequencer_make_script(int keep_empty, FILE *out,
+int sequencer_make_script(int keep_empty, int abbreviate_commands, FILE *out,
 		int argc, const char **argv)
 {
 	char *format = NULL;
@@ -2430,7 +2430,9 @@ int sequencer_make_script(int keep_empty, FILE *out,
 		strbuf_reset(&buf);
 		if (!keep_empty && is_original_commit_empty(commit))
 			strbuf_addf(&buf, "%c ", comment_line_char);
-		strbuf_addf(&buf, "pick %s ", oid_to_hex(&commit->object.oid));
+		strbuf_addf(&buf, "%s %s ",
+			    abbreviate_commands ? "p" : "pick",
+			    oid_to_hex(&commit->object.oid));
 		pretty_print_commit(&pp, commit, &buf);
 		strbuf_addch(&buf, '\n');
 		fputs(buf.buf, out);
@@ -2440,7 +2442,7 @@ int sequencer_make_script(int keep_empty, FILE *out,
 }
 
 
-int transform_todo_ids(int shorten_sha1s)
+int transform_todo_ids(int shorten_sha1s, int abbreviate_commands)
 {
 	const char *todo_file = rebase_path_todo();
 	struct todo_list todo_list = TODO_LIST_INIT;
@@ -2476,16 +2478,31 @@ int transform_todo_ids(int shorten_sha1s)
 			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);
+		if (item->command >= TODO_EXEC && item->command != TODO_DROP) {
+			if (!abbreviate_commands)
+				fwrite(p, eol - bol, 1, out);
+			else {
+				const char *end_of_line = p + eol;
+				p += strspn(p, " \t"); /* skip whitespace */
+				p += strcspn(p, " \t"); /* skip command */
+				fprintf(out, "%c%.*s",
+					todo_command_info[item->command].c,
+					(int)(end_of_line - p), p);
+			}
+		}
 		else {
 			const char *sha1 = shorten_sha1s ?
 				short_commit_name(item->commit) :
 				oid_to_hex(&item->commit->object.oid);
 			int len;
 
-			p += strspn(p, " \t"); /* left-trim command */
-			len = strcspn(p, " \t"); /* length of command */
+			if (abbreviate_commands) {
+				p = &todo_command_info[item->command].c;
+				len = 1;
+			} else {
+				p += strspn(p, " \t"); /* left-trim command */
+				len = strcspn(p, " \t"); /* length of command */
+			}
 
 			fprintf(out, "%.*s %s %.*s\n",
 				len, p, sha1, item->arg_len, item->arg);
diff --git a/sequencer.h b/sequencer.h
index 1c94bec7622..9a7f4333900 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -45,10 +45,10 @@ int sequencer_continue(struct replay_opts *opts);
 int sequencer_rollback(struct replay_opts *opts);
 int sequencer_remove_state(struct replay_opts *opts);
 
-int sequencer_make_script(int keep_empty, FILE *out,
+int sequencer_make_script(int keep_empty, int abbreviate_commands, FILE *out,
 		int argc, const char **argv);
 
-int transform_todo_ids(int shorten_sha1s);
+int transform_todo_ids(int shorten_sha1s, int abbreviate_commands);
 int check_todo_list(void);
 int skip_unnecessary_picks(void);
 int rearrange_squash(void);
Previous: Liam BeguinNext: Liam Beguin
Message 5 of 21 in “rebase -i: add config to abbreviate command-names”
  1. 0/6 rebase -i: add config to abbreviate command-namesLiam Beguin, May 2, 2017
  2. 1/6 rebase -i: add abbreviated command-names handlingLiam Beguin, May 2, 2017
  3. Johannes SchindelinMay 2, 2017
  4. 2/6 rebase -i: add abbreviate_commands functionLiam Beguin, May 2, 2017
  5. Johannes SchindelinMay 2, 2017
  6. 3/6 rebase -i: add short command-name in --autosquashLiam Beguin, May 2, 2017
  7. Johannes SchindelinMay 2, 2017
  8. Liam BeguinMay 2, 2017
  9. 6/6 Documentation: document the rebase.abbreviateCommands optionLiam Beguin, May 2, 2017
  10. 4/6 Documentation: move rebase.* config variables to a separate rebase-config.txtLiam Beguin, May 2, 2017
  11. Johannes SchindelinMay 2, 2017
  12. 5/6 Documentation: use preferred name for the 'todo list' scriptLiam Beguin, May 2, 2017
  13. Ævar Arnfjörð BjarmasonMay 2, 2017
  14. Johannes SchindelinMay 2, 2017
  15. Johannes SchindelinMay 2, 2017
  16. Liam BeguinMay 2, 2017
  17. Johannes SchindelinMay 3, 2017
  18. Junio C HamanoMay 4, 2017
  19. Liam BeguinMay 7, 2017
  20. Junio C HamanoMay 8, 2017
  21. Liam BeguinMay 8, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.