Re: [RFC/PATCH] revision.c: add --format option for 'git log'

Felipe Contreras [off-list ref] writes:

quoted

quoted

People already are used to finding the shed in the scenery by looking for
that original color, however ugly the color might be.  The answer to your
question has to become quite different when you take that into account;
otherwise you are being irresponsible to your users.

People somehow got used to the ugly color, they'll get used to the
pretty one, in fact, they would probably like it better, and maybe
even thought more than once on changing it.

If they're the kind of people that don't like new things they'll
probably not be using git anyway.

You do not have to send two messages in a row to reaffirm that you are of
irresponsible kind.  I heard you enough already.

Go away.

Quoting Junio C Hamano [off-list ref]:

quoted

quoted

quoted

People already are used to finding the shed in the scenery by looking for
that original color, however ugly the color might be.  The answer to your
question has to become quite different when you take that into account;
otherwise you are being irresponsible to your users.

People somehow got used to the ugly color, they'll get used to the
pretty one, in fact, they would probably like it better, and maybe
even thought more than once on changing it.

If they're the kind of people that don't like new things they'll
probably not be using git anyway.

You do not have to send two messages in a row to reaffirm that you are of
irresponsible kind.  I heard you enough already.

Go away.

Junio, what got into you?

I've always admired your calm and reasoned way to deal with even the most obnoxious people, and unlike more abrasive people on this list I've never seen you say "Go away" to anybody here.

Especially because I agree with you that calling pretty-printing as "pretty" isn't so broken to make such a big deal out of, it would be better not to chase a potentially useful contributor away on such a minor issue.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Some people prefer to call the pretty-print styles "format", and get
annoyed to see "git log --format=short" fail.  Introduce it as a synonym
to --pretty so that both can be used interchangeably without breaking
examples in existing web pages or ppeople's expectations.

Having to say --format="format:%h %s" is redundant because none of the
predefined pretty-print styles have per-cent sign in it, so this patch
also makes it possible to say --pretty="%h %s" (and --format="%h %s").

Signed-off-by: Nanako Shiraishi <redacted>
---

I think doing it this way won't risk any compatibility issue.
Please be gentle; this is my first real patch in the C language.

 Documentation/pretty-formats.txt |    9 +++++++++
 Documentation/pretty-options.txt |    1 +
 pretty.c                         |   20 ++++++++++++++------
 revision.c                       |    2 +-
 4 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 159390c..5c6e678 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt

@@ -152,3 +152,12 @@ $ git log -2 --pretty=tformat:%h 4da45bef \
 4da45be
 7134973
 ---------------------
++
+In addition, any unrecognized string that has a `%` in it is interpreted
+as if it has `tformat:` in front of it.  For example, these two are
+equivalent:
++
+---------------------
+$ git log -2 --pretty=tformat:%h 4da45bef
+$ git log -2 --pretty=%h 4da45bef
+---------------------

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 5f21efe..6596019 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt

@@ -1,4 +1,5 @@
 --pretty[='<format>']::
+--format[='<format>']::
 
 	Pretty-print the contents of the commit logs in a given format,
 	where '<format>' can be one of 'oneline', 'short', 'medium',

diff --git a/pretty.c b/pretty.c
index 6cd9149..d739f6d 100644
--- a/pretty.c
+++ b/pretty.c

@@ -10,6 +10,15 @@
 
 static char *user_format;
 
+static void save_user_format(struct rev_info *rev, const char *cp, int is_tformat)
+{
+	free(user_format);
+	user_format = xstrdup(cp);
+	if (is_tformat)
+		rev->use_terminator = 1;
+	rev->commit_format = CMIT_FMT_USERFORMAT;
+}
+
 void get_commit_format(const char *arg, struct rev_info *rev)
 {
 	int i;

@@ -33,12 +42,7 @@ void get_commit_format(const char *arg, struct rev_info *rev)
 		return;
 	}
 	if (!prefixcmp(arg, "format:") || !prefixcmp(arg, "tformat:")) {
-		const char *cp = strchr(arg, ':') + 1;
-		free(user_format);
-		user_format = xstrdup(cp);
-		if (arg[0] == 't')
-			rev->use_terminator = 1;
-		rev->commit_format = CMIT_FMT_USERFORMAT;
+		save_user_format(rev, strchr(arg, ':') + 1, arg[0] == 't');
 		return;
 	}
 	for (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {

@@ -50,6 +54,10 @@ void get_commit_format(const char *arg, struct rev_info *rev)
 			return;
 		}
 	}
+	if (strchr(arg, '%')) {
+		save_user_format(rev, arg, 1);
+		return;
+	}
 
 	die("invalid --pretty format: %s", arg);
 }

diff --git a/revision.c b/revision.c
index 286e416..556c319 100644
--- a/revision.c
+++ b/revision.c

@@ -1144,7 +1144,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--pretty")) {
 		revs->verbose_header = 1;
 		get_commit_format(arg+8, revs);
-	} else if (!prefixcmp(arg, "--pretty=")) {
+	} else if (!prefixcmp(arg, "--pretty=") || !prefixcmp(arg, "--format=")) {
 		revs->verbose_header = 1;
 		get_commit_format(arg+9, revs);
 	} else if (!strcmp(arg, "--graph")) {

-- 
1.6.2.rc1

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

On Tue, Feb 24, 2009 at 01:06:26PM +0900, Nanako Shiraishi wrote:

Some people prefer to call the pretty-print styles "format", and get
annoyed to see "git log --format=short" fail.  Introduce it as a synonym
to --pretty so that both can be used interchangeably without breaking
examples in existing web pages or ppeople's expectations.

Thinking about this in context of the proposal to support --oneline (et
al), I think this part by itself gives confusing behavior. That is,
--pretty=oneline can be shortened to --oneline, but --pretty=format:$x
cannot be shortened to --format=$x.

But that is modified by what happens next:

Having to say --format="format:%h %s" is redundant because none of the
predefined pretty-print styles have per-cent sign in it, so this patch
also makes it possible to say --pretty="%h %s" (and --format="%h %s").

This implies that --format=$x is equivalent to --pretty=format:$x, but
the patch actually implements the equivalent of --pretty=tformat:$x.

So that raises two concerns:

  1. We have to pick one as the "most common" for this shorthand; are we
     sure tformat is it? (Personally, I think it is, but I think it is a
     subtle point which we should be sure of).

  2. This _almost_ fixes the point I raised above. That is, --format=$x
     would match its longer --pretty=format:$x counterpart. Except that
     --format does _tformat_, which I would have expected to get via
     --tformat under such a proposal.

-Peff

On Tue, Feb 24, 2009 at 3:33 AM, Junio C Hamano [off-list ref] wrote:

Felipe Contreras [off-list ref] writes:

quoted

quoted

quoted

People already are used to finding the shed in the scenery by looking for
that original color, however ugly the color might be.  The answer to your
question has to become quite different when you take that into account;
otherwise you are being irresponsible to your users.

People somehow got used to the ugly color, they'll get used to the
pretty one, in fact, they would probably like it better, and maybe
even thought more than once on changing it.

If they're the kind of people that don't like new things they'll
probably not be using git anyway.

You do not have to send two messages in a row to reaffirm that you are of
irresponsible kind.  I heard you enough already.

Go away.

After sending the email I realized I didn't answer your argument about
people already used to the existing stuff.

I might sound too aggressive on email, but my intention is not to
attack anything or anybody, just improve things.

-- 
Felipe Contreras