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

[RFH] convert shortlog to use parse_options

From
Jeff King <peff@peff.net>
Date
Dec 13, 2007, 05:52 UTC
Message-ID
<20071213055226.GA3636@coredump.intra.peff.net>
Pierre,

I am converting shortlog to use parse_options, but I have hit a behavior I can't seem to represent.

The "-w" option has an optional argument, and we used to accept only "-w" or "-wX,Y,Z". However, parse_options allows "-w X,Y,Z".

This means that "-w HEAD" used to mean "turn on wrapping with default parameters, look at HEAD" and now translates to "-w with parameter HEAD".

Is there a way to do this currently, or can we add an option flag?
---
 builtin-shortlog.c |  111 +++++++++++++++++++++++++++++-----------------------
 1 files changed, 62 insertions(+), 49 deletions(-)
diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index 3d8d709..37fdb45 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -6,9 +6,12 @@
 #include "revision.h"
 #include "utf8.h"
 #include "mailmap.h"
+#include "parse-options.h"
 
-static const char shortlog_usage[] =
-"git-shortlog [-n] [-s] [<commit-id>... ]";
+static const char * const shortlog_usage[] = {
+	"git-shortlog [-n] [-s] [-e] [<commit-id>... ]",
+	NULL
+};
 
 static char *common_repo_prefix;
 static int email;
@@ -181,34 +184,45 @@ static int parse_uint(char const **arg, int comma)
 }
 
 static const char wrap_arg_usage[] = "-w[<width>[,<indent1>[,<indent2>]]]";
+struct wrap_arg {
+	int enabled;
+	int len;
+	int in1;
+	int in2;
+};
 #define DEFAULT_WRAPLEN 76
 #define DEFAULT_INDENT1 6
 #define DEFAULT_INDENT2 9
 
-static void parse_wrap_args(const char *arg, int *in1, int *in2, int *wrap)
+static int parse_wrap(const struct option *opt, const char *arg, int unset)
 {
-	arg += 2; /* skip -w */
+	struct wrap_arg *wrap = opt->value;
 
-	*wrap = parse_uint(&arg, ',');
-	if (*wrap < 0)
+	wrap->enabled = 1;
+
+	if (!arg)
+		return 0;
+	wrap->len = parse_uint(&arg, ',');
+	if (wrap->len < 0)
 		die(wrap_arg_usage);
-	*in1 = parse_uint(&arg, ',');
-	if (*in1 < 0)
+	wrap->in1 = parse_uint(&arg, ',');
+	if (wrap->in1 < 0)
 		die(wrap_arg_usage);
-	*in2 = parse_uint(&arg, '\0');
-	if (*in2 < 0)
+	wrap->in2 = parse_uint(&arg, '\0');
+	if (wrap->in2 < 0)
 		die(wrap_arg_usage);
 
-	if (!*wrap)
-		*wrap = DEFAULT_WRAPLEN;
-	if (!*in1)
-		*in1 = DEFAULT_INDENT1;
-	if (!*in2)
-		*in2 = DEFAULT_INDENT2;
-	if (*wrap &&
-	    ((*in1 && *wrap <= *in1) ||
-	     (*in2 && *wrap <= *in2)))
+	if (!wrap->len)
+		wrap->len = DEFAULT_WRAPLEN;
+	if (!wrap->in1)
+		wrap->in1 = DEFAULT_INDENT1;
+	if (!wrap->in2)
+		wrap->in2 = DEFAULT_INDENT2;
+	if (wrap->len &&
+	    ((wrap->in1 && wrap->len <= wrap->in1) ||
+	     (wrap->in2 && wrap->len <= wrap->in2)))
 		die(wrap_arg_usage);
+	return 0;
 }
 
 int cmd_shortlog(int argc, const char **argv, const char *prefix)
@@ -216,34 +230,31 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 	struct rev_info rev;
 	struct path_list list = { NULL, 0, 0, 1 };
 	int i, j, sort_by_number = 0, summary = 0;
-	int wrap_lines = 0;
-	int wrap = DEFAULT_WRAPLEN;
-	int in1 = DEFAULT_INDENT1;
-	int in2 = DEFAULT_INDENT2;
-
-	/* since -n is a shadowed rev argument, parse our args first */
-	while (argc > 1) {
-		if (!strcmp(argv[1], "-n") || !strcmp(argv[1], "--numbered"))
-			sort_by_number = 1;
-		else if (!strcmp(argv[1], "-s") ||
-				!strcmp(argv[1], "--summary"))
-			summary = 1;
-		else if (!strcmp(argv[1], "-e") ||
-			 !strcmp(argv[1], "--email"))
-			email = 1;
-		else if (!prefixcmp(argv[1], "-w")) {
-			wrap_lines = 1;
-			parse_wrap_args(argv[1], &in1, &in2, &wrap);
-		}
-		else if (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help"))
-			usage(shortlog_usage);
-		else
-			break;
-		argv++;
-		argc--;
-	}
+	struct wrap_arg wrap = {
+		0,
+		DEFAULT_WRAPLEN,
+		DEFAULT_INDENT1,
+		DEFAULT_INDENT2
+	};
+	struct option options[] = {
+		OPT_BOOLEAN('n', "numbered", &sort_by_number, "sort by number"),
+		OPT_BOOLEAN('s', "summary", &summary,
+				"show commit count summary"),
+		OPT_BOOLEAN('e', "email", &email, "show email addresses"),
+		{
+			OPTION_CALLBACK, 'w', NULL, &wrap, "wrap",
+			"line-wrap commit message", PARSE_OPT_OPTARG,
+			parse_wrap
+		},
+	};
+
+	argc = parse_options(argc, argv, options, shortlog_usage, 0);
+
 	init_revisions(&rev, prefix);
-	argc = setup_revisions(argc, argv, &rev, NULL);
+	/* setup_revisions expects to have the program name in argv[0],
+	 * but parse_options has removed it. We have to do parse_options
+	 * before setup_revisions, though, to claim "-n". */
+	argc = setup_revisions(argc+1, argv-1, &rev, NULL);
 	if (argc > 1)
 		die ("unrecognized argument: %s", argv[1]);
 
@@ -272,9 +283,11 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 			for (j = onelines->nr - 1; j >= 0; j--) {
 				const char *msg = onelines->items[j].path;
 
-				if (wrap_lines) {
-					int col = print_wrapped_text(msg, in1, in2, wrap);
-					if (col != wrap)
+				if (wrap.enabled) {
+					int col = print_wrapped_text(msg,
+							wrap.in1, wrap.in2,
+							wrap.len);
+					if (col != wrap.len)
 						putchar('\n');
 				}
 				else
-- 
1.5.4.rc0.1088.gfa79c-dirty
Next: Pierre Habouzit
Message 1 of 51 in “[RFH] convert shortlog to use parse_options”
  1. Jeff KingDec 13, 2007
  2. Pierre HabouzitDec 13, 2007
  3. Jeff KingDec 13, 2007
  4. Pierre HabouzitDec 13, 2007
  5. 1/2 parseopt: Enforce the use of the sticked form for optional arguments.Pierre Habouzit, Dec 13, 2007
  6. 2/2 parseopt: Add a gitcli(5) man page.Pierre Habouzit, Dec 13, 2007
  7. Wincent ColaiutaDec 13, 2007
  8. Pierre HabouzitDec 13, 2007
  9. (squashme) gitcli documentation fixupsJunio C Hamano, Dec 17, 2007
  10. Pierre HabouzitDec 17, 2007
  11. Pierre HabouzitDec 17, 2007
  12. builtin-tag: fix fallouts from recent parsopt restriction.Junio C Hamano, Dec 17, 2007
  13. Pierre HabouzitDec 17, 2007
  14. Junio C HamanoDec 17, 2007
  15. Pierre HabouzitDec 17, 2007
  16. Junio C HamanoDec 17, 2007
  17. Pierre HabouzitDec 17, 2007
  18. Junio C HamanoDec 17, 2007
  19. Jeff KingDec 17, 2007
  20. Pierre HabouzitDec 17, 2007
  21. Pierre HabouzitDec 17, 2007
  22. Jeff KingDec 17, 2007
  23. Pierre HabouzitDec 17, 2007
  24. Junio C HamanoDec 17, 2007
  25. Jeff KingDec 17, 2007
  26. Pierre HabouzitDec 17, 2007
  27. Pierre HabouzitDec 17, 2007
  28. Junio C HamanoDec 17, 2007
  29. Pierre HabouzitDec 17, 2007
  30. Pierre HabouzitDec 17, 2007
  31. Junio C HamanoDec 13, 2007
  32. Pierre HabouzitDec 13, 2007
  33. Pierre HabouzitDec 13, 2007
  34. Junio C HamanoDec 13, 2007
  35. Kristian HøgsbergDec 13, 2007
  36. Kristian HøgsbergDec 13, 2007
  37. Junio C HamanoDec 13, 2007
  38. Jeff KingDec 14, 2007
  39. Junio C HamanoDec 14, 2007
  40. Andreas EricssonDec 14, 2007
  41. Pierre HabouzitDec 14, 2007
  42. Junio C HamanoDec 14, 2007
  43. Pierre HabouzitDec 15, 2007
  44. Junio C HamanoDec 17, 2007
  45. Andreas EricssonDec 17, 2007
  46. Junio C HamanoDec 17, 2007
  47. Andreas EricssonDec 17, 2007
  48. Kristian HøgsbergDec 17, 2007
  49. Junio C HamanoDec 13, 2007
  50. Pierre HabouzitDec 13, 2007
  51. Pierre HabouzitDec 13, 2007

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.