Re: [PATCH v4 3/3] pretty: add aliases for pretty formats

From

Jonathan Nieder <jrnieder@gmail.com>

Date

May 2, 2010, 12:30 UTC

Message-ID

<20100502123039.GB13419@progeny.tock>

In-Reply-To

<1272798044-10487-4-git-send-email-wmpalmer@gmail.com>

Will Palmer wrote:

> Signed-off-by: Will Palmer <wmpalmer@gmail.com>

[...]

> +pretty.<name>::
> +	Alias for a --pretty= format string, as specified in
> +	linkgit:git-log[1]. Any aliases defined here can be used just
> +	as the built-in pretty formats could. For example, defining
> +	"pretty.hash = format:%H" would cause the invocation
> +	"git log --pretty=hash" to be equivalent to running
> +	"git log --pretty=format:%H".

	For example, running `git config pretty.changelog "format:* %H %s"`
	would cause the invocation `git log --pretty=changelog` to be
	equivalent to running `git log '--pretty=format:* %H %s'`.

> +static int git_pretty_formats_config(const char *var, const char *value, void *cb)
> +{
> +	struct cmt_fmt_map *commit_format = NULL;
> +	const char *name;
> +	const char *fmt;
> +	int i;
> +
> +	if (prefixcmp(var, "pretty."))
> +		return 0;
> +
> +	name = &var[7];

Can this magic number be avoided?  Maybe

	name = &var[strlen("pretty.")];

> +++ b/t/t4205-log-pretty-formats.sh

[...]

> +test_expect_code 128 'alias non-existant format' '
> +	git config pretty.test-alias format-that-will-never-exist &&
> +	git log --pretty=test-alias'

This will succeed if the ‘git config’ fails.  Why not use

	test_expect_success 'alias non-existent format' '
		git config ... &&
		test_must_fail git log --pretty=test-alias
	'

or something like

	test_exit_status() {
		test "${2+set}" || return 1
		code=$1
		shift
		eval "$*"
		test $code -eq $?
	}

	test_expect_success 'alias non-existent format' '
		git config ... &&
		test_exit_status 128 git log --pretty=test-alias
	'

?

[...]

> +test_expect_code 128 'alias loop' '
> +	git config pretty.test-foo test-bar &&
> +	git config pretty.test-bar test-foo &&
> +	git log --pretty=test-foo'

Likewise.

With whatever subset of the above changes you deem suitable,

  Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks again.

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 85d5b90..03f2a29 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1469,11 +1469,12 @@ pager.<cmd>::
 pretty.<name>::
 	Alias for a --pretty= format string, as specified in
 	linkgit:git-log[1]. Any aliases defined here can be used just
-	as the built-in pretty formats could. For example, defining
-	"pretty.hash = format:%H" would cause the invocation
-	"git log --pretty=hash" to be equivalent to running
-	"git log --pretty=format:%H". Note that an alias with the same
-	name as a built-in format will be silently ignored.
+	as the built-in pretty formats could. For example,
+	running `git config pretty.changelog "format:* %H %s"`
+	would cause the invocation `git log --pretty=changelog`
+	to be equivalent to running `git log '--pretty=format:* %H %s'`.
+	Note that an alias with the same name as a built-in format
+	will be silently ignored.
 
 pull.octopus::
 	The default merge strategy to use when pulling multiple branches
diff --git a/pretty.c b/pretty.c
index 1c16bf8..9e3b26b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -42,7 +42,7 @@ static int git_pretty_formats_config(const char *var, const char *value, void *c
 	if (prefixcmp(var, "pretty."))
 		return 0;
 
-	name = &var[7];
+	name = &var[strlen("pretty.")];
 	for (i = 0; i < builtin_formats_len; i++) {
 		if (!strcmp(commit_formats[i].name, name))
 			return 0;
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index af96984..cb9f2bd 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -14,53 +14,61 @@ test_expect_success 'set up basic repos' '
 	git commit -m initial &&
 	git add bar &&
 	test_tick &&
-	git commit -m "add bar"'
+	git commit -m "add bar"
+'
 
 test_expect_success 'alias builtin format' '
 	git log --pretty=oneline >expected &&
 	git config pretty.test-alias oneline &&
 	git log --pretty=test-alias >actual &&
-	test_cmp expected actual'
+	test_cmp expected actual
+'
 
 test_expect_success 'alias masking builtin format' '
 	git log --pretty=oneline >expected &&
 	git config pretty.oneline "%H" &&
 	git log --pretty=oneline >actual &&
-	test_cmp expected actual'
+	test_cmp expected actual
+'
 
 test_expect_success 'alias user-defined format' '
 	git log --pretty="format:%h" >expected &&
 	git config pretty.test-alias "format:%h" &&
 	git log --pretty=test-alias >actual &&
-	test_cmp expected actual'
+	test_cmp expected actual
+'
 
 test_expect_success 'alias user-defined tformat' '
 	git log --pretty="tformat:%h" >expected &&
 	git config pretty.test-alias "tformat:%h" &&
 	git log --pretty=test-alias >actual &&
-	test_cmp expected actual'
+	test_cmp expected actual
+'
 
-test_expect_code 128 'alias non-existant format' '
+test_expect_success 'alias non-existant format' '
 	git config pretty.test-alias format-that-will-never-exist &&
-	git log --pretty=test-alias'
+	test_must_fail git log --pretty=test-alias
+'
 
 test_expect_success 'alias of an alias' '
 	git log --pretty="tformat:%h" >expected &&
 	git config pretty.test-foo "tformat:%h" &&
 	git config pretty.test-bar test-foo &&
-	git log --pretty=test-bar >actual &&
-	test_cmp expected actual'
+	git log --pretty=test-bar >actual && test_cmp expected actual
+'
 
 test_expect_success 'alias masking an alias' '
 	git log --pretty=format:"Two %H" >expected &&
 	git config pretty.duplicate "format:One %H" &&
 	git config --add pretty.duplicate "format:Two %H" &&
 	git log --pretty=duplicate >actual &&
-	test_cmp expected actual'
+	test_cmp expected actual
+'
 
-test_expect_code 128 'alias loop' '
+test_expect_success 'alias loop' '
 	git config pretty.test-foo test-bar &&
 	git config pretty.test-bar test-foo &&
-	git log --pretty=test-foo'
+	test_must_fail git log --pretty=test-foo
+'
 
 test_done