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

Convert 'git blame' to parse_options()

From
Linus Torvalds <torvalds@linux-foundation.org>
Date
Jun 23, 2008, 05:15 UTC
Message-ID
<alpine.LFD.1.10.0806222207220.2926@woody.linux-foundation.org>

Why? I did some more size statistics, and 'cmd_blame()' was the biggest function in all of git. Admittedly my script for finding sizes is probably broken, but it does look very messy.

Just cleaning up the option parsing actually cuts down the size a bit, and while parse_options() isn't really perfect for what cmd_blame() wants, it's manageable.

I think I'll want to add a PARSE_OPT_IGNORE_UNRECOGNIZED flag, and also make it not write the resulting array into argv[0..] but back into argv[1..] (so that you can use parse_options() as a _filter_ for options parsing and make it easier to do partial conversions), but in the meantime this mostly works.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

That builtin-blame option parsing is really ugly, trying to then move unrecognized options to be parsed by a secondary

	setup_revisions(unk, argv, &revs, NULL);

which parses the rest. This is _really_ not what parse_options() was written with in mind, and to give strictly the same behaviour as before you really do need some kind of PARSE_OPT_IGNORE_UNRECOGNIZED flag, but I think this is already a good enough halfway point.

Hmm?

Btw, making the "struct options" array be 'static' avoids having to build it up at run-time on the stack, which is why it's usually a good idea. Of course, it means that the option variables themselves have to be static, which has its own set of downsides, so it's a balancing act.

 builtin-blame.c |  177 +++++++++++++++++++++++++++----------------------------
 1 files changed, 86 insertions(+), 91 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index b451f6c..f04bd93 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -18,24 +18,11 @@
 #include "cache-tree.h"
 #include "path-list.h"
 #include "mailmap.h"
+#include "parse-options.h"
 
-static char blame_usage[] =
-"git-blame [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-p] [-w] [-L n,m] [-S <revs-file>] [-M] [-C] [-C] [--contents <filename>] [--incremental] [commit] [--] file\n"
-"  -c                  Use the same output mode as git-annotate (Default: off)\n"
-"  -b                  Show blank SHA-1 for boundary commits (Default: off)\n"
-"  -l                  Show long commit SHA1 (Default: off)\n"
-"  --root              Do not treat root commits as boundaries (Default: off)\n"
-"  -t                  Show raw timestamp (Default: off)\n"
-"  -f, --show-name     Show original filename (Default: auto)\n"
-"  -n, --show-number   Show original linenumber (Default: off)\n"
-"  -s                  Suppress author name and timestamp (Default: off)\n"
-"  -p, --porcelain     Show in a format designed for machine consumption\n"
-"  -w                  Ignore whitespace differences\n"
-"  -L n,m              Process only line range n,m, counting from 1\n"
-"  -M, -C              Find line movements within and across files\n"
-"  --incremental       Show blame entries as we find them, incrementally\n"
-"  --contents file     Use <file>'s contents as the final image\n"
-"  -S revs-file        Use revisions from revs-file instead of calling git-rev-list\n";
+static char blame_usage[] = "git-blame [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-p] [-w] [-L n,m] [-S <revs-file>] [-M] [-C] [-C] [--contents <filename>] [--incremental] [commit] [--] file";
+
+static const char *blame_opt_usage[] = { blame_usage, NULL };
 
 static int longest_file;
 static int longest_author;
@@ -2122,6 +2109,50 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
 	return commit;
 }
 
+static int blame_copy_callback(const struct option *option, const char *arg, int unset)
+{
+	int *opt = option->value;
+
+	/*
+	 * -C enables copy from removed files;
+	 * -C -C enables copy from existing files, but only
+	 *       when blaming a new file;
+	 * -C -C -C enables copy from existing files for
+	 *          everybody
+	 */
+	if (*opt & PICKAXE_BLAME_COPY_HARDER)
+		*opt |= PICKAXE_BLAME_COPY_HARDEST;
+	if (*opt & PICKAXE_BLAME_COPY)
+		*opt |= PICKAXE_BLAME_COPY_HARDER;
+	*opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
+
+	if (arg)
+		blame_copy_score = parse_score(arg);
+	return 0;
+}
+
+static int blame_move_callback(const struct option *option, const char *arg, int unset)
+{
+	int *opt = option->value;
+
+	*opt |= PICKAXE_BLAME_MOVE;
+
+	if (arg)
+		blame_move_score = parse_score(arg);
+	return 0;
+}
+
+static int blame_bottomtop_callback(const struct option *option, const char *arg, int unset)
+{
+	const char **bottomtop = option->value;
+	if (!arg)
+		return -1;
+	if (*bottomtop)
+		die("More than one '-L n,m' option given");
+	*bottomtop = arg;
+	return 0;
+}
+
 int cmd_blame(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
@@ -2129,94 +2160,58 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	struct scoreboard sb;
 	struct origin *o;
 	struct blame_entry *ent;
-	int i, seen_dashdash, unk, opt;
+	int i, seen_dashdash, unk;
 	long bottom, top, lno;
-	int output_option = 0;
-	int show_stats = 0;
-	const char *revs_file = NULL;
 	const char *final_commit_name = NULL;
 	enum object_type type;
-	const char *bottomtop = NULL;
-	const char *contents_from = NULL;
+
+	static const char *bottomtop = NULL;
+	static int output_option = 0, opt = 0;
+	static int show_stats = 0;
+	static const char *revs_file = NULL;
+	static const char *contents_from = NULL;
+	static const struct option options[] = {
+		OPT_BOOLEAN(0, "incremental", &incremental, "Show blame entries as we find them, incrementally"),
+		OPT_BOOLEAN('b', NULL, &blank_boundary, "Show blank SHA-1 for boundary commits (Default: off)"),
+		OPT_BOOLEAN(0, "root", &show_root, "Do not treat root commits as boundaries (Default: off)"),
+		OPT_BOOLEAN(0, "show-stats", &show_stats, "Show work cost statistics"),
+		OPT_BIT(0, "score-debug", &output_option, "Show output score for blame entries", OUTPUT_SHOW_SCORE),
+		OPT_BIT('f', "show-name", &output_option, "Show original filename (Default: auto)", OUTPUT_SHOW_NAME),
+		OPT_BIT('n', "show-number", &output_option, "Show original linenumber (Default: off)", OUTPUT_SHOW_NUMBER),
+		OPT_BIT('p', "porcelain", &output_option, "Show in a format designed for machine consumption", OUTPUT_PORCELAIN),
+		OPT_BIT('c', NULL, &output_option, "Use the same output mode as git-annotate (Default: off)", OUTPUT_ANNOTATE_COMPAT),
+		OPT_BIT('t', NULL, &output_option, "Show raw timestamp (Default: off)", OUTPUT_RAW_TIMESTAMP),
+		OPT_BIT('l', NULL, &output_option, "Show long commit SHA1 (Default: off)", OUTPUT_LONG_OBJECT_NAME),
+		OPT_BIT('s', NULL, &output_option, "Suppress author name and timestamp (Default: off)", OUTPUT_NO_AUTHOR),
+		OPT_BIT('w', NULL, &xdl_opts, "Ignore whitespace differences", XDF_IGNORE_WHITESPACE),
+		OPT_STRING('S', NULL, &revs_file, "file", "Use revisions from <file> instead of calling git-rev-list"),
+		OPT_STRING(0, "contents", &contents_from, "file", "Use <file>'s contents as the final image"),
+		{ OPTION_CALLBACK, 'C', NULL, &opt, "score", "Find line copies within and across files", PARSE_OPT_OPTARG, blame_copy_callback },
+		{ OPTION_CALLBACK, 'M', NULL, &opt, "score", "Find line movements within and across files", PARSE_OPT_OPTARG, blame_move_callback },
+		OPT_CALLBACK('L', NULL, &bottomtop, "n,m", "Process only line range n,m, counting from 1", blame_bottomtop_callback),
+		OPT_END()
+	};
 
 	cmd_is_annotate = !strcmp(argv[0], "annotate");
 
+	argc = parse_options(argc, argv, options, blame_opt_usage,
+		PARSE_OPT_STOP_AT_NON_OPTION | PARSE_OPT_KEEP_DASHDASH);
+
+	/*
+	 * parse_options() offsets things by one - undo for now to make
+	 * old code happy.
+	 */
+	argc++;
+	argv--;
+
 	git_config(git_blame_config, NULL);
 	save_commit_buffer = 0;
 
-	opt = 0;
 	seen_dashdash = 0;
 	for (unk = i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 		if (*arg != '-')
 			break;
-		else if (!strcmp("-b", arg))
-			blank_boundary = 1;
-		else if (!strcmp("--root", arg))
-			show_root = 1;
-		else if (!strcmp(arg, "--show-stats"))
-			show_stats = 1;
-		else if (!strcmp("-c", arg))
-			output_option |= OUTPUT_ANNOTATE_COMPAT;
-		else if (!strcmp("-t", arg))
-			output_option |= OUTPUT_RAW_TIMESTAMP;
-		else if (!strcmp("-l", arg))
-			output_option |= OUTPUT_LONG_OBJECT_NAME;
-		else if (!strcmp("-s", arg))
-			output_option |= OUTPUT_NO_AUTHOR;
-		else if (!strcmp("-w", arg))
-			xdl_opts |= XDF_IGNORE_WHITESPACE;
-		else if (!strcmp("-S", arg) && ++i < argc)
-			revs_file = argv[i];
-		else if (!prefixcmp(arg, "-M")) {
-			opt |= PICKAXE_BLAME_MOVE;
-			blame_move_score = parse_score(arg+2);
-		}
-		else if (!prefixcmp(arg, "-C")) {
-			/*
-			 * -C enables copy from removed files;
-			 * -C -C enables copy from existing files, but only
-			 *       when blaming a new file;
-			 * -C -C -C enables copy from existing files for
-			 *          everybody
-			 */
-			if (opt & PICKAXE_BLAME_COPY_HARDER)
-				opt |= PICKAXE_BLAME_COPY_HARDEST;
-			if (opt & PICKAXE_BLAME_COPY)
-				opt |= PICKAXE_BLAME_COPY_HARDER;
-			opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
-			blame_copy_score = parse_score(arg+2);
-		}
-		else if (!prefixcmp(arg, "-L")) {
-			if (!arg[2]) {
-				if (++i >= argc)
-					usage(blame_usage);
-				arg = argv[i];
-			}
-			else
-				arg += 2;
-			if (bottomtop)
-				die("More than one '-L n,m' option given");
-			bottomtop = arg;
-		}
-		else if (!strcmp("--contents", arg)) {
-			if (++i >= argc)
-				usage(blame_usage);
-			contents_from = argv[i];
-		}
-		else if (!strcmp("--incremental", arg))
-			incremental = 1;
-		else if (!strcmp("--score-debug", arg))
-			output_option |= OUTPUT_SHOW_SCORE;
-		else if (!strcmp("-f", arg) ||
-			 !strcmp("--show-name", arg))
-			output_option |= OUTPUT_SHOW_NAME;
-		else if (!strcmp("-n", arg) ||
-			 !strcmp("--show-number", arg))
-			output_option |= OUTPUT_SHOW_NUMBER;
-		else if (!strcmp("-p", arg) ||
-			 !strcmp("--porcelain", arg))
-			output_option |= OUTPUT_PORCELAIN;
 		else if (!strcmp("--", arg)) {
 			seen_dashdash = 1;
 			i++;
Next: Junio C Hamano
Message 1 of 82 in “Convert 'git blame' to parse_options()”
  1. Linus TorvaldsJun 23, 2008
  2. Junio C HamanoJun 23, 2008
  3. Johannes SchindelinJun 23, 2008
  4. [RFC] Re: Convert 'git blame' to parse_options()Pierre Habouzit, Jun 23, 2008
  5. Johannes SchindelinJun 23, 2008
  6. Pierre HabouzitJun 23, 2008
  7. Johannes SchindelinJun 23, 2008
  8. Linus TorvaldsJun 23, 2008
  9. Jeff KingJun 23, 2008
  10. Linus TorvaldsJun 23, 2008
  11. Jeff KingJun 23, 2008
  12. Linus TorvaldsJun 23, 2008
  13. Jeff KingJun 23, 2008
  14. Linus TorvaldsJun 23, 2008
  15. Linus TorvaldsJun 23, 2008
  16. Jeff KingJun 23, 2008
  17. Linus TorvaldsJun 23, 2008
  18. Linus TorvaldsJun 23, 2008
  19. Pierre HabouzitJun 23, 2008
  20. parse-opt: have parse_options_{start,end}.Pierre Habouzit, Jun 23, 2008
  21. parse-opt: Export a non NORETURN usage dumper.Pierre Habouzit, Jun 23, 2008
  22. parse-opt: create parse_options_step.Pierre Habouzit, Jun 23, 2008
  23. parse-opt: do not pring errors on unknown options, return -2 intead.Pierre Habouzit, Jun 23, 2008
  24. parse-opt: fake short strings for callers to believe in.Pierre Habouzit, Jun 23, 2008
  25. Junio C HamanoJun 23, 2008
  26. Pierre HabouzitJun 23, 2008
  27. Pierre HabouzitJun 23, 2008
  28. Junio C HamanoJun 23, 2008
  29. Pierre HabouzitJun 23, 2008
  30. Linus TorvaldsJun 23, 2008
  31. Linus TorvaldsJun 23, 2008
  32. Pierre HabouzitJun 23, 2008
  33. Junio C HamanoJun 23, 2008
  34. Pierre HabouzitJun 23, 2008
  35. Pierre HabouzitJun 23, 2008
  36. Junio C HamanoJun 23, 2008
  37. Pierre HabouzitJun 23, 2008
  38. Linus TorvaldsJun 23, 2008
  39. Junio C HamanoJun 23, 2008
  40. Pierre HabouzitJun 24, 2008
  41. Jeff KingJun 24, 2008
  42. Jeff KingJun 23, 2008
  43. Pierre HabouzitJun 23, 2008
  44. Linus TorvaldsJun 23, 2008
  45. Jeff KingJun 24, 2008
  46. Linus TorvaldsJun 24, 2008
  47. Johannes SchindelinJun 24, 2008
  48. Jeff KingJun 24, 2008
  49. Linus TorvaldsJun 24, 2008
  50. Jeff KingJun 24, 2008
  51. Junio C HamanoJun 24, 2008
  52. Pierre HabouzitJun 24, 2008
  53. Linus TorvaldsJun 24, 2008
  54. Pierre HabouzitJun 24, 2008
  55. Pierre HabouzitJun 24, 2008
  56. Johannes SixtJun 25, 2008
  57. Johannes SchindelinJun 23, 2008
  58. Linus TorvaldsJun 23, 2008
  59. Johannes SchindelinJun 23, 2008
  60. Jeff KingJun 23, 2008
  61. Johannes SchindelinJun 23, 2008
  62. Pierre HabouzitJun 23, 2008
  63. Linus TorvaldsJun 23, 2008
  64. Making parse-opt incremental, reworked seriesPierre Habouzit, Jun 24, 2008
  65. 1/7 parse-opt: have parse_options_{start,end}.Pierre Habouzit, Jun 24, 2008
  66. 2/7 parse-opt: Export a non NORETURN usage dumper.Pierre Habouzit, Jun 24, 2008
  67. 3/7 parse-opt: create parse_options_step.Pierre Habouzit, Jun 24, 2008
  68. 4/7 parse-opt: do not pring errors on unknown options, return -2 intead.Pierre Habouzit, Jun 24, 2008
  69. 5/7 parse-opt: fake short strings for callers to believe in.Pierre Habouzit, Jun 24, 2008
  70. 6/7 parse-opt: add PARSE_OPT_KEEP_ARGV0 parser option.Pierre Habouzit, Jun 24, 2008
  71. 7/7 Migrate git-blame to parse-option partially.Pierre Habouzit, Jun 24, 2008
  72. Pierre HabouzitJun 24, 2008
  73. Linus TorvaldsJun 24, 2008
  74. Pierre HabouzitJun 24, 2008
  75. Pierre HabouzitJun 24, 2008
  76. Linus TorvaldsJun 24, 2008
  77. Pierre HabouzitJun 24, 2008
  78. Andreas EricssonJun 25, 2008
  79. parse-opt: fake short strings for callers to believe in.Pierre Habouzit, Jun 24, 2008
  80. Pierre HabouzitJun 26, 2008
  81. Junio C HamanoJun 26, 2008
  82. Pierre HabouzitJun 26, 2008

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.