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

Common option parsing..

From
Linus Torvalds <torvalds@osdl.org>
Date
Apr 13, 2006, 01:48 UTC
Message-ID
<Pine.LNX.4.64.0604121828370.14565@g5.osdl.org>
Junio,
 right now we actually haev very consistent command line options for git, 
but we have two (and in your "next" branch, three) different structures 
that they get parsed into, and lots of it is duplicated. We have 
"diff_options", "rev_info" and now "log_info".

To make matters worse, some things aren't actually in any of them, ie "--cc", "--abbrev" and friends actually end up being parsed into their own private flags in diff-files. Some are in _both_ rev_info and diff_options (the "--pretty" parsing), because both diff and rev-parse supported that option set.

And almost all commands that take any of those options at all end up actually taking the combination of them these days. Yeah, git-rev-parse doesn't, but quite frankly, with your "git log --diff" changes, that's actually the odd man out, and I think we should just make git-rev-parse basically do it too.

And some things, like doing a builtin "git diff" would actually be quite easy to do, except for the fact that having three different option parsers _and_ having some options you parse by hand on top of that is just crazy ("git diff" wants even the stage diff flags that git-diff-files takes).

The easiest way to just solve all this mess would be to 
 - add the diff-options into "struct rev_list" and make the 
   "setup_revisions()" parser parse the diff flags too.
 - get rid of "log_info" and "diff_options"
 - possibly rename the resulting super-options structure as "struct 
   git_options" or something if we want to.

At that point, it would become a lot easier to do things like a built-in "git diff", where command line parsing really is the biggest deal. It would become something like this:

	struct rev_info revs;
	struct commit *src, *dst;
	if (setup_revisions(&revs))
		die(git_diff_usage);
	/* No revision arguments: git-diff-files */
	if (!revs->commits)
		return diff_files(&revs);
	src = revs->commits->item;
	revs->commits = revs->commits->next;
	/* Just one rev: git-diff-index against that */
	if (!revs->commits)
		return diff_index(&revs, src);
	dst = revs->commits->item;
	revs->commits = revs->commits->next;
	/*
	 * More than two revs? Maybe that means a combined diff?
	 * Some day.. In the meantime, just make it an error.
	 */
	if (revs->commits->next)
		die(git_diff_usage);
	/*
	 * If it was "a..b" using git-rev-parse, the second commit
	 * on the list is the initial (and uninteresting) one, we
	 * need to make that the source..
	 */
	if (dst->object.flags & UNINTERESTING) {
		struct commit *tmp = dst;
		dst = src;
		src = tmp;
	}
	return diff_trees(&revs, src, dst);

where obviously we'd need to do some minor moving-around to make "diff_files()" be a function interface, but I actually did that, and it was really trivial. The bigger part would be to just change the structures around (which could be done first as a fairly big but trivial patch, kind of the same way the initial "struct rev_info" was done when the "revision.c" file was split away).

What do you think?
		Linus
Next: Junio C Hamano
Message 1 of 3 in “Common option parsing..”
  1. Linus TorvaldsApr 13, 2006
  2. Junio C HamanoApr 13, 2006
  3. Linus TorvaldsApr 13, 2006

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.