[PATCH 0/4] cat-file: optionally convert to worktree version

[PATCH 0/4] cat-file: optionally convert to worktree version

[Johannes Schindelin]

When third-party tools need to access to contents of blobs in the
database, they might be more interested in the worktree version than in
the "clean" version of said contents.

This branch introduces the --filters option to make that happen, the
--use-path option to provide the path separately if the blob name rather
than the tree name is availale, and offers batch support in which case
it expects the object names and the path on its input lines, separated
by white space.

The new --filters option is an obvious sibling of --textconv, and shares
the peculiar feature that the drivers (and end-of-line convention) are
determined from the current worktree, not from the attributes stored in
the revision that may have been part of the object name.

As --textconv is so similar to --filters, it was taught to understand
the --use-path option and it was made compatible with batch mode, too.

I briefly considered teaching the batch mode to extract the path from
object names if they are specified as <tree-ish>:<path>. The changes
would be quite intrusive, though, and uglify the code substanitially. So
I decided against that.


Johannes Schindelin (4):
  cat-file: fix a grammo in the man page
  cat-file: introduce the --filters option
  cat-file --textconv/--filters: allow specifying the path separately
  cat-file: support --textconv/--filters in batch mode

 Documentation/git-cat-file.txt |  40 ++++++++++++----
 builtin/cat-file.c             | 106 +++++++++++++++++++++++++++++++++++++----
 t/t8010-cat-file-filters.sh    |  57 ++++++++++++++++++++++
 3 files changed, 185 insertions(+), 18 deletions(-)
 create mode 100755 t/t8010-cat-file-filters.sh

Published-As: https://github.com/dscho/git/releases/tag/cat-file-filters-v1
Fetch-It-Via: git fetch https://github.com/dscho/git cat-file-filters-v1

-- 
2.9.2.691.g78954f3

base-commit: d63263a4dee8fc7da9b97bbdedf9c0d1f33024d4

[PATCH 1/4] cat-file: fix a grammo in the man page

[Johannes Schindelin]

"... has be ..." -> "... has to be ..."

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-cat-file.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 18d03d8..071029b 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -54,8 +54,9 @@ OPTIONS
 
 --textconv::
 	Show the content as transformed by a textconv filter. In this case,
-	<object> has be of the form <tree-ish>:<path>, or :<path> in order
-	to apply the filter to the content recorded in the index at <path>.
+	<object> has to be of the form <tree-ish>:<path>, or :<path> in
+	order to apply the filter to the content recorded in the index at
+	<path>.
 
 --batch::
 --batch=<format>::
-- 
2.9.2.691.g78954f3

[PATCH 2/4] cat-file: introduce the --filters option

[Johannes Schindelin]

As suggested by its name, the --filters option applies the filters
that are currently configured for the specified path.

This feature comes in handy when a 3rd-party tool wants to work with
the contents of files from past revisions as if they had been checked
out, but without detouring via temporary files.

Note that we ensure that symbolic links are unaffected (we know from
looking at the mode whether it refers to a symbolic link or a regular
file).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-cat-file.txt | 12 +++++++++---
 builtin/cat-file.c             | 41 ++++++++++++++++++++++++++++++++++++++++-
 t/t8010-cat-file-filters.sh    | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 4 deletions(-)
 create mode 100755 t/t8010-cat-file-filters.sh

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 071029b..7d48735 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -9,15 +9,15 @@ git-cat-file - Provide content or type and size information for repository objec
 SYNOPSIS
 --------
 [verse]
-'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv ) <object>
+'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) <object>
 'git cat-file' (--batch | --batch-check) [--follow-symlinks]
 
 DESCRIPTION
 -----------
 In its first form, the command provides the content or the type of an object in
 the repository. The type is required unless `-t` or `-p` is used to find the
-object type, or `-s` is used to find the object size, or `--textconv` is used
-(which implies type "blob").
+object type, or `-s` is used to find the object size, or `--textconv` or
+`--filters` is used (which imply type "blob").
 
 In the second form, a list of objects (separated by linefeeds) is provided on
 stdin, and the SHA-1, type, and size of each object is printed on stdout.
@@ -58,6 +58,12 @@ OPTIONS
 	order to apply the filter to the content recorded in the index at
 	<path>.
 
+--filters::
+	Show the content as transformed by the filters configured in
+	the current working tree for the given <path> (i.e. smudge filters,
+	end-of-line conversion, etc). In this case, <object> has to be of
+	the form <tree-ish>:<path>, or :<path>.
+
 --batch::
 --batch=<format>::
 	Print object information and contents for each object provided
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 2dfe626..0b74afa 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,6 +20,33 @@ struct batch_options {
 	const char *format;
 };
 
+static int filter_object(const char *path, unsigned mode,
+			 const unsigned char *sha1,
+			 char **buf, unsigned long *size)
+{
+	enum object_type type;
+
+	*buf = read_sha1_file(sha1, &type, size);
+	if (!*buf)
+		return error(_("cannot read object %s '%s'"),
+			sha1_to_hex(sha1), path);
+	if (type != OBJ_BLOB) {
+		free(*buf);
+		return error(_("blob expected for %s '%s'"),
+			sha1_to_hex(sha1), path);
+	}
+	if (S_ISREG(mode)) {
+		struct strbuf strbuf = STRBUF_INIT;
+		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
+			free(*buf);
+			*size = strbuf.len;
+			*buf = strbuf_detach(&strbuf, NULL);
+		}
+	}
+
+	return 0;
+}
+
 static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 			int unknown_type)
 {
@@ -61,6 +88,16 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	case 'e':
 		return !has_sha1_file(sha1);
 
+	case 'w':
+		if (!obj_context.path[0])
+			die("git cat-file --filters %s: <object> must be "
+			    "<sha1:path>", obj_name);
+
+		if (filter_object(obj_context.path, obj_context.mode,
+				  sha1, &buf, &size))
+			return -1;
+		break;
+
 	case 'c':
 		if (!obj_context.path[0])
 			die("git cat-file --textconv %s: <object> must be <sha1:path>",
@@ -440,7 +477,7 @@ static int batch_objects(struct batch_options *opt)
 }
 
 static const char * const cat_file_usage[] = {
-	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv) <object>"),
+	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) <object>"),
 	N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
 	NULL
 };
@@ -486,6 +523,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE('p', NULL, &opt, N_("pretty-print object's content"), 'p'),
 		OPT_CMDMODE(0, "textconv", &opt,
 			    N_("for blob objects, run textconv on object's content"), 'c'),
+		OPT_CMDMODE(0, "filters", &opt,
+			    N_("for blob objects, run filters on object's content"), 'w'),
 		OPT_BOOL(0, "allow-unknown-type", &unknown_type,
 			  N_("allow -s and -t to work with broken/corrupt objects")),
 		OPT_BOOL(0, "buffer", &batch.buffer_output, N_("buffer --batch output")),
diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
new file mode 100755
index 0000000..e466634
--- /dev/null
+++ b/t/t8010-cat-file-filters.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='git cat-file filters support'
+. ./test-lib.sh
+
+test_expect_success 'setup ' '
+	echo "*.txt eol=crlf diff=txt" >.gitattributes &&
+	echo "hello" | append_cr >world.txt &&
+	git add .gitattributes world.txt &&
+	test_tick &&
+	git commit -m "Initial commit"
+'
+
+has_cr () {
+	tr '\015' Q <"$1" | grep Q >/dev/null
+}
+
+test_expect_success 'no filters with `git show`' '
+	git show HEAD:world.txt >actual &&
+	! has_cr actual
+
+'
+
+test_expect_success 'no filters with cat-file' '
+	git cat-file blob HEAD:world.txt >actual &&
+	! has_cr actual
+'
+
+test_expect_success 'cat-file --filters converts to worktree version' '
+	git cat-file --filters HEAD:world.txt >actual &&
+	has_cr actual
+'
+
+test_done
-- 
2.9.2.691.g78954f3

[PATCH 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Johannes Schindelin]

There are circumstances when it is relatively easy to figure out the
object name for a given path, but not the revision. For example, when
looking at a diff generated by Git, the object names are recorded, but
not the revision. As a matter of fact, the revisions from which the diff
was generated may not even exist locally.

In such a case, the user would have to generate a fake revision just to
be able to use --textconv or --filters.

Let's simplify this dramatically, because we do not really need that
revision at all: all we care about is that we know the path. In the
scenario described above, we do know the path, and we just want to
specify it separately from the object name.

Example usage:

	git cat-file --textconv --use-path=main.c 0f1937fd

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-cat-file.txt |  7 ++++++-
 builtin/cat-file.c             | 22 +++++++++++++++++-----
 t/t8010-cat-file-filters.sh    | 13 +++++++++++++
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 7d48735..59a3c37 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -9,7 +9,7 @@ git-cat-file - Provide content or type and size information for repository objec
 SYNOPSIS
 --------
 [verse]
-'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) <object>
+'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) [--use-path=<path>] <object>
 'git cat-file' (--batch | --batch-check) [--follow-symlinks]
 
 DESCRIPTION
@@ -64,6 +64,11 @@ OPTIONS
 	end-of-line conversion, etc). In this case, <object> has to be of
 	the form <tree-ish>:<path>, or :<path>.
 
+--use-path=<path>::
+	For use with --textconv or --filters, to allow specifying an object
+	name and a path separately, e.g. when it is difficult to figure out
+	the revision from which the blob came.
+
 --batch::
 --batch=<format>::
 	Print object information and contents for each object provided
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 0b74afa..5ff58b3 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,6 +20,8 @@ struct batch_options {
 	const char *format;
 };
 
+static const char *force_path;
+
 static int filter_object(const char *path, unsigned mode,
 			 const unsigned char *sha1,
 			 char **buf, unsigned long *size)
@@ -89,21 +91,24 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 		return !has_sha1_file(sha1);
 
 	case 'w':
-		if (!obj_context.path[0])
+		if (!force_path && !obj_context.path[0])
 			die("git cat-file --filters %s: <object> must be "
 			    "<sha1:path>", obj_name);
 
-		if (filter_object(obj_context.path, obj_context.mode,
+		if (filter_object(force_path ? force_path : obj_context.path,
+				  force_path ? 0100644 : obj_context.mode,
 				  sha1, &buf, &size))
 			return -1;
 		break;
 
 	case 'c':
-		if (!obj_context.path[0])
+		if (!force_path && !obj_context.path[0])
 			die("git cat-file --textconv %s: <object> must be <sha1:path>",
 			    obj_name);
 
-		if (textconv_object(obj_context.path, obj_context.mode, sha1, 1, &buf, &size))
+		if (textconv_object(force_path ? force_path : obj_context.path,
+				    force_path ? 0100644 : obj_context.mode,
+				    sha1, 1, &buf, &size))
 			break;
 
 	case 'p':
@@ -477,7 +482,7 @@ static int batch_objects(struct batch_options *opt)
 }
 
 static const char * const cat_file_usage[] = {
-	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) <object>"),
+	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) [--use-path=<path>] <object>"),
 	N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
 	NULL
 };
@@ -525,6 +530,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 			    N_("for blob objects, run textconv on object's content"), 'c'),
 		OPT_CMDMODE(0, "filters", &opt,
 			    N_("for blob objects, run filters on object's content"), 'w'),
+		OPT_STRING(0, "use-path", &force_path, N_("blob"),
+			   N_("use a specific path for --textconv/--filters")),
 		OPT_BOOL(0, "allow-unknown-type", &unknown_type,
 			  N_("allow -s and -t to work with broken/corrupt objects")),
 		OPT_BOOL(0, "buffer", &batch.buffer_output, N_("buffer --batch output")),
@@ -567,6 +574,11 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		usage_with_options(cat_file_usage, options);
 	}
 
+	if (force_path && opt != 'c' && opt != 'w') {
+		error("--use-path=<path> needs --textconv or --filters");
+		usage_with_options(cat_file_usage, options);
+	}
+
 	if (batch.buffer_output < 0)
 		batch.buffer_output = batch.all_objects;
 
diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
index e466634..fd17159 100755
--- a/t/t8010-cat-file-filters.sh
+++ b/t/t8010-cat-file-filters.sh
@@ -31,4 +31,17 @@ test_expect_success 'cat-file --filters converts to worktree version' '
 	has_cr actual
 '
 
+test_expect_success 'cat-file --filters --use-path=<path> works' '
+	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
+	git cat-file --filters --use-path=world.txt $sha1 >actual &&
+	has_cr actual
+'
+
+test_expect_success 'cat-file --textconv --use-path=<path> works' '
+	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
+	test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
+	git cat-file --textconv --use-path=hello.txt $sha1 >rot13 &&
+	test uryyb = "$(cat rot13 | remove_cr)"
+'
+
 test_done
-- 
2.9.2.691.g78954f3

[PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Johannes Schindelin]

With this patch, --batch can be combined with --textconv or --filters.
For this to work, the input needs to have the form

	<object name><single white space><path>

so that the filters can be chosen appropriately.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-cat-file.txt | 18 +++++++++++-----
 builtin/cat-file.c             | 49 +++++++++++++++++++++++++++++++++++++-----
 t/t8010-cat-file-filters.sh    | 10 +++++++++
 3 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 59a3c37..1f4d954 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) [--use-path=<path>] <object>
-'git cat-file' (--batch | --batch-check) [--follow-symlinks]
+'git cat-file' (--batch | --batch-check) [ --textconv | --filters ] [--follow-symlinks]
 
 DESCRIPTION
 -----------
@@ -20,7 +20,11 @@ object type, or `-s` is used to find the object size, or `--textconv` or
 `--filters` is used (which imply type "blob").
 
 In the second form, a list of objects (separated by linefeeds) is provided on
-stdin, and the SHA-1, type, and size of each object is printed on stdout.
+stdin, and the SHA-1, type, and size of each object is printed on stdout. The
+output format can be overridden using the optional `<format>` argument. If
+either `--textconv` or `--filters` was specified, the input is expected to
+list the object names followed by the path name, separated by a single white
+space, so that the appropriate drivers can be determined.
 
 OPTIONS
 -------
@@ -72,13 +76,17 @@ OPTIONS
 --batch::
 --batch=<format>::
 	Print object information and contents for each object provided
-	on stdin.  May not be combined with any other options or arguments.
-	See the section `BATCH OUTPUT` below for details.
+	on stdin.  May not be combined with any other options or arguments
+	except `--textconv` or `--filters`, in which case the input lines
+	also need to specify the path, separated by white space.  See the
+	section `BATCH OUTPUT` below for details.
 
 --batch-check::
 --batch-check=<format>::
 	Print object information for each object provided on stdin.  May
-	not be combined with any other options or arguments.  See the
+	not be combined with any other options or arguments except
+	`--textconv` or `--filters`, in which case the input lines also
+	need to specify the path, separated by white space.  See the
 	section `BATCH OUTPUT` below for details.
 
 --batch-all-objects::
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 5ff58b3..5f91cf4 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -17,6 +17,7 @@ struct batch_options {
 	int print_contents;
 	int buffer_output;
 	int all_objects;
+	int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
 	const char *format;
 };
 
@@ -281,7 +282,32 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
 	if (data->type == OBJ_BLOB) {
 		if (opt->buffer_output)
 			fflush(stdout);
-		if (stream_blob_to_fd(1, sha1, NULL, 0) < 0)
+		if (opt->cmdmode) {
+			char *contents;
+			unsigned long size;
+
+			if (!data->rest)
+				die("missing path for '%s'", sha1_to_hex(sha1));
+
+			if (opt->cmdmode == 'w') {
+				if (filter_object(data->rest, 0100644, sha1,
+						  &contents, &size))
+					die("could not convert '%s' %s",
+					    sha1_to_hex(sha1), data->rest);
+			} else if (opt->cmdmode == 'c') {
+				enum object_type type;
+				if (!textconv_object(data->rest, 0100644, sha1,
+						     1, &contents, &size))
+					contents = read_sha1_file(sha1, &type,
+								  &size);
+				if (!contents)
+					die("could not convert '%s' %s",
+					    sha1_to_hex(sha1), data->rest);
+			} else
+				die("BUG: invalid cmdmode: %c", opt->cmdmode);
+			batch_write(opt, contents, size);
+			free(contents);
+		} else if (stream_blob_to_fd(1, sha1, NULL, 0) < 0)
 			die("unable to stream %s to stdout", sha1_to_hex(sha1));
 	}
 	else {
@@ -418,6 +444,8 @@ static int batch_objects(struct batch_options *opt)
 	data.mark_query = 1;
 	strbuf_expand(&buf, opt->format, expand_format, &data);
 	data.mark_query = 0;
+	if (opt->cmdmode)
+		data.split_on_whitespace = 1;
 
 	if (opt->all_objects) {
 		struct object_info empty;
@@ -483,7 +511,7 @@ static int batch_objects(struct batch_options *opt)
 
 static const char * const cat_file_usage[] = {
 	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) [--use-path=<path>] <object>"),
-	N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
+	N_("git cat-file (--batch | --batch-check) [--follow-symlinks] [--textconv|--filters]"),
 	NULL
 };
 
@@ -554,7 +582,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, cat_file_usage, 0);
 
 	if (opt) {
-		if (argc == 1)
+		if (batch.enabled && (opt == 'c' || opt == 'w'))
+			batch.cmdmode = opt;
+		else if (argc == 1)
 			obj_name = argv[0];
 		else
 			usage_with_options(cat_file_usage, options);
@@ -566,8 +596,12 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		} else
 			usage_with_options(cat_file_usage, options);
 	}
-	if (batch.enabled && (opt || argc)) {
-		usage_with_options(cat_file_usage, options);
+	if (batch.enabled) {
+		if (batch.cmdmode != opt || argc)
+			usage_with_options(cat_file_usage, options);
+		if (batch.cmdmode && batch.all_objects)
+			die("--batch-all-objects cannot be combined with "
+			    "--textconv nor with --filters");
 	}
 
 	if ((batch.follow_symlinks || batch.all_objects) && !batch.enabled) {
@@ -579,6 +613,11 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		usage_with_options(cat_file_usage, options);
 	}
 
+	if (force_path && batch.enabled) {
+		error("--use-path=<path> incompatible with --batch");
+		usage_with_options(cat_file_usage, options);
+	}
+
 	if (batch.buffer_output < 0)
 		batch.buffer_output = batch.all_objects;
 
diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
index fd17159..89ae868 100755
--- a/t/t8010-cat-file-filters.sh
+++ b/t/t8010-cat-file-filters.sh
@@ -44,4 +44,14 @@ test_expect_success 'cat-file --textconv --use-path=<path> works' '
 	test uryyb = "$(cat rot13 | remove_cr)"
 '
 
+test_expect_success 'cat-file --textconv --batch works' '
+	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
+	test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
+	printf "%s hello.txt\n%s hello\n" $sha1 $sha1 |
+	git cat-file --textconv --batch >actual &&
+	printf "%s blob 6\nuryyb\r\n\n%s blob 6\nhello\n\n" \
+		$sha1 $sha1 >expect &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.9.2.691.g78954f3

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Torsten Bögershausen]

On Thu, Aug 18, 2016 at 02:46:28PM +0200, Johannes Schindelin wrote:
> With this patch, --batch can be combined with --textconv or --filters.
> For this to work, the input needs to have the form
> 
> 	<object name><single white space><path>
> 
> so that the filters can be chosen appropriately.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  Documentation/git-cat-file.txt | 18 +++++++++++-----
>  builtin/cat-file.c             | 49 +++++++++++++++++++++++++++++++++++++-----
>  t/t8010-cat-file-filters.sh    | 10 +++++++++
>  3 files changed, 67 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
> index 59a3c37..1f4d954 100644
> --- a/Documentation/git-cat-file.txt
> +++ b/Documentation/git-cat-file.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  --------
>  [verse]
>  'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) [--use-path=<path>] <object>
> -'git cat-file' (--batch | --batch-check) [--follow-symlinks]
> +'git cat-file' (--batch | --batch-check) [ --textconv | --filters ] [--follow-symlinks]
>  
>  DESCRIPTION
>  -----------
> @@ -20,7 +20,11 @@ object type, or `-s` is used to find the object size, or `--textconv` or
>  `--filters` is used (which imply type "blob").
>  
>  In the second form, a list of objects (separated by linefeeds) is provided on
> -stdin, and the SHA-1, type, and size of each object is printed on stdout.
> +stdin, and the SHA-1, type, and size of each object is printed on stdout. The
> +output format can be overridden using the optional `<format>` argument. If
> +either `--textconv` or `--filters` was specified, the input is expected to
> +list the object names followed by the path name, separated by a single white
> +space, so that the appropriate drivers can be determined.
>  
>  OPTIONS
>  -------
> @@ -72,13 +76,17 @@ OPTIONS
>  --batch::
>  --batch=<format>::
>  	Print object information and contents for each object provided
> -	on stdin.  May not be combined with any other options or arguments.
> -	See the section `BATCH OUTPUT` below for details.
> +	on stdin.  May not be combined with any other options or arguments
> +	except `--textconv` or `--filters`, in which case the input lines
> +	also need to specify the path, separated by white space.  See the
> +	section `BATCH OUTPUT` below for details.
>  
>  --batch-check::
>  --batch-check=<format>::
>  	Print object information for each object provided on stdin.  May
> -	not be combined with any other options or arguments.  See the
> +	not be combined with any other options or arguments except
> +	`--textconv` or `--filters`, in which case the input lines also
> +	need to specify the path, separated by white space.  See the
>  	section `BATCH OUTPUT` below for details.
>  
>  --batch-all-objects::
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 5ff58b3..5f91cf4 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -17,6 +17,7 @@ struct batch_options {
>  	int print_contents;
>  	int buffer_output;
>  	int all_objects;
> +	int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
How do I read 'w' and 'c' ?
wilter and cextconv ? Does it make sense to use an enum here ?
Or a #define ?

Re: [PATCH 2/4] cat-file: introduce the --filters option

[Torsten Bögershausen]

On Thu, Aug 18, 2016 at 02:46:17PM +0200, Johannes Schindelin wrote:
> As suggested by its name, the --filters option applies the filters

[]
> diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
Does it make sense to integrate tests into t1006-cat-file ?
> new file mode 100755
> index 0000000..e466634
> --- /dev/null
> +++ b/t/t8010-cat-file-filters.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +test_description='git cat-file filters support'
> +. ./test-lib.sh
> +
> +test_expect_success 'setup ' '
> +	echo "*.txt eol=crlf diff=txt" >.gitattributes &&
> +	echo "hello" | append_cr >world.txt &&
> +	git add .gitattributes world.txt &&
> +	test_tick &&
> +	git commit -m "Initial commit"
> +'
> +
> +has_cr () {
> +	tr '\015' Q <"$1" | grep Q >/dev/null
> +}
> +
> +test_expect_success 'no filters with `git show`' '
> +	git show HEAD:world.txt >actual &&
I would prefer to have something using
  cat >expect <<-\EOF &&
  xxx
  test_cmp expect actual
to make it easier to debug in case of a failure ?

Re: [PATCH 1/4] cat-file: fix a grammo in the man page

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> "... has be ..." -> "... has to be ..."
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  Documentation/git-cat-file.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
> index 18d03d8..071029b 100644
> --- a/Documentation/git-cat-file.txt
> +++ b/Documentation/git-cat-file.txt
> @@ -54,8 +54,9 @@ OPTIONS
>  
>  --textconv::
>  	Show the content as transformed by a textconv filter. In this case,
> -	<object> has be of the form <tree-ish>:<path>, or :<path> in order
> -	to apply the filter to the content recorded in the index at <path>.
> +	<object> has to be of the form <tree-ish>:<path>, or :<path> in
> +	order to apply the filter to the content recorded in the index at
> +	<path>.

Thanks.  That's an ancient typo dating back to 2010 ;-)

Re: [PATCH 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> As suggested by its name, the --filters option applies the filters
> that are currently configured for the specified path.
>
> This feature comes in handy when a 3rd-party tool wants to work with
> the contents of files from past revisions as if they had been checked
> out, but without detouring via temporary files.
>
> Note that we ensure that symbolic links are unaffected (we know from
> looking at the mode whether it refers to a symbolic link or a regular
> file).

I think you can lose the last paragraph and enrich the first one
instead.  The text as-given does not even say anything about the new
option affecting only blobs.

	The --filters option applies the convert_to_working_tree()
	filter for the path when showing the contents of a regular
	file blob object.

or something like that.

> +--filters::
> +	Show the content as transformed by the filters configured in
> +	the current working tree for the given <path> (i.e. smudge filters,
> +	end-of-line conversion, etc). In this case, <object> has to be of
> +	the form <tree-ish>:<path>, or :<path>.

Makes sense.  We show the contents as if we are checking the blob
out to the given <path>, in other words.

> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 2dfe626..0b74afa 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -20,6 +20,33 @@ struct batch_options {
>  	const char *format;
>  };
>  
> +static int filter_object(const char *path, unsigned mode,
> +			 const unsigned char *sha1,
> +			 char **buf, unsigned long *size)
> +{
> +	enum object_type type;
> +
> +	*buf = read_sha1_file(sha1, &type, size);
> +	if (!*buf)
> +		return error(_("cannot read object %s '%s'"),
> +			sha1_to_hex(sha1), path);
> +	if (type != OBJ_BLOB) {
> +		free(*buf);
> +		return error(_("blob expected for %s '%s'"),
> +			sha1_to_hex(sha1), path);
> +	}
> +	if (S_ISREG(mode)) {
> +		struct strbuf strbuf = STRBUF_INIT;
> +		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
> +			free(*buf);
> +			*size = strbuf.len;
> +			*buf = strbuf_detach(&strbuf, NULL);
> +		}
> +	}

This is just a tangent for future clean-up of the coding convention,
but if we used size_t internally throughout the system, we would be
able to use strbuf_detach(&strbuf, size) instead.  With the current
coding convention, all internal sizes are ulong, and this is the
best we could do.

> @@ -61,6 +88,16 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
>  	case 'e':
>  		return !has_sha1_file(sha1);
>  
> +	case 'w':
> +		if (!obj_context.path[0])
> +			die("git cat-file --filters %s: <object> must be "
> +			    "<sha1:path>", obj_name);
> +
> +		if (filter_object(obj_context.path, obj_context.mode,
> +				  sha1, &buf, &size))
> +			return -1;
> +		break;
> +

Makes sense.

> diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
> new file mode 100755
> index 0000000..e466634
> --- /dev/null
> +++ b/t/t8010-cat-file-filters.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +test_description='git cat-file filters support'
> +. ./test-lib.sh
> +
> +test_expect_success 'setup ' '
> +	echo "*.txt eol=crlf diff=txt" >.gitattributes &&
> +	echo "hello" | append_cr >world.txt &&
> +	git add .gitattributes world.txt &&
> +	test_tick &&
> +	git commit -m "Initial commit"
> +'

OK, so we ask *.txt in the working tree to be CRLF (and presumably
that means the in-repository objects are LF).

> +has_cr () {
> +	tr '\015' Q <"$1" | grep Q >/dev/null
> +}
> +
> +test_expect_success 'no filters with `git show`' '
> +	git show HEAD:world.txt >actual &&
> +	! has_cr actual
> +
> +'
> +
> +test_expect_success 'no filters with cat-file' '
> +	git cat-file blob HEAD:world.txt >actual &&
> +	! has_cr actual
> +'

OK.

> +test_expect_success 'cat-file --filters converts to worktree version' '
> +	git cat-file --filters HEAD:world.txt >actual &&
> +	has_cr actual
> +'

OK.

Re: [PATCH 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> There are circumstances when it is relatively easy to figure out the
> object name for a given path, but not the revision. For example, when

revision of the containing tree, I presume?

> Let's simplify this dramatically, because we do not really need that
> revision at all: all we care about is that we know the path. In the
> scenario described above, we do know the path, and we just want to
> specify it separately from the object name.

I wouldn't call it "simplifying dramatically".  It is just to
support two use cases that is different from the use case where you
want to use "<tree>:<path>".

We already have a precedent in "hash-object --path=<path>" for these
two different uses cases from the primary one.  That form can be
used when we know the contents but we do not know where the contents
came from.  It can also be used when we want to pretend that
contents came from a specific path, that may be different from the
file we are hashing.

Let's be consistent and (1) call it "--path", and (2) explain it as
a feature to allow you to tell the path separately or allow you to
pretend that the content is at a path different from reality.

After all, you would want to ignore <path2> part in this construct:

	git cat-file --filter --path=<path1> HEAD:<path2>

for the purpose of filtering, right?

> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 0b74afa..5ff58b3 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -20,6 +20,8 @@ struct batch_options {
>  	const char *format;
>  };
>  
> +static const char *force_path;
> +
>  static int filter_object(const char *path, unsigned mode,
>  			 const unsigned char *sha1,
>  			 char **buf, unsigned long *size)
> @@ -89,21 +91,24 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
>  		return !has_sha1_file(sha1);
>  
>  	case 'w':
> -		if (!obj_context.path[0])
> +		if (!force_path && !obj_context.path[0])
>  			die("git cat-file --filters %s: <object> must be "
>  			    "<sha1:path>", obj_name);
>  
> -		if (filter_object(obj_context.path, obj_context.mode,
> +		if (filter_object(force_path ? force_path : obj_context.path,
> +				  force_path ? 0100644 : obj_context.mode,
>  				  sha1, &buf, &size))

The mode override is somewhat questionable.  Wouldn't you want to
reject

	git cat-file --filter --path=Makefile HEAD:RelNotes

when HEAD:RelNotes blob is known to be a symbolic link?  After all,
you would reject this

	git cat-file --filter --path=Makefile HEAD:t/

and it is quite similar, no?  I think this can be argued both ways,
but I have a feeling that obj_context.mode, if available, should be
honored with or without force_path.

> diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
> index e466634..fd17159 100755
> --- a/t/t8010-cat-file-filters.sh
> +++ b/t/t8010-cat-file-filters.sh
> @@ -31,4 +31,17 @@ test_expect_success 'cat-file --filters converts to worktree version' '
>  	has_cr actual
>  '
>  
> +test_expect_success 'cat-file --filters --use-path=<path> works' '
> +	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
> +	git cat-file --filters --use-path=world.txt $sha1 >actual &&
> +	has_cr actual
> +'
> +
> +test_expect_success 'cat-file --textconv --use-path=<path> works' '
> +	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
> +	test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
> +	git cat-file --textconv --use-path=hello.txt $sha1 >rot13 &&
> +	test uryyb = "$(cat rot13 | remove_cr)"
> +'

I think I saw some code to ensure "when giving this option you need
that option in effect, too"; they should be tested here, too, no?

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> With this patch, --batch can be combined with --textconv or --filters.
> For this to work, the input needs to have the form
>
> 	<object name><single white space><path>
>
> so that the filters can be chosen appropriately.
>  --batch::
>  --batch=<format>::
>  	Print object information and contents for each object provided
> -	on stdin.  May not be combined with any other options or arguments.
> -	See the section `BATCH OUTPUT` below for details.
> +	on stdin.  May not be combined with any other options or arguments
> +	except `--textconv` or `--filters`, in which case the input lines
> +	also need to specify the path, separated by white space.  See the
> +	section `BATCH OUTPUT` below for details.

Makes sense.  This format still allows

	HEAD:<path2> <path1>

i.e. the object name is taken from path2 but we forget it and
pretend that the blob sits at path2, which is a good feature.

If I am not mistaken, your cover letter alluded that it might be
ideal to take "HEAD:<path>" (nothing else) as input, grab "<path>"
and feed that to the filtering machinery, but you decided to stop
short of doing that.  I actually think that it is the right thing to
do for this feature to ignore the trailing :<path> in the object
name, iow, I agree with the result from the feature design POV.

The only thing that somewhat is worrysome is what would happen to
%(rest).  I guess [*1*] it is OK to declare that you cannot use %(rest) in
your output format when --filter/--textconv is in use, but if that
is the direction we are going, that limitation needs to be
documented.


[Footnote]

*1* This is just a "guess", because I do not know what people are
using %(rest) for.  It is plausible that their use case do not need
--filter/--textconv at all, and if that is the case, having this
limitation is perfectly fine.

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Jeff King]

On Thu, Aug 18, 2016 at 02:46:28PM +0200, Johannes Schindelin wrote:

> With this patch, --batch can be combined with --textconv or --filters.
> For this to work, the input needs to have the form
> 
> 	<object name><single white space><path>
> 
> so that the filters can be chosen appropriately.

The object name can have spaces in it, too. E.g.:

  HEAD:path with spaces

or even:

  :/grep for this

(as was pointed out to me when I tried to turn on %(rest) handling by
default, long ago). How do those work with your patch?

It looks like the extra split isn't enabled unless one of those options
is selected. Since --filters is new, that's OK for backwards
compatibility. But --textconv isn't. So I think:

  echo "HEAD:path with spaces" |
  git cat-file --textconv --batch

is regressed by this patch.

I wonder if we need an option specifically to say "the object name can
be split". Right now it kicks in automatically if you use "%(rest)" in
your format, but you might not care about passing along that output
(e.g., a lot of times I am piping "rev-list" straight to cat-file, and I
have to use a separate "cut" to throw away the pathnames).

-Peff

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Aug 18, 2016 at 02:46:28PM +0200, Johannes Schindelin wrote:
>
>> With this patch, --batch can be combined with --textconv or --filters.
>> For this to work, the input needs to have the form
>> 
>> 	<object name><single white space><path>
>> 
>> so that the filters can be chosen appropriately.
>
> The object name can have spaces in it, too. E.g.:
>
>   HEAD:path with spaces
>
> or even:
>
>   :/grep for this

When I wrote my review, I didn't consider this use case.

There is no -z format in --batch, which is unfortunate.  If we had
one, it would trivially make it possible to do so, and we can even
have paths with LF in them ;-).  On the other hand, producing a NUL
separated input is a chore.

Perhaps a new and separate option that is similar to "--batch" but
lacks support for %(rest) and accepts _ONLY_ 40-hex as object name
is the best we can do, then?

> (as was pointed out to me when I tried to turn on %(rest) handling by
> default, long ago). How do those work with your patch?
>
> It looks like the extra split isn't enabled unless one of those options
> is selected. Since --filters is new, that's OK for backwards
> compatibility. But --textconv isn't. So I think:
>
>   echo "HEAD:path with spaces" |
>   git cat-file --textconv --batch
>
> is regressed by this patch.
>
> I wonder if we need an option specifically to say "the object name can
> be split". Right now it kicks in automatically if you use "%(rest)" in
> your format, but you might not care about passing along that output
> (e.g., a lot of times I am piping "rev-list" straight to cat-file, and I
> have to use a separate "cut" to throw away the pathnames).
>
> -Peff

Re: [PATCH 2/4] cat-file: introduce the --filters option

[Torsten Bögershausen]

[]
On Thu, Aug 18, 2016 at 02:46:17PM +0200, Johannes Schindelin wrote:

> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
> index 071029b..7d48735 100644
> --- a/Documentation/git-cat-file.txt
> +++ b/Documentation/git-cat-file.txt
> @@ -9,15 +9,15 @@ git-cat-file - Provide content or type and size information for repository objec
>  SYNOPSIS
>  --------
>  [verse]
> -'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv ) <object>
> +'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) <object>
>  'git cat-file' (--batch | --batch-check) [--follow-symlinks]
>  
>  DESCRIPTION
>  -----------
>  In its first form, the command provides the content or the type of an object in
>  the repository. The type is required unless `-t` or `-p` is used to find the
> -object type, or `-s` is used to find the object size, or `--textconv` is used
> -(which implies type "blob").
> +object type, or `-s` is used to find the object size, or `--textconv` or
> +`--filters` is used (which imply type "blob").
>  
>  In the second form, a list of objects (separated by linefeeds) is provided on
>  stdin, and the SHA-1, type, and size of each object is printed on stdout.
> @@ -58,6 +58,12 @@ OPTIONS
>  	order to apply the filter to the content recorded in the index at
>  	<path>.
>  
> +--filters::
> +	Show the content as transformed by the filters configured in
Minor comment:
s/transformed/converted/ ?

Does it make sense to be more specific here:
The order of conversion is
- ident
- CRLF
- smudge

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

Hi Torsten,

On Thu, 18 Aug 2016, Torsten Bögershausen wrote:

> On Thu, Aug 18, 2016 at 02:46:28PM +0200, Johannes Schindelin wrote:
> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > index 5ff58b3..5f91cf4 100644
> > --- a/builtin/cat-file.c
> > +++ b/builtin/cat-file.c
> > @@ -17,6 +17,7 @@ struct batch_options {
> >  	int print_contents;
> >  	int buffer_output;
> >  	int all_objects;
> > +	int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
> How do I read 'w' and 'c' ?
> wilter and cextconv ? Does it make sense to use an enum here ?
> Or a #define ?

Sorry, Torsten, this is not my doing. So I cannot explain why it is not an
enum.

I *guess* the rationale for 'c' being the cmdmode of --textconv is "c for
convert". That is why I chose "w as in worktree" as new cmdmode.

Ciao,
Dscho

Re: [PATCH 2/4] cat-file: introduce the --filters option

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 1555 bytes --]

Hi Torsten,

On Thu, 18 Aug 2016, Torsten Bögershausen wrote:

> On Thu, Aug 18, 2016 at 02:46:17PM +0200, Johannes Schindelin wrote:
> > As suggested by its name, the --filters option applies the filters
> 
> []
> > diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
>
> Does it make sense to integrate tests into t1006-cat-file ?

t1006 is already a 500+ line script, running 110 cases in 13 seconds over
here. I really rather have a new, small, concise and parallelizable
script.

> > +test_expect_success 'no filters with `git show`' '
> > +	git show HEAD:world.txt >actual &&
>
> I would prefer to have something using
>   cat >expect <<-\EOF &&
>   xxx
>   test_cmp expect actual
> to make it easier to debug in case of a failure ?

Actually, I find it much harder to debug these "the output must match
these precise bytes, else we fail" type of test cases. Instead, I describe
in a natural way what is expected:

	! has_cr actual

Now, when the test fails, whoever is that poor soul tasked with debugging
and fixing the breakage knows *what* goes wrong, conceptually.

So I do not think it would be a good idea to change the test in the way
you requested.

And now I have a request of my own. When you quote parts of my mail, you
insert your answers without surrounding them by empty lines. That makes it
extraordinarily hard for me to read your mails (in fact, I almost missed
your second comment).

Could you please start surrounding your replies by empty lines?

Thank you,
Dscho

Re: [PATCH 2/4] cat-file: introduce the --filters option

[Johannes Schindelin]

Hi Junio,

On Thu, 18 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > As suggested by its name, the --filters option applies the filters
> > that are currently configured for the specified path.
> >
> > [...]
> 
> I think you can lose the last paragraph and enrich the first one
> instead.  The text as-given does not even say anything about the new
> option affecting only blobs.
> 
> 	The --filters option applies the convert_to_working_tree()
> 	filter for the path when showing the contents of a regular
> 	file blob object.
> 
> or something like that.

Done.

Thanks,
Dscho

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Johannes Schindelin]

Hi Junio,

On Thu, 18 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > With this patch, --batch can be combined with --textconv or --filters.
> > For this to work, the input needs to have the form
> >
> > 	<object name><single white space><path>
> >
> > so that the filters can be chosen appropriately.
> >  --batch::
> >  --batch=<format>::
> >  	Print object information and contents for each object provided
> > -	on stdin.  May not be combined with any other options or arguments.
> > -	See the section `BATCH OUTPUT` below for details.
> > +	on stdin.  May not be combined with any other options or arguments
> > +	except `--textconv` or `--filters`, in which case the input lines
> > +	also need to specify the path, separated by white space.  See the
> > +	section `BATCH OUTPUT` below for details.
> 
> Makes sense.  This format still allows
> 
> 	HEAD:<path2> <path1>
> 
> i.e. the object name is taken from path2 but we forget it and
> pretend that the blob sits at path2, which is a good feature.

Correct.

> If I am not mistaken, your cover letter alluded that it might be
> ideal to take "HEAD:<path>" (nothing else) as input, grab "<path>"
> and feed that to the filtering machinery, but you decided to stop
> short of doing that.  I actually think that it is the right thing to
> do for this feature to ignore the trailing :<path> in the object
> name, iow, I agree with the result from the feature design POV.

I actually decided against it only because it would uglify the code. From
a user point of view, I am a bit annoyed having to say

	:README README
	:Makefile Makefile
	:main.c main.c
	[...]

> The only thing that somewhat is worrysome is what would happen to
> %(rest).

What would happen is that it would print out the path, as it is exactly
that "rest" field that is used in the implementation, piggybacking on that
very nice feature.

Of course, this means that you simply cannot send "SHA-1 path rest" to
cat-file --batch and expect that to work. Should anybody need that in the
future, they can give it a try themselves to patch cat-file.

Ciao,
Dscho

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Johannes Schindelin]

Hi Peff,

On Thu, 18 Aug 2016, Jeff King wrote:

> On Thu, Aug 18, 2016 at 02:46:28PM +0200, Johannes Schindelin wrote:
> 
> > With this patch, --batch can be combined with --textconv or --filters.
> > For this to work, the input needs to have the form
> > 
> > 	<object name><single white space><path>
> > 
> > so that the filters can be chosen appropriately.
> 
> The object name can have spaces in it, too. E.g.:
> 
>   HEAD:path with spaces
> 
> or even:
> 
>   :/grep for this
> 
> (as was pointed out to me when I tried to turn on %(rest) handling by
> default, long ago). How do those work with your patch?

They don't ;-)

And quite frankly, the documentation should make it clear to users that
batch mode with --filters or --textconv won't work when the object name
contains white space: it says that the object name is split from the path
at the first white space character.

> It looks like the extra split isn't enabled unless one of those options
> is selected. Since --filters is new, that's OK for backwards
> compatibility. But --textconv isn't.

Except that it is okay, because --textconv *was not even supported in
batch mode*. So there is no backwards compatibility that could be broken.

> I wonder if we need an option specifically to say "the object name can
> be split". Right now it kicks in automatically if you use "%(rest)" in
> your format, but you might not care about passing along that output
> (e.g., a lot of times I am piping "rev-list" straight to cat-file, and I
> have to use a separate "cut" to throw away the pathnames).

As I said elsewhere in this thread: if anybody encounters that problem,
they are welcome to fix it themselves.

My patch series purpose is to introduce --filters and make it compatible
with the batch mode. As I imitated --textconv, it was both easy and
correct to make it compatible with the batch mode, too. But that is the
extent of this here patch series.

Fixing %(rest) for object names containing spaces is distinctly outside
the original intent, and certainly outside of my use case.

Ciao,
Dscho

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Johannes Schindelin]

Hi Junio,

On Thu, 18 Aug 2016, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Thu, Aug 18, 2016 at 02:46:28PM +0200, Johannes Schindelin wrote:
> >
> >> With this patch, --batch can be combined with --textconv or --filters.
> >> For this to work, the input needs to have the form
> >> 
> >> 	<object name><single white space><path>
> >> 
> >> so that the filters can be chosen appropriately.
> >
> > The object name can have spaces in it, too. E.g.:
> >
> >   HEAD:path with spaces
> >
> > or even:
> >
> >   :/grep for this
> 
> When I wrote my review, I didn't consider this use case.
> 
> There is no -z format in --batch, which is unfortunate.  If we had
> one, it would trivially make it possible to do so, and we can even
> have paths with LF in them ;-).  On the other hand, producing a NUL
> separated input is a chore.

I think it would make for a fine little project to add support for --batch
-z.

Just not in this here patch series.

Ciao,
Dscho

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Jeff King]

On Fri, Aug 19, 2016 at 03:09:19PM +0200, Johannes Schindelin wrote:

> > The object name can have spaces in it, too. E.g.:
> > 
> >   HEAD:path with spaces
> > 
> > or even:
> > 
> >   :/grep for this
> > 
> > (as was pointed out to me when I tried to turn on %(rest) handling by
> > default, long ago). How do those work with your patch?
> 
> They don't ;-)
> 
> And quite frankly, the documentation should make it clear to users that
> batch mode with --filters or --textconv won't work when the object name
> contains white space: it says that the object name is split from the path
> at the first white space character.

I think that is an obvious implication of the new documentation. I was
just concerned with a regression from the previous behavior.

> > It looks like the extra split isn't enabled unless one of those options
> > is selected. Since --filters is new, that's OK for backwards
> > compatibility. But --textconv isn't.
> 
> Except that it is okay, because --textconv *was not even supported in
> batch mode*. So there is no backwards compatibility that could be broken.

Ah, OK. I thought we handled "HEAD:path with spaces" there, but I see
that you cannot even specify "--textconv" with "--batch", because it
complains of the cmdmode.

So the new rule becomes "we split if we see %(rest), or --textconv, or
--filter", which is reasonable.

> Fixing %(rest) for object names containing spaces is distinctly outside
> the original intent, and certainly outside of my use case.

Yeah, I agree it is not necessary for this series (I was only
considering it as an option to fix the regression I now see doesn't
exist).

-Peff

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Jeff King]

On Fri, Aug 19, 2016 at 02:59:32PM +0200, Johannes Schindelin wrote:

> > The only thing that somewhat is worrysome is what would happen to
> > %(rest).
> 
> What would happen is that it would print out the path, as it is exactly
> that "rest" field that is used in the implementation, piggybacking on that
> very nice feature.

Good, that is what I would expect to happen.

> Of course, this means that you simply cannot send "SHA-1 path rest" to
> cat-file --batch and expect that to work. Should anybody need that in the
> future, they can give it a try themselves to patch cat-file.

Right. The real motivation for %(rest) was to accept "git rev-list
--objects" format, so you can do stuff like:

  git rev-list --objects --all |
  git cat-file --batch-check='%(objectsize) %(rest)' |
  sort -rn |
  head

to get the paths of the largest objects. So the fact that "%(rest)" will
be the filename in this case is almost certainly the most useful default
(and if somebody really wants something else, they can try to figure out
other semantics later).

-Peff

Re: [PATCH 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Johannes Schindelin]

Hi Junio,

On Thu, 18 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > There are circumstances when it is relatively easy to figure out the
> > object name for a given path, but not the revision. For example, when
> 
> revision of the containing tree, I presume?

name of containing tree, actually.

> > Let's simplify this dramatically, because we do not really need that
> > revision at all: all we care about is that we know the path. In the
> > scenario described above, we do know the path, and we just want to
> > specify it separately from the object name.
> 
> I wouldn't call it "simplifying dramatically".  It is just to
> support two use cases that is different from the use case where you
> want to use "<tree>:<path>".

"this" was not the code in question. "this" is the use case. Just put my
shoes on for a moment. As described: you have a list of object names and
their path. Now you need to use the --textconv or --filters option, but
you do not have the commit name. What do you do? Concoct a script that
goes through `git rev-list --all -- <path>` in the hopes of finding a
revision soon? I hope after this little exercise you agree that
introducing the --path=<path> option simplifies this use case
dramatically.

> We already have a precedent in "hash-object --path=<path>" for these
> two different uses cases from the primary one.  That form can be
> used when we know the contents but we do not know where the contents
> came from.  It can also be used when we want to pretend that
> contents came from a specific path, that may be different from the
> file we are hashing.

Agreed. I was unaware of hash-object's existing option.

> Let's be consistent and (1) call it "--path", and (2) explain it as
> a feature to allow you to tell the path separately or allow you to
> pretend that the content is at a path different from reality.
> 
> After all, you would want to ignore <path2> part in this construct:
> 
> 	git cat-file --filter --path=<path1> HEAD:<path2>
> 
> for the purpose of filtering, right?

True, and I even make use of that in the test for the batch mode.

> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > index 0b74afa..5ff58b3 100644
> > --- a/builtin/cat-file.c
> > +++ b/builtin/cat-file.c
> > @@ -20,6 +20,8 @@ struct batch_options {
> >  	const char *format;
> >  };
> >  
> > +static const char *force_path;
> > +
> >  static int filter_object(const char *path, unsigned mode,
> >  			 const unsigned char *sha1,
> >  			 char **buf, unsigned long *size)
> > @@ -89,21 +91,24 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
> >  		return !has_sha1_file(sha1);
> >  
> >  	case 'w':
> > -		if (!obj_context.path[0])
> > +		if (!force_path && !obj_context.path[0])
> >  			die("git cat-file --filters %s: <object> must be "
> >  			    "<sha1:path>", obj_name);
> >  
> > -		if (filter_object(obj_context.path, obj_context.mode,
> > +		if (filter_object(force_path ? force_path : obj_context.path,
> > +				  force_path ? 0100644 : obj_context.mode,
> >  				  sha1, &buf, &size))
> 
> The mode override is somewhat questionable.  Wouldn't you want to
> reject
> 
> 	git cat-file --filter --path=Makefile HEAD:RelNotes
> 
> when HEAD:RelNotes blob is known to be a symbolic link?  After all,
> you would reject this
> 
> 	git cat-file --filter --path=Makefile HEAD:t/
> 
> and it is quite similar, no?  I think this can be argued both ways,
> but I have a feeling that obj_context.mode, if available, should be
> honored with or without force_path.

I see your point. All I wanted to do was to enable

	git cat-file --filters --path=Makefile cafebabe

in which case obj_context.mode == S_IFINVALID. I fixed it (see interdiff
of the next iteration).

> > diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
> > index e466634..fd17159 100755
> > --- a/t/t8010-cat-file-filters.sh
> > +++ b/t/t8010-cat-file-filters.sh
> > @@ -31,4 +31,17 @@ test_expect_success 'cat-file --filters converts to worktree version' '
> >  	has_cr actual
> >  '
> >  
> > +test_expect_success 'cat-file --filters --use-path=<path> works' '
> > +	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
> > +	git cat-file --filters --use-path=world.txt $sha1 >actual &&
> > +	has_cr actual
> > +'
> > +
> > +test_expect_success 'cat-file --textconv --use-path=<path> works' '
> > +	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
> > +	test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
> > +	git cat-file --textconv --use-path=hello.txt $sha1 >rot13 &&
> > +	test uryyb = "$(cat rot13 | remove_cr)"
> > +'
> 
> I think I saw some code to ensure "when giving this option you need
> that option in effect, too"; they should be tested here, too, no?

No, I would rather not test for that. These conditionals are purely for
any user's convenience, in case they specify an option that has no effect.
They are absolutely not essential for the function introduced in this
patch series.

Ciao,
Dscho

Re: [PATCH 2/4] cat-file: introduce the --filters option

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 632 bytes --]

Hi Torsten,

On Fri, 19 Aug 2016, Torsten Bögershausen wrote:

> On Thu, Aug 18, 2016 at 02:46:17PM +0200, Johannes Schindelin wrote:
> 
> > +--filters::
> > +	Show the content as transformed by the filters configured in
>
> Minor comment:
> s/transformed/converted/ ?

Sure.

> Does it make sense to be more specific here:
> The order of conversion is
> - ident
> - CRLF
> - smudge

I do not think it makes sense to complexify the documentation in that
manner. The filters should always be applied in the same order, methinks,
and it would only clutter the man page to repeat that order here.

Ciao,
Dscho

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Jeff King]

On Fri, Aug 19, 2016 at 02:25:51PM +0200, Johannes Schindelin wrote:

> > On Thu, Aug 18, 2016 at 02:46:28PM +0200, Johannes Schindelin wrote:
> > > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > > index 5ff58b3..5f91cf4 100644
> > > --- a/builtin/cat-file.c
> > > +++ b/builtin/cat-file.c
> > > @@ -17,6 +17,7 @@ struct batch_options {
> > >  	int print_contents;
> > >  	int buffer_output;
> > >  	int all_objects;
> > > +	int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
> > How do I read 'w' and 'c' ?
> > wilter and cextconv ? Does it make sense to use an enum here ?
> > Or a #define ?
> 
> Sorry, Torsten, this is not my doing. So I cannot explain why it is not an
> enum.
> 
> I *guess* the rationale for 'c' being the cmdmode of --textconv is "c for
> convert". That is why I chose "w as in worktree" as new cmdmode.

I think it started as a 'char' because it was representing the
single-letter options of "cat-file -e", "cat-file -s", etc. And then the
textconv patches started the monstrosity of "c".

I don't think cleaning it up needs to be part of your series, but it
might be nice low-hanging fruit for somebody to clean up on top.

-Peff

Re: [PATCH 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Actually, I find it much harder to debug these "the output must match
> these precise bytes, else we fail" type of test cases. Instead, I describe
> in a natural way what is expected:
>
> 	! has_cr actual
>
> Now, when the test fails, whoever is that poor soul tasked with debugging
> and fixing the breakage knows *what* goes wrong, conceptually.

I am of two minds but 60% in favor of the "We only care about
presense of CR" approach.  Of course, the remaining 40% is "other
people may break the code in such a way that still has CR at the end
of the line but change ealier bytes on the line in the future (an
obvious way to do so is to turn an input "ABC\n" into "AB\r\n"), and
we would want to catch it".  But the conversion machinery is tested
elsewhere in the test suite, and this test is more about the cat-file
command making a call into the conversion machinery when and only
when it is necessary, so that sways me towards "has_cr is what we
want here".

> Could you please start surrounding your replies by empty lines?

Good suggestion.  I have exactly the same problem whenever I read
messages without blanks at paragraph boundaries.

Thanks.

Re: [PATCH 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Does it make sense to be more specific here:
>> The order of conversion is
>> - ident
>> - CRLF
>> - smudge
>
> I do not think it makes sense to complexify the documentation in that
> manner.

Right.

Re: [PATCH 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> I think I saw some code to ensure "when giving this option you need
>> that option in effect, too"; they should be tested here, too, no?
>
> No, I would rather not test for that. These conditionals are purely for
> any user's convenience, in case they specify an option that has no effect.
> They are absolutely not essential for the function introduced in this
> patch series.

I didn't say "you would want to test these, no?", did I?

I do not want to see bugreports that say "I wanted to use this new
feature and by mistake gave only --path without giving --filter; Git
should have complained.  I found a bug, hooray!" when somebody in
the future refactors the command line option parsing and breaks the
check you already have.

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> Sorry, Torsten, this is not my doing. So I cannot explain why it is not an
>> enum.
>> 
>> I *guess* the rationale for 'c' being the cmdmode of --textconv is "c for
>> convert". That is why I chose "w as in worktree" as new cmdmode.
>
> I think it started as a 'char' because it was representing the
> single-letter options of "cat-file -e", "cat-file -s", etc. And then the
> textconv patches started the monstrosity of "c".

I think it was merely 't' was taken for "-t" (one beauty of using
the OPT_CMDMODE for options with shorthand is that we do not need
any enum, as the short-form options already form an enum), so
"textConv" was the compromise.  It could have been 'T' or even \C-t
;-)

> I don't think cleaning it up needs to be part of your series, but it
> might be nice low-hanging fruit for somebody to clean up on top.

I agree.

I briefly wondered if we want to add a check to ensure uniqueness of
these cmdmode letters in parse_options_check(), but that would
forbid us from having synonymous options (e.g. "am --continue" and
"am --resolved" both map to the same), so it would not work.

Re: [PATCH 2/4] cat-file: introduce the --filters option

[Torsten Bögershausen]

On 19.08.16 17:00, Johannes Schindelin wrote:
> Hi Torsten,
>
> On Fri, 19 Aug 2016, Torsten Bögershausen wrote:
>
>> On Thu, Aug 18, 2016 at 02:46:17PM +0200, Johannes Schindelin wrote:
>>
>>> +--filters::
>>> +	Show the content as transformed by the filters configured in
>> Minor comment:
>> s/transformed/converted/ ?
> Sure.
>
>> Does it make sense to be more specific here:
>> The order of conversion is
>> - ident
>> - CRLF
>> - smudge
> I do not think it makes sense to complexify the documentation in that
> manner. The filters should always be applied in the same order, methinks,
> and it would only clutter the man page to repeat that order here.
Can we can shorten the description and have something like this:

--filters::
+	Show the content converted by the filters configured in
+	the current working tree for the given <path>. 
+ 	<object> has to be of the form <tree-ish>:<path> or :<path>.
+

Re: [PATCH 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Johannes Schindelin]

Hi Junio,

On Fri, 19 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> I think I saw some code to ensure "when giving this option you need
> >> that option in effect, too"; they should be tested here, too, no?
> >
> > No, I would rather not test for that. These conditionals are purely for
> > any user's convenience, in case they specify an option that has no effect.
> > They are absolutely not essential for the function introduced in this
> > patch series.
> 
> I didn't say "you would want to test these, no?", did I?
> 
> I do not want to see bugreports that say "I wanted to use this new
> feature and by mistake gave only --path without giving --filter; Git
> should have complained.  I found a bug, hooray!" when somebody in
> the future refactors the command line option parsing and breaks the
> check you already have.

I added a test to verify that --path without --filters nor --textconv
complains.

Ciao,
Dscho

Re: [PATCH 2/4] cat-file: introduce the --filters option

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]

Hi Torsten,

On Mon, 22 Aug 2016, Torsten Bögershausen wrote:

> On 19.08.16 17:00, Johannes Schindelin wrote:
> >
> > On Fri, 19 Aug 2016, Torsten Bögershausen wrote:
> >
> >> On Thu, Aug 18, 2016 at 02:46:17PM +0200, Johannes Schindelin wrote:
> >>
> >>> +--filters::
> >>> +	Show the content as transformed by the filters configured in
> >> Minor comment:
> >> s/transformed/converted/ ?
> > Sure.
> >
> >> Does it make sense to be more specific here:
> >> The order of conversion is
> >> - ident
> >> - CRLF
> >> - smudge
> > I do not think it makes sense to complexify the documentation in that
> > manner. The filters should always be applied in the same order, methinks,
> > and it would only clutter the man page to repeat that order here.
> Can we can shorten the description and have something like this:
> 
> --filters::
> +	Show the content converted by the filters configured in
> +	the current working tree for the given <path>. 
> + 	<object> has to be of the form <tree-ish>:<path> or :<path>.

I do not want it shortened *that* much. I, for one, would be confused
reading about filters, as there are so many of them. So while I do not
want cat-file's man page to include an extensive description how filters
work, I do want it to have an indication which filters this option is
talking about.

Ciao,
Dscho

[PATCH v2 0/4] cat-file: optionally convert to worktree version

[Johannes Schindelin]

When third-party tools need to access to contents of blobs in the
database, they might be more interested in the worktree version than in
the "clean" version of said contents.

This branch introduces the --filters option to make that happen, the
--use-path option to provide the path separately if the blob name rather
than the tree name is availale, and offers batch support in which case
it expects the object names and the path on its input lines, separated
by white space.

The new --filters option is an obvious sibling of --textconv, and shares
the peculiar feature that the drivers (and end-of-line convention) are
determined from the current worktree, not from the attributes stored in
the revision that may have been part of the object name.

As --textconv is so similar to --filters, it was taught to understand
the --use-path option and it was made compatible with batch mode, too.

I briefly considered teaching the batch mode to extract the path from
object names if they are specified as <tree-ish>:<path>. The changes
would be quite intrusive, though, and uglify the code substanitially. So
I decided against that.

Changes vs v2:

- two commit messages were touched up

- --use-path=<path> was renamed to --path=<path>

- a test was added to verify that --path without --textconv/--filters
  errors out with helpful advice


Johannes Schindelin (4):
  cat-file: fix a grammo in the man page
  cat-file: introduce the --filters option
  cat-file --textconv/--filters: allow specifying the path separately
  cat-file: support --textconv/--filters in batch mode

 Documentation/git-cat-file.txt |  40 +++++++++++----
 builtin/cat-file.c             | 110 ++++++++++++++++++++++++++++++++++++++---
 t/t8010-cat-file-filters.sh    |  64 ++++++++++++++++++++++++
 3 files changed, 196 insertions(+), 18 deletions(-)
 create mode 100755 t/t8010-cat-file-filters.sh

Published-As: https://github.com/dscho/git/releases/tag/cat-file-filters-v2
Fetch-It-Via: git fetch https://github.com/dscho/git cat-file-filters-v2

Interdiff vs v1:

 diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
 index 1f4d954..204541c 100644
 --- a/Documentation/git-cat-file.txt
 +++ b/Documentation/git-cat-file.txt
 @@ -9,7 +9,7 @@ git-cat-file - Provide content or type and size information for repository objec
  SYNOPSIS
  --------
  [verse]
 -'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) [--use-path=<path>] <object>
 +'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) [--path=<path>] <object>
  'git cat-file' (--batch | --batch-check) [ --textconv | --filters ] [--follow-symlinks]
  
  DESCRIPTION
 @@ -63,12 +63,12 @@ OPTIONS
  	<path>.
  
  --filters::
 -	Show the content as transformed by the filters configured in
 +	Show the content as converted by the filters configured in
  	the current working tree for the given <path> (i.e. smudge filters,
  	end-of-line conversion, etc). In this case, <object> has to be of
  	the form <tree-ish>:<path>, or :<path>.
  
 ---use-path=<path>::
 +--path=<path>::
  	For use with --textconv or --filters, to allow specifying an object
  	name and a path separately, e.g. when it is difficult to figure out
  	the revision from which the blob came.
 diff --git a/builtin/cat-file.c b/builtin/cat-file.c
 index 5f91cf4..f8a3a08 100644
 --- a/builtin/cat-file.c
 +++ b/builtin/cat-file.c
 @@ -61,6 +61,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
  	struct object_info oi = {NULL};
  	struct strbuf sb = STRBUF_INIT;
  	unsigned flags = LOOKUP_REPLACE_OBJECT;
 +	const char *path = force_path;
  
  	if (unknown_type)
  		flags |= LOOKUP_UNKNOWN_OBJECT;
 @@ -68,6 +69,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
  	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
  		die("Not a valid object name %s", obj_name);
  
 +	if (!path)
 +		path = obj_context.path;
 +	else if (obj_context.mode == S_IFINVALID)
 +		obj_context.mode = 0100644;
 +
  	buf = NULL;
  	switch (opt) {
  	case 't':
 @@ -92,23 +98,21 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
  		return !has_sha1_file(sha1);
  
  	case 'w':
 -		if (!force_path && !obj_context.path[0])
 +		if (!path[0])
  			die("git cat-file --filters %s: <object> must be "
  			    "<sha1:path>", obj_name);
  
 -		if (filter_object(force_path ? force_path : obj_context.path,
 -				  force_path ? 0100644 : obj_context.mode,
 +		if (filter_object(path, obj_context.mode,
  				  sha1, &buf, &size))
  			return -1;
  		break;
  
  	case 'c':
 -		if (!force_path && !obj_context.path[0])
 +		if (!path[0])
  			die("git cat-file --textconv %s: <object> must be <sha1:path>",
  			    obj_name);
  
 -		if (textconv_object(force_path ? force_path : obj_context.path,
 -				    force_path ? 0100644 : obj_context.mode,
 +		if (textconv_object(path, obj_context.mode,
  				    sha1, 1, &buf, &size))
  			break;
  
 @@ -510,7 +514,7 @@ static int batch_objects(struct batch_options *opt)
  }
  
  static const char * const cat_file_usage[] = {
 -	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) [--use-path=<path>] <object>"),
 +	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) [--path=<path>] <object>"),
  	N_("git cat-file (--batch | --batch-check) [--follow-symlinks] [--textconv|--filters]"),
  	NULL
  };
 @@ -558,7 +562,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
  			    N_("for blob objects, run textconv on object's content"), 'c'),
  		OPT_CMDMODE(0, "filters", &opt,
  			    N_("for blob objects, run filters on object's content"), 'w'),
 -		OPT_STRING(0, "use-path", &force_path, N_("blob"),
 +		OPT_STRING(0, "path", &force_path, N_("blob"),
  			   N_("use a specific path for --textconv/--filters")),
  		OPT_BOOL(0, "allow-unknown-type", &unknown_type,
  			  N_("allow -s and -t to work with broken/corrupt objects")),
 @@ -609,12 +613,12 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
  	}
  
  	if (force_path && opt != 'c' && opt != 'w') {
 -		error("--use-path=<path> needs --textconv or --filters");
 +		error("--path=<path> needs --textconv or --filters");
  		usage_with_options(cat_file_usage, options);
  	}
  
  	if (force_path && batch.enabled) {
 -		error("--use-path=<path> incompatible with --batch");
 +		error("--path=<path> incompatible with --batch");
  		usage_with_options(cat_file_usage, options);
  	}
  
 diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
 index 89ae868..acdfa09 100755
 --- a/t/t8010-cat-file-filters.sh
 +++ b/t/t8010-cat-file-filters.sh
 @@ -31,19 +31,26 @@ test_expect_success 'cat-file --filters converts to worktree version' '
  	has_cr actual
  '
  
 -test_expect_success 'cat-file --filters --use-path=<path> works' '
 +test_expect_success 'cat-file --filters --path=<path> works' '
  	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
 -	git cat-file --filters --use-path=world.txt $sha1 >actual &&
 +	git cat-file --filters --path=world.txt $sha1 >actual &&
  	has_cr actual
  '
  
 -test_expect_success 'cat-file --textconv --use-path=<path> works' '
 +test_expect_success 'cat-file --textconv --path=<path> works' '
  	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
  	test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
 -	git cat-file --textconv --use-path=hello.txt $sha1 >rot13 &&
 +	git cat-file --textconv --path=hello.txt $sha1 >rot13 &&
  	test uryyb = "$(cat rot13 | remove_cr)"
  '
  
 +test_expect_success '----path=<path> complains without --textconv/--filters' '
 +	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
 +	test_must_fail git cat-file --path=hello.txt blob $sha1 >actual 2>err &&
 +	test ! -s actual &&
 +	grep "path.*needs.*filters" err
 +'
 +
  test_expect_success 'cat-file --textconv --batch works' '
  	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
  	test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&

-- 
2.10.0.rc1.99.gcd66998

base-commit: 2632c897f74b1cc9b5533f467da459b9ec725538

[PATCH v2 1/4] cat-file: fix a grammo in the man page

[Johannes Schindelin]

"... has be ..." -> "... has to be ..."

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-cat-file.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 18d03d8..071029b 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -54,8 +54,9 @@ OPTIONS
 
 --textconv::
 	Show the content as transformed by a textconv filter. In this case,
-	<object> has be of the form <tree-ish>:<path>, or :<path> in order
-	to apply the filter to the content recorded in the index at <path>.
+	<object> has to be of the form <tree-ish>:<path>, or :<path> in
+	order to apply the filter to the content recorded in the index at
+	<path>.
 
 --batch::
 --batch=<format>::
-- 
2.10.0.rc1.99.gcd66998

[PATCH v2 2/4] cat-file: introduce the --filters option

[Johannes Schindelin]

The --filters option applies the convert_to_working_tree() filter for
the path when showing the contents of a regular file blob object.

This feature comes in handy when a 3rd-party tool wants to work with
the contents of files from past revisions as if they had been checked
out, but without detouring via temporary files.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-cat-file.txt | 12 +++++++++---
 builtin/cat-file.c             | 41 ++++++++++++++++++++++++++++++++++++++++-
 t/t8010-cat-file-filters.sh    | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 4 deletions(-)
 create mode 100755 t/t8010-cat-file-filters.sh

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 071029b..537d02c 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -9,15 +9,15 @@ git-cat-file - Provide content or type and size information for repository objec
 SYNOPSIS
 --------
 [verse]
-'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv ) <object>
+'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) <object>
 'git cat-file' (--batch | --batch-check) [--follow-symlinks]
 
 DESCRIPTION
 -----------
 In its first form, the command provides the content or the type of an object in
 the repository. The type is required unless `-t` or `-p` is used to find the
-object type, or `-s` is used to find the object size, or `--textconv` is used
-(which implies type "blob").
+object type, or `-s` is used to find the object size, or `--textconv` or
+`--filters` is used (which imply type "blob").
 
 In the second form, a list of objects (separated by linefeeds) is provided on
 stdin, and the SHA-1, type, and size of each object is printed on stdout.
@@ -58,6 +58,12 @@ OPTIONS
 	order to apply the filter to the content recorded in the index at
 	<path>.
 
+--filters::
+	Show the content as converted by the filters configured in
+	the current working tree for the given <path> (i.e. smudge filters,
+	end-of-line conversion, etc). In this case, <object> has to be of
+	the form <tree-ish>:<path>, or :<path>.
+
 --batch::
 --batch=<format>::
 	Print object information and contents for each object provided
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 2dfe626..0b74afa 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,6 +20,33 @@ struct batch_options {
 	const char *format;
 };
 
+static int filter_object(const char *path, unsigned mode,
+			 const unsigned char *sha1,
+			 char **buf, unsigned long *size)
+{
+	enum object_type type;
+
+	*buf = read_sha1_file(sha1, &type, size);
+	if (!*buf)
+		return error(_("cannot read object %s '%s'"),
+			sha1_to_hex(sha1), path);
+	if (type != OBJ_BLOB) {
+		free(*buf);
+		return error(_("blob expected for %s '%s'"),
+			sha1_to_hex(sha1), path);
+	}
+	if (S_ISREG(mode)) {
+		struct strbuf strbuf = STRBUF_INIT;
+		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
+			free(*buf);
+			*size = strbuf.len;
+			*buf = strbuf_detach(&strbuf, NULL);
+		}
+	}
+
+	return 0;
+}
+
 static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 			int unknown_type)
 {
@@ -61,6 +88,16 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	case 'e':
 		return !has_sha1_file(sha1);
 
+	case 'w':
+		if (!obj_context.path[0])
+			die("git cat-file --filters %s: <object> must be "
+			    "<sha1:path>", obj_name);
+
+		if (filter_object(obj_context.path, obj_context.mode,
+				  sha1, &buf, &size))
+			return -1;
+		break;
+
 	case 'c':
 		if (!obj_context.path[0])
 			die("git cat-file --textconv %s: <object> must be <sha1:path>",
@@ -440,7 +477,7 @@ static int batch_objects(struct batch_options *opt)
 }
 
 static const char * const cat_file_usage[] = {
-	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv) <object>"),
+	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) <object>"),
 	N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
 	NULL
 };
@@ -486,6 +523,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE('p', NULL, &opt, N_("pretty-print object's content"), 'p'),
 		OPT_CMDMODE(0, "textconv", &opt,
 			    N_("for blob objects, run textconv on object's content"), 'c'),
+		OPT_CMDMODE(0, "filters", &opt,
+			    N_("for blob objects, run filters on object's content"), 'w'),
 		OPT_BOOL(0, "allow-unknown-type", &unknown_type,
 			  N_("allow -s and -t to work with broken/corrupt objects")),
 		OPT_BOOL(0, "buffer", &batch.buffer_output, N_("buffer --batch output")),
diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
new file mode 100755
index 0000000..e466634
--- /dev/null
+++ b/t/t8010-cat-file-filters.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='git cat-file filters support'
+. ./test-lib.sh
+
+test_expect_success 'setup ' '
+	echo "*.txt eol=crlf diff=txt" >.gitattributes &&
+	echo "hello" | append_cr >world.txt &&
+	git add .gitattributes world.txt &&
+	test_tick &&
+	git commit -m "Initial commit"
+'
+
+has_cr () {
+	tr '\015' Q <"$1" | grep Q >/dev/null
+}
+
+test_expect_success 'no filters with `git show`' '
+	git show HEAD:world.txt >actual &&
+	! has_cr actual
+
+'
+
+test_expect_success 'no filters with cat-file' '
+	git cat-file blob HEAD:world.txt >actual &&
+	! has_cr actual
+'
+
+test_expect_success 'cat-file --filters converts to worktree version' '
+	git cat-file --filters HEAD:world.txt >actual &&
+	has_cr actual
+'
+
+test_done
-- 
2.10.0.rc1.99.gcd66998

[PATCH v2 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Johannes Schindelin]

There are circumstances when it is relatively easy to figure out the
object name for a given path, but not the name of the containing tree.
For example, when looking at a diff generated by Git, the object names
are recorded, but not the revision. As a matter of fact, the revisions
from which the diff was generated may not even exist locally.

In such a case, the user would have to generate a fake revision just to
be able to use --textconv or --filters.

Let's simplify this dramatically, because we do not really need that
revision at all: all we care about is that we know the path. In the
scenario described above, we do know the path, and we just want to
specify it separately from the object name.

Example usage:

	git cat-file --textconv --path=main.c 0f1937fd

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-cat-file.txt |  7 ++++++-
 builtin/cat-file.c             | 26 +++++++++++++++++++++-----
 t/t8010-cat-file-filters.sh    | 20 ++++++++++++++++++++
 3 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 537d02c..4fa9041 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -9,7 +9,7 @@ git-cat-file - Provide content or type and size information for repository objec
 SYNOPSIS
 --------
 [verse]
-'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) <object>
+'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) [--path=<path>] <object>
 'git cat-file' (--batch | --batch-check) [--follow-symlinks]
 
 DESCRIPTION
@@ -64,6 +64,11 @@ OPTIONS
 	end-of-line conversion, etc). In this case, <object> has to be of
 	the form <tree-ish>:<path>, or :<path>.
 
+--path=<path>::
+	For use with --textconv or --filters, to allow specifying an object
+	name and a path separately, e.g. when it is difficult to figure out
+	the revision from which the blob came.
+
 --batch::
 --batch=<format>::
 	Print object information and contents for each object provided
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 0b74afa..2c799ac 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,6 +20,8 @@ struct batch_options {
 	const char *format;
 };
 
+static const char *force_path;
+
 static int filter_object(const char *path, unsigned mode,
 			 const unsigned char *sha1,
 			 char **buf, unsigned long *size)
@@ -58,6 +60,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	struct object_info oi = {NULL};
 	struct strbuf sb = STRBUF_INIT;
 	unsigned flags = LOOKUP_REPLACE_OBJECT;
+	const char *path = force_path;
 
 	if (unknown_type)
 		flags |= LOOKUP_UNKNOWN_OBJECT;
@@ -65,6 +68,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
 		die("Not a valid object name %s", obj_name);
 
+	if (!path)
+		path = obj_context.path;
+	else if (obj_context.mode == S_IFINVALID)
+		obj_context.mode = 0100644;
+
 	buf = NULL;
 	switch (opt) {
 	case 't':
@@ -89,21 +97,22 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 		return !has_sha1_file(sha1);
 
 	case 'w':
-		if (!obj_context.path[0])
+		if (!path[0])
 			die("git cat-file --filters %s: <object> must be "
 			    "<sha1:path>", obj_name);
 
-		if (filter_object(obj_context.path, obj_context.mode,
+		if (filter_object(path, obj_context.mode,
 				  sha1, &buf, &size))
 			return -1;
 		break;
 
 	case 'c':
-		if (!obj_context.path[0])
+		if (!path[0])
 			die("git cat-file --textconv %s: <object> must be <sha1:path>",
 			    obj_name);
 
-		if (textconv_object(obj_context.path, obj_context.mode, sha1, 1, &buf, &size))
+		if (textconv_object(path, obj_context.mode,
+				    sha1, 1, &buf, &size))
 			break;
 
 	case 'p':
@@ -477,7 +486,7 @@ static int batch_objects(struct batch_options *opt)
 }
 
 static const char * const cat_file_usage[] = {
-	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) <object>"),
+	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) [--path=<path>] <object>"),
 	N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
 	NULL
 };
@@ -525,6 +534,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 			    N_("for blob objects, run textconv on object's content"), 'c'),
 		OPT_CMDMODE(0, "filters", &opt,
 			    N_("for blob objects, run filters on object's content"), 'w'),
+		OPT_STRING(0, "path", &force_path, N_("blob"),
+			   N_("use a specific path for --textconv/--filters")),
 		OPT_BOOL(0, "allow-unknown-type", &unknown_type,
 			  N_("allow -s and -t to work with broken/corrupt objects")),
 		OPT_BOOL(0, "buffer", &batch.buffer_output, N_("buffer --batch output")),
@@ -567,6 +578,11 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		usage_with_options(cat_file_usage, options);
 	}
 
+	if (force_path && opt != 'c' && opt != 'w') {
+		error("--path=<path> needs --textconv or --filters");
+		usage_with_options(cat_file_usage, options);
+	}
+
 	if (batch.buffer_output < 0)
 		batch.buffer_output = batch.all_objects;
 
diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
index e466634..0d5c33e 100755
--- a/t/t8010-cat-file-filters.sh
+++ b/t/t8010-cat-file-filters.sh
@@ -31,4 +31,24 @@ test_expect_success 'cat-file --filters converts to worktree version' '
 	has_cr actual
 '
 
+test_expect_success 'cat-file --filters --path=<path> works' '
+	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
+	git cat-file --filters --path=world.txt $sha1 >actual &&
+	has_cr actual
+'
+
+test_expect_success 'cat-file --textconv --path=<path> works' '
+	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
+	test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
+	git cat-file --textconv --path=hello.txt $sha1 >rot13 &&
+	test uryyb = "$(cat rot13 | remove_cr)"
+'
+
+test_expect_success '----path=<path> complains without --textconv/--filters' '
+	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
+	test_must_fail git cat-file --path=hello.txt blob $sha1 >actual 2>err &&
+	test ! -s actual &&
+	grep "path.*needs.*filters" err
+'
+
 test_done
-- 
2.10.0.rc1.99.gcd66998

[PATCH v2 4/4] cat-file: support --textconv/--filters in batch mode

[Johannes Schindelin]

With this patch, --batch can be combined with --textconv or --filters.
For this to work, the input needs to have the form

	<object name><single white space><path>

so that the filters can be chosen appropriately.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-cat-file.txt | 18 +++++++++++-----
 builtin/cat-file.c             | 49 +++++++++++++++++++++++++++++++++++++-----
 t/t8010-cat-file-filters.sh    | 10 +++++++++
 3 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 4fa9041..204541c 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) [--path=<path>] <object>
-'git cat-file' (--batch | --batch-check) [--follow-symlinks]
+'git cat-file' (--batch | --batch-check) [ --textconv | --filters ] [--follow-symlinks]
 
 DESCRIPTION
 -----------
@@ -20,7 +20,11 @@ object type, or `-s` is used to find the object size, or `--textconv` or
 `--filters` is used (which imply type "blob").
 
 In the second form, a list of objects (separated by linefeeds) is provided on
-stdin, and the SHA-1, type, and size of each object is printed on stdout.
+stdin, and the SHA-1, type, and size of each object is printed on stdout. The
+output format can be overridden using the optional `<format>` argument. If
+either `--textconv` or `--filters` was specified, the input is expected to
+list the object names followed by the path name, separated by a single white
+space, so that the appropriate drivers can be determined.
 
 OPTIONS
 -------
@@ -72,13 +76,17 @@ OPTIONS
 --batch::
 --batch=<format>::
 	Print object information and contents for each object provided
-	on stdin.  May not be combined with any other options or arguments.
-	See the section `BATCH OUTPUT` below for details.
+	on stdin.  May not be combined with any other options or arguments
+	except `--textconv` or `--filters`, in which case the input lines
+	also need to specify the path, separated by white space.  See the
+	section `BATCH OUTPUT` below for details.
 
 --batch-check::
 --batch-check=<format>::
 	Print object information for each object provided on stdin.  May
-	not be combined with any other options or arguments.  See the
+	not be combined with any other options or arguments except
+	`--textconv` or `--filters`, in which case the input lines also
+	need to specify the path, separated by white space.  See the
 	section `BATCH OUTPUT` below for details.
 
 --batch-all-objects::
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 2c799ac..f8a3a08 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -17,6 +17,7 @@ struct batch_options {
 	int print_contents;
 	int buffer_output;
 	int all_objects;
+	int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
 	const char *format;
 };
 
@@ -285,7 +286,32 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
 	if (data->type == OBJ_BLOB) {
 		if (opt->buffer_output)
 			fflush(stdout);
-		if (stream_blob_to_fd(1, sha1, NULL, 0) < 0)
+		if (opt->cmdmode) {
+			char *contents;
+			unsigned long size;
+
+			if (!data->rest)
+				die("missing path for '%s'", sha1_to_hex(sha1));
+
+			if (opt->cmdmode == 'w') {
+				if (filter_object(data->rest, 0100644, sha1,
+						  &contents, &size))
+					die("could not convert '%s' %s",
+					    sha1_to_hex(sha1), data->rest);
+			} else if (opt->cmdmode == 'c') {
+				enum object_type type;
+				if (!textconv_object(data->rest, 0100644, sha1,
+						     1, &contents, &size))
+					contents = read_sha1_file(sha1, &type,
+								  &size);
+				if (!contents)
+					die("could not convert '%s' %s",
+					    sha1_to_hex(sha1), data->rest);
+			} else
+				die("BUG: invalid cmdmode: %c", opt->cmdmode);
+			batch_write(opt, contents, size);
+			free(contents);
+		} else if (stream_blob_to_fd(1, sha1, NULL, 0) < 0)
 			die("unable to stream %s to stdout", sha1_to_hex(sha1));
 	}
 	else {
@@ -422,6 +448,8 @@ static int batch_objects(struct batch_options *opt)
 	data.mark_query = 1;
 	strbuf_expand(&buf, opt->format, expand_format, &data);
 	data.mark_query = 0;
+	if (opt->cmdmode)
+		data.split_on_whitespace = 1;
 
 	if (opt->all_objects) {
 		struct object_info empty;
@@ -487,7 +515,7 @@ static int batch_objects(struct batch_options *opt)
 
 static const char * const cat_file_usage[] = {
 	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) [--path=<path>] <object>"),
-	N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
+	N_("git cat-file (--batch | --batch-check) [--follow-symlinks] [--textconv|--filters]"),
 	NULL
 };
 
@@ -558,7 +586,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, cat_file_usage, 0);
 
 	if (opt) {
-		if (argc == 1)
+		if (batch.enabled && (opt == 'c' || opt == 'w'))
+			batch.cmdmode = opt;
+		else if (argc == 1)
 			obj_name = argv[0];
 		else
 			usage_with_options(cat_file_usage, options);
@@ -570,8 +600,12 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		} else
 			usage_with_options(cat_file_usage, options);
 	}
-	if (batch.enabled && (opt || argc)) {
-		usage_with_options(cat_file_usage, options);
+	if (batch.enabled) {
+		if (batch.cmdmode != opt || argc)
+			usage_with_options(cat_file_usage, options);
+		if (batch.cmdmode && batch.all_objects)
+			die("--batch-all-objects cannot be combined with "
+			    "--textconv nor with --filters");
 	}
 
 	if ((batch.follow_symlinks || batch.all_objects) && !batch.enabled) {
@@ -583,6 +617,11 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		usage_with_options(cat_file_usage, options);
 	}
 
+	if (force_path && batch.enabled) {
+		error("--path=<path> incompatible with --batch");
+		usage_with_options(cat_file_usage, options);
+	}
+
 	if (batch.buffer_output < 0)
 		batch.buffer_output = batch.all_objects;
 
diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
index 0d5c33e..acdfa09 100755
--- a/t/t8010-cat-file-filters.sh
+++ b/t/t8010-cat-file-filters.sh
@@ -51,4 +51,14 @@ test_expect_success '----path=<path> complains without --textconv/--filters' '
 	grep "path.*needs.*filters" err
 '
 
+test_expect_success 'cat-file --textconv --batch works' '
+	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
+	test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
+	printf "%s hello.txt\n%s hello\n" $sha1 $sha1 |
+	git cat-file --textconv --batch >actual &&
+	printf "%s blob 6\nuryyb\r\n\n%s blob 6\nhello\n\n" \
+		$sha1 $sha1 >expect &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.10.0.rc1.99.gcd66998

Re: [PATCH v2 0/4] cat-file: optionally convert to worktree version

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

>  diff --git a/builtin/cat-file.c b/builtin/cat-file.c
>  index 5f91cf4..f8a3a08 100644
>  --- a/builtin/cat-file.c
>  +++ b/builtin/cat-file.c
>  @@ -61,6 +61,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
>   	struct object_info oi = {NULL};
>   	struct strbuf sb = STRBUF_INIT;
>   	unsigned flags = LOOKUP_REPLACE_OBJECT;
>  +	const char *path = force_path;
>   
>   	if (unknown_type)
>   		flags |= LOOKUP_UNKNOWN_OBJECT;
>  @@ -68,6 +69,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
>   	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
>   		die("Not a valid object name %s", obj_name);
>   
>  +	if (!path)
>  +		path = obj_context.path;
>  +	else if (obj_context.mode == S_IFINVALID)
>  +		obj_context.mode = 0100644;
>  +
>   	buf = NULL;
>   	switch (opt) {
>   	case 't':

The above two hunks make all the difference in the ease of reading
the remainder of the function.  Very good.

>  +test_expect_success '----path=<path> complains without --textconv/--filters' '
>  +	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
>  +	test_must_fail git cat-file --path=hello.txt blob $sha1 >actual 2>err &&
>  +	test ! -s actual &&
>  +	grep "path.*needs.*filters" err
>  +'

This will need to become test_i18ngrep once the error message is
made translatable, but it is correct for now.  I personally think
there is no need to check "actual" or "err", though---just running
cat-file under test_must_fail should be sufficient.

Thanks, will queue.

Re: [PATCH v2 0/4] cat-file: optionally convert to worktree version

[Jeff King]

On Wed, Aug 24, 2016 at 09:09:06AM -0700, Junio C Hamano wrote:

> >  +	if (!path)
> >  +		path = obj_context.path;
> >  +	else if (obj_context.mode == S_IFINVALID)
> >  +		obj_context.mode = 0100644;
> >  +
> >   	buf = NULL;
> >   	switch (opt) {
> >   	case 't':
> 
> The above two hunks make all the difference in the ease of reading
> the remainder of the function.  Very good.

Yeah, I agree. Though it took me a moment to figure out why we were
setting obj_context.mode but not obj_context.path; the reason is that
"mode" is convenient to use as local storage, but "path" is not, because
it is not a pointer but an array.

So it would have been a little clearer to me as:

  const char *path;
  unsigned mode;
  ...
  if (!force_path) {
	/* use file info from sha1 lookup */
	path = obj_context.path;
	mode = obj_context.mode;
  } else {
	/* use path requested by user, and assume it is a regular file */
	path = force_path;
	mode = 0100644;
  }

but I don't know if that is even worth a re-roll.

> >  +test_expect_success '----path=<path> complains without --textconv/--filters' '
> >  +	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
> >  +	test_must_fail git cat-file --path=hello.txt blob $sha1 >actual 2>err &&
> >  +	test ! -s actual &&
> >  +	grep "path.*needs.*filters" err
> >  +'
> 
> This will need to become test_i18ngrep once the error message is
> made translatable, but it is correct for now.  I personally think
> there is no need to check "actual" or "err", though---just running
> cat-file under test_must_fail should be sufficient.
> 
> Thanks, will queue.

The whole thing looks good to me, except for the weird doubled "--" in
the test description you quoted above. :)

-Peff

Re: [PATCH v2 0/4] cat-file: optionally convert to worktree version

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Aug 24, 2016 at 09:09:06AM -0700, Junio C Hamano wrote:
>
>> >  +	if (!path)
>> >  +		path = obj_context.path;
>> >  +	else if (obj_context.mode == S_IFINVALID)
>> >  +		obj_context.mode = 0100644;
>> >  +
>> >   	buf = NULL;
>> >   	switch (opt) {
>> >   	case 't':
>> 
>> The above two hunks make all the difference in the ease of reading
>> the remainder of the function.  Very good.
>
> Yeah, I agree. Though it took me a moment to figure out why we were
> setting obj_context.mode but not obj_context.path; the reason is that
> "mode" is convenient to use as local storage, but "path" is not, because
> it is not a pointer but an array.

Wait a minute.  Why is it a cascaded if/elseif, not two independent
if statements that gives a default value?  In other words, wouldn't
these two independent and orthogonal decisions?

 * When forced to use some path, we ignore obj_context.path

 * Whether we are forced to use a path or not, if we do not know the
   mode from the lookup context, we want to use the regular blob
   mode.

So that part of the patch is wrong after all, I would have to say.

	if (!path)
        	path = obj_context.path;
	if (obj_context.mode == S_IFINVALID)
        	obj_context.mode = 0100644;

or something like that, perhaps.

> So it would have been a little clearer to me as:
>
>   const char *path;
>   unsigned mode;
>   ...
>   if (!force_path) {
> 	/* use file info from sha1 lookup */
> 	path = obj_context.path;
> 	mode = obj_context.mode;
>   } else {
> 	/* use path requested by user, and assume it is a regular file */
> 	path = force_path;
> 	mode = 0100644;
>   }

Hmph, if you read it that way, then if/elseif makes some sense, but
we need to assume that the obj_context.mode can be garbage and have
a fallback for it.

Just like

	git cat-file --filters --path=git.c HEAD:t

would error out because HEAD:t is not even a blob, I would expect

	git cat-file --filters --path=git.c :RelNotes

to error out, because the object itself _is_ known to be a
blob that is not a regular file.

And that kind of type checking will not be possible with "if the
user gave us a path, assume it is a regular file".

Re: [PATCH v2 0/4] cat-file: optionally convert to worktree version

[Jeff King]

On Wed, Aug 24, 2016 at 10:02:39AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Wed, Aug 24, 2016 at 09:09:06AM -0700, Junio C Hamano wrote:
> >
> >> >  +	if (!path)
> >> >  +		path = obj_context.path;
> >> >  +	else if (obj_context.mode == S_IFINVALID)
> >> >  +		obj_context.mode = 0100644;
> >> >  +
> >> >   	buf = NULL;
> >> >   	switch (opt) {
> >> >   	case 't':
> >> 
> >> The above two hunks make all the difference in the ease of reading
> >> the remainder of the function.  Very good.
> >
> > Yeah, I agree. Though it took me a moment to figure out why we were
> > setting obj_context.mode but not obj_context.path; the reason is that
> > "mode" is convenient to use as local storage, but "path" is not, because
> > it is not a pointer but an array.
> 
> Wait a minute.  Why is it a cascaded if/elseif, not two independent
> if statements that gives a default value?  In other words, wouldn't
> these two independent and orthogonal decisions?
> 
>  * When forced to use some path, we ignore obj_context.path
> 
>  * Whether we are forced to use a path or not, if we do not know the
>    mode from the lookup context, we want to use the regular blob
>    mode.
> 
> So that part of the patch is wrong after all, I would have to say.
> 
> 	if (!path)
>         	path = obj_context.path;
> 	if (obj_context.mode == S_IFINVALID)
>         	obj_context.mode = 0100644;
> 
> or something like that, perhaps.

Oh, hrm, you are right. I assumed we wanted to force the mode when
--path was in effect, but that is not what the original does. If you
say:

  --path=foo HEAD:bar

then we will take the mode for "bar", whatever it is (maybe a tree or
symlink). But if you say:

  --path=foo $(git rev-parse HEAD:bar)

then we will use 100644, regardless of what "bar" is in HEAD.

I have not thought about it enough to know if that is a good thing or a
bad thing. But I'll bet Dscho has, so I will wait for him to comment. :)

> >   if (!force_path) {
> > 	/* use file info from sha1 lookup */
> > 	path = obj_context.path;
> > 	mode = obj_context.mode;
> >   } else {
> > 	/* use path requested by user, and assume it is a regular file */
> > 	path = force_path;
> > 	mode = 0100644;
> >   }
> 
> Hmph, if you read it that way, then if/elseif makes some sense, but
> we need to assume that the obj_context.mode can be garbage and have
> a fallback for it.
> 
> Just like
> 
> 	git cat-file --filters --path=git.c HEAD:t
> 
> would error out because HEAD:t is not even a blob, I would expect
> 
> 	git cat-file --filters --path=git.c :RelNotes
> 
> to error out, because the object itself _is_ known to be a
> blob that is not a regular file.
> 
> And that kind of type checking will not be possible with "if the
> user gave us a path, assume it is a regular file".

Right, I agree that is the outcome, but I just wasn't sure that the
second case _should_ error out. IOW, does "--filters --path" mean "treat
this as a regular file at path X", or is the "regular file" part not
implied?

I don't suppose anybody cares that much either way, but it feels weird
to behave differently depending on how we looked up the blob (whereas
for the HEAD:t case, a tree is always a tree).

-Peff

Re: [PATCH v2 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> +static int filter_object(const char *path, unsigned mode,
> +			 const unsigned char *sha1,
> +			 char **buf, unsigned long *size)
> +{
> +	enum object_type type;
> +
> +	*buf = read_sha1_file(sha1, &type, size);
> +	if (!*buf)
> +		return error(_("cannot read object %s '%s'"),
> +			sha1_to_hex(sha1), path);
> +	if (type != OBJ_BLOB) {
> +		free(*buf);
> +		return error(_("blob expected for %s '%s'"),
> +			sha1_to_hex(sha1), path);
> +	}
> +	if (S_ISREG(mode)) {
> +		struct strbuf strbuf = STRBUF_INIT;
> +		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
> +			free(*buf);
> +			*size = strbuf.len;
> +			*buf = strbuf_detach(&strbuf, NULL);
> +		}
> +	}

When we see a blob that is not ISREG, what is expected to happen?
Is it an error?

We can argue both ways.

Currently the ONLY kind of blob that is not ISREG is a symbolic
link, and it might be OK to output it as-is without any conversion
[*1*], in which case we can simply lose the S_ISREG(mode) check
altogether (and "mode" parameter to this function).

On the other hand, because "cat-file --filters" is meant to be a
smaller-granularity operation that could be used as a building block
to emulate what "git checkout" does, i.e. "imagine that we had the
blob in the index and checking it out from the path, and hand the
caller what would have been written out to the filesystem, so that
the convert_to_working_tree() logic does not have to be emulated in
the userspace", erroring out when we see a symbolic link may be also
a valid way to handle it.  We know the usual CRLF and other conversions
do not apply to them.

In any case, silently succeeding without any output is probably what
the end-user least expects.

If we choose to fail the request, the necessary update to the test
would look like this, I think.

> diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
> new file mode 100755
> index 0000000..e466634
> --- /dev/null
> +++ b/t/t8010-cat-file-filters.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +test_description='git cat-file filters support'
> +. ./test-lib.sh
> +
> +test_expect_success 'setup ' '
> +	echo "*.txt eol=crlf diff=txt" >.gitattributes &&
> +	echo "hello" | append_cr >world.txt &&
> +	git add .gitattributes world.txt &&

	git update-index --cacheinfo :world.txt,$EMPTY_BLOB,symlink &&

> +	test_tick &&
> +	git commit -m "Initial commit"
> +'
> +
> +has_cr () {
> +	tr '\015' Q <"$1" | grep Q >/dev/null
> +}
> +
> +test_expect_success 'no filters with `git show`' '
> +	git show HEAD:world.txt >actual &&
> +	! has_cr actual
> +
> +'
> +
> +test_expect_success 'no filters with cat-file' '
> +	git cat-file blob HEAD:world.txt >actual &&
> +	! has_cr actual
> +'
> +
> +test_expect_success 'cat-file --filters converts to worktree version' '
> +	git cat-file --filters HEAD:world.txt >actual &&
> +	has_cr actual
> +'

test_expect_success 'cat-file --filters rejects a non-blob' '
	test_must_fail git cat-file --filters HEAD:
'

test_expect_success 'cat-file --filters rejects a non-regular blob' '
	test_must_fail git cat-file --filters HEAD:symlink
'

And if we choose to accept as long as it is a blob, then the last
step can lose test_must_fail (and perhaps the result needs to be
checked against "hello" without CR).


[Footnote]

*1* But of course other kinds of non-ISREG blob we would add later
    may not be something you would want to write out as-is.

Re: [PATCH v2 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> @@ -65,6 +68,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
>  	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
>  		die("Not a valid object name %s", obj_name);
>  
> +	if (!path)
> +		path = obj_context.path;
> +	else if (obj_context.mode == S_IFINVALID)
> +		obj_context.mode = 0100644;
> +
>  	buf = NULL;
>  	switch (opt) {
>  	case 't':

Mentioned elsewhere, but I think the above should be

	if (!path)
        	path = obj_context.path;

	if (obj_context.mode == S_IFINVALID)
        	obj_context.mode = 0100644;

IOW, even when there is an explicit path supplied, we should fall
back to assumed "regular blob" mode, so that

	git cat-file --filters --path=README $(git rev-parse :README)

would work as expected.

Re: [PATCH v2 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> +test_expect_success 'setup ' '
>> +	echo "*.txt eol=crlf diff=txt" >.gitattributes &&
>> +	echo "hello" | append_cr >world.txt &&
>> +	git add .gitattributes world.txt &&
>
> 	git update-index --cacheinfo :world.txt,$EMPTY_BLOB,symlink &&

Sorry, last-minute-edit-gone-bad-without-proofreading.  It should
have been something like:

	git update-index --add --cacheinfo 160000,$EMPTY_BLOB,symlink &&

or

       	sym=$(echo "hello" | git hash-objects --stdin -w) &&
	git update-index --add --cacheinfo 160000,$sym,symlink &&

Re: [PATCH v2 0/4] cat-file: optionally convert to worktree version

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I don't suppose anybody cares that much either way, but it feels weird
> to behave differently depending on how we looked up the blob (whereas
> for the HEAD:t case, a tree is always a tree).

I do not care strongly either way, but HEAD:RelNotes case we _know_
it is not a regular blob, and discarding that info for the sake of
being consistent with the common denominator case, instead of using
it to point out a possible mistake, feels wrong, too.

In any case, not outputting anything and silently succeeding is
definitely wrong ;-).

Re: [PATCH v2 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Mentioned elsewhere, but I think the above should be
>
> 	if (!path)
>         	path = obj_context.path;
>
> 	if (obj_context.mode == S_IFINVALID)
>         	obj_context.mode = 0100644;
>
> IOW, even when there is an explicit path supplied, we should fall
> back to assumed "regular blob" mode, so that
>
> 	git cat-file --filters --path=README $(git rev-parse :README)
>
> would work as expected.

Actually, I am reading the conditional the other way, but the
conclusion "defaulting from unknown mode to regular blob is
necessary whether the user gave us a path or not" is the same.

The current code may fail if --path is not available and 40-hex that
does not give us any context of look up is given because it won't be
able to decide how to filter, so using "else if" would not have
practical difference there, but conceptually it still is wrong.

Re: [PATCH v2 0/4] cat-file: optionally convert to worktree version

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> When third-party tools need to access to contents of blobs in the
> database, they might be more interested in the worktree version than in
> the "clean" version of said contents.

Just a friendly reminder before you completely shift your attention
to unrelated topics.  I think this topic is almost there; let's not
stretch ourselves too thin by nibbling-here-nibbling-there and leaving
loose ends untied.

Thanks.

Re: [PATCH v2 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Johannes Schindelin]

Hi Junio,

On Wed, 24 Aug 2016, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Mentioned elsewhere, but I think the above should be
> >
> > 	if (!path)
> >         	path = obj_context.path;
> >
> > 	if (obj_context.mode == S_IFINVALID)
> >         	obj_context.mode = 0100644;
> >
> > IOW, even when there is an explicit path supplied, we should fall
> > back to assumed "regular blob" mode, so that
> >
> > 	git cat-file --filters --path=README $(git rev-parse :README)
> >
> > would work as expected.
> 
> Actually, I am reading the conditional the other way, but the
> conclusion "defaulting from unknown mode to regular blob is
> necessary whether the user gave us a path or not" is the same.
> 
> The current code may fail if --path is not available and 40-hex that
> does not give us any context of look up is given because it won't be
> able to decide how to filter, so using "else if" would not have
> practical difference there, but conceptually it still is wrong.

Let's translate the logic

	if (!path)
		path = obj_context.path;
	else if (obj_context.mode == S_IFINVALID)
		obj_context.mode = 0100644;

into plain English.

Basically, we have two cases:

1) the user provided us with a --path option, in which case we only
   override the mode if get_sha1_with_context() could not determine the
   mode.

2) the user did *not* provide us with a --path, in which case we keep the
   mode exactly as determined by get_sha1_with_context().

Now, the change you propose would change 2) such that we would *still*
override an undecided mode with 100644, even if the user did not bother to
specify a --path option.

It is true that it is currently impossible to infer a path from a blob ID
without being able to infer also a mode. So the question is whether it
makes sense to allow cat-file to proceed on the assumption that it is a
regular file if it does not know?

I would say: no, it does not make sense.

However, I do not want to hold this patch series up just by being
stubborn.

Will change it,
Dscho

Re: [PATCH v2 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> However, I do not want to hold this patch series up just by being
> stubborn.

Peff and I disussed this further in the thread in which the message
<xmqqa8g2qcf3.fsf@gitster.mtv.corp.google.com> appears, and agreed
that it does not matter that much either way.

The comment to [v2 2/4] would be more important than this one, I
would think.  What should happen when a blob that is not ISREG is
given to filter_object()?  Giving a symlink contents as-is without
filtering would be OK.  Erroring out saying "regular file blob
expected" just like the function reacts to non-blob is also OK.
Just being silent and not doing anything is probably not.

Thanks.

Re: [PATCH v2 2/4] cat-file: introduce the --filters option

[Johannes Schindelin]

Hi Junio,

On Wed, 24 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > +static int filter_object(const char *path, unsigned mode,
> > +			 const unsigned char *sha1,
> > +			 char **buf, unsigned long *size)
> > +{
> > +	enum object_type type;
> > +
> > +	*buf = read_sha1_file(sha1, &type, size);
> > +	if (!*buf)
> > +		return error(_("cannot read object %s '%s'"),
> > +			sha1_to_hex(sha1), path);
> > +	if (type != OBJ_BLOB) {
> > +		free(*buf);
> > +		return error(_("blob expected for %s '%s'"),
> > +			sha1_to_hex(sha1), path);
> > +	}
> > +	if (S_ISREG(mode)) {
> > +		struct strbuf strbuf = STRBUF_INIT;
> > +		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
> > +			free(*buf);
> > +			*size = strbuf.len;
> > +			*buf = strbuf_detach(&strbuf, NULL);
> > +		}
> > +	}
> 
> When we see a blob that is not ISREG, what is expected to happen?
> Is it an error?

This is not a user-facing command, therefore we have to trust the caller
that they know what they are doing.

And it is quite conceivable that a caller wants to simply apply filters
whenever a blob is specified, and simply not apply any filters when
something else was specified.

That would be in line with specifying the --filters options on a path for
which no filters are configured: the --filters option is basically a
no-op, then.

> In any case, silently succeeding without any output is probably what
> the end-user least expects.

Except that 1) the command is not for an end-user, and 2) when calling
`git cat-file --filters HEAD:directory/` the command does not silently
succeed:

	error: blob expected for <sha-1> 'directory/'

> If we choose to fail the request, the necessary update to the test
> would look like this, I think.

Quite frankly, as cat-file is not an end-user-facing command, I think it
is serious overkill to add more testing here.

Ciao,
Dscho

Re: [PATCH v2 0/4] cat-file: optionally convert to worktree version

[Johannes Schindelin]

Hi Junio,

On Mon, 29 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > When third-party tools need to access to contents of blobs in the
> > database, they might be more interested in the worktree version than in
> > the "clean" version of said contents.
> 
> Just a friendly reminder before you completely shift your attention
> to unrelated topics.  I think this topic is almost there; let's not
> stretch ourselves too thin by nibbling-here-nibbling-there and leaving
> loose ends untied.

I answered the comments and changed one thing locally. Will send out v3
tomorrow.

Ciao,
Dscho

Re: [PATCH v2 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> > +	if (S_ISREG(mode)) {
>> > +		struct strbuf strbuf = STRBUF_INIT;
>> > +		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
>> > +			free(*buf);
>> > +			*size = strbuf.len;
>> > +			*buf = strbuf_detach(&strbuf, NULL);
>> > +		}
>> > +	}
>> 
>> When we see a blob that is not ISREG, what is expected to happen?
>> Is it an error?
>
> This is not a user-facing command, therefore we have to trust the caller
> that they know what they are doing.

The caller that knows what s/he is doing would rely on a documented
behaviour out of the command.  That behaviour hopefully is an
intuitive and useful one for script writers.

You say

> Quite frankly, as cat-file is not an end-user-facing command, I think it
> is serious overkill to add more testing here.

and I think you would need a serious attitude adjustment here.
Scriptors are also an important class of end-users and cat-file
directly faces them.

Thinking about this one a bit more, as 'cat-file' especially with
the "--filters" option is the lowest-level way for scriptors to
externalize the data stored in Git object database to the
representation used in the outside world (in other words, it would
be a good ingredient if they want to implement what "checkout"
does), I would expect that an intuitive behaviour for

	git cat-file --filters HEAD:Makefile >Makefile
	git cat-file --filters HEAD:RelNotes >Relnotes
	git cat-file --filters HEAD:t >t

to send the requested contents to the standard output stream, but
error out when the result of the command shown above would not
mimick what "checkout" would leave in the filesystem.  IOW, the
first one should succeed, the second and the third ones should fail
the same way to signal what is requested cannot be performed to the
script that is calling these commands.

[PATCH v3 0/4] cat-file: optionally convert to worktree version

[Johannes Schindelin]

When third-party tools need to access to contents of blobs in the
database, they might be more interested in the worktree version than in
the "clean" version of said contents.

This branch introduces the --filters option to make that happen, the
--use-path option to provide the path separately if the blob name rather
than the tree name is availale, and offers batch support in which case
it expects the object names and the path on its input lines, separated
by white space.

The new --filters option is an obvious sibling of --textconv, and shares
the peculiar feature that the drivers (and end-of-line convention) are
determined from the current worktree, not from the attributes stored in
the revision that may have been part of the object name.

As --textconv is so similar to --filters, it was taught to understand
the --use-path option and it was made compatible with batch mode, too.

I briefly considered teaching the batch mode to extract the path from
object names if they are specified as <tree-ish>:<path>. The changes
would be quite intrusive, though, and uglify the code substanitially. So
I decided against that.

Changes vs v2:

- always override unknown mode to imply 0100644, even if the user did
  not specify the --path option.


Johannes Schindelin (4):
  cat-file: fix a grammo in the man page
  cat-file: introduce the --filters option
  cat-file --textconv/--filters: allow specifying the path separately
  cat-file: support --textconv/--filters in batch mode

 Documentation/git-cat-file.txt |  40 +++++++++++----
 builtin/cat-file.c             | 110 ++++++++++++++++++++++++++++++++++++++---
 t/t8010-cat-file-filters.sh    |  64 ++++++++++++++++++++++++
 3 files changed, 196 insertions(+), 18 deletions(-)
 create mode 100755 t/t8010-cat-file-filters.sh

Published-As: https://github.com/dscho/git/releases/tag/cat-file-filters-v3
Fetch-It-Via: git fetch https://github.com/dscho/git cat-file-filters-v3

Interdiff vs v2:

 diff --git a/builtin/cat-file.c b/builtin/cat-file.c
 index f8a3a08..4461153 100644
 --- a/builtin/cat-file.c
 +++ b/builtin/cat-file.c
 @@ -71,7 +71,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
  
  	if (!path)
  		path = obj_context.path;
 -	else if (obj_context.mode == S_IFINVALID)
 +	if (obj_context.mode == S_IFINVALID)
  		obj_context.mode = 0100644;
  
  	buf = NULL;

-- 
2.10.0.windows.1.10.g803177d

base-commit: 6ebdac1bab966b720d776aa43ca188fe378b1f4b

[PATCH v3 1/4] cat-file: fix a grammo in the man page

[Johannes Schindelin]

"... has be ..." -> "... has to be ..."

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-cat-file.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 18d03d8..071029b 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -54,8 +54,9 @@ OPTIONS
 
 --textconv::
 	Show the content as transformed by a textconv filter. In this case,
-	<object> has be of the form <tree-ish>:<path>, or :<path> in order
-	to apply the filter to the content recorded in the index at <path>.
+	<object> has to be of the form <tree-ish>:<path>, or :<path> in
+	order to apply the filter to the content recorded in the index at
+	<path>.
 
 --batch::
 --batch=<format>::
-- 
2.10.0.windows.1.10.g803177d

[PATCH v3 2/4] cat-file: introduce the --filters option

[Johannes Schindelin]

The --filters option applies the convert_to_working_tree() filter for
the path when showing the contents of a regular file blob object.

This feature comes in handy when a 3rd-party tool wants to work with
the contents of files from past revisions as if they had been checked
out, but without detouring via temporary files.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-cat-file.txt | 12 +++++++++---
 builtin/cat-file.c             | 41 ++++++++++++++++++++++++++++++++++++++++-
 t/t8010-cat-file-filters.sh    | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 4 deletions(-)
 create mode 100755 t/t8010-cat-file-filters.sh

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 071029b..537d02c 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -9,15 +9,15 @@ git-cat-file - Provide content or type and size information for repository objec
 SYNOPSIS
 --------
 [verse]
-'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv ) <object>
+'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) <object>
 'git cat-file' (--batch | --batch-check) [--follow-symlinks]
 
 DESCRIPTION
 -----------
 In its first form, the command provides the content or the type of an object in
 the repository. The type is required unless `-t` or `-p` is used to find the
-object type, or `-s` is used to find the object size, or `--textconv` is used
-(which implies type "blob").
+object type, or `-s` is used to find the object size, or `--textconv` or
+`--filters` is used (which imply type "blob").
 
 In the second form, a list of objects (separated by linefeeds) is provided on
 stdin, and the SHA-1, type, and size of each object is printed on stdout.
@@ -58,6 +58,12 @@ OPTIONS
 	order to apply the filter to the content recorded in the index at
 	<path>.
 
+--filters::
+	Show the content as converted by the filters configured in
+	the current working tree for the given <path> (i.e. smudge filters,
+	end-of-line conversion, etc). In this case, <object> has to be of
+	the form <tree-ish>:<path>, or :<path>.
+
 --batch::
 --batch=<format>::
 	Print object information and contents for each object provided
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 2dfe626..0b74afa 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,6 +20,33 @@ struct batch_options {
 	const char *format;
 };
 
+static int filter_object(const char *path, unsigned mode,
+			 const unsigned char *sha1,
+			 char **buf, unsigned long *size)
+{
+	enum object_type type;
+
+	*buf = read_sha1_file(sha1, &type, size);
+	if (!*buf)
+		return error(_("cannot read object %s '%s'"),
+			sha1_to_hex(sha1), path);
+	if (type != OBJ_BLOB) {
+		free(*buf);
+		return error(_("blob expected for %s '%s'"),
+			sha1_to_hex(sha1), path);
+	}
+	if (S_ISREG(mode)) {
+		struct strbuf strbuf = STRBUF_INIT;
+		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
+			free(*buf);
+			*size = strbuf.len;
+			*buf = strbuf_detach(&strbuf, NULL);
+		}
+	}
+
+	return 0;
+}
+
 static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 			int unknown_type)
 {
@@ -61,6 +88,16 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	case 'e':
 		return !has_sha1_file(sha1);
 
+	case 'w':
+		if (!obj_context.path[0])
+			die("git cat-file --filters %s: <object> must be "
+			    "<sha1:path>", obj_name);
+
+		if (filter_object(obj_context.path, obj_context.mode,
+				  sha1, &buf, &size))
+			return -1;
+		break;
+
 	case 'c':
 		if (!obj_context.path[0])
 			die("git cat-file --textconv %s: <object> must be <sha1:path>",
@@ -440,7 +477,7 @@ static int batch_objects(struct batch_options *opt)
 }
 
 static const char * const cat_file_usage[] = {
-	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv) <object>"),
+	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) <object>"),
 	N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
 	NULL
 };
@@ -486,6 +523,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE('p', NULL, &opt, N_("pretty-print object's content"), 'p'),
 		OPT_CMDMODE(0, "textconv", &opt,
 			    N_("for blob objects, run textconv on object's content"), 'c'),
+		OPT_CMDMODE(0, "filters", &opt,
+			    N_("for blob objects, run filters on object's content"), 'w'),
 		OPT_BOOL(0, "allow-unknown-type", &unknown_type,
 			  N_("allow -s and -t to work with broken/corrupt objects")),
 		OPT_BOOL(0, "buffer", &batch.buffer_output, N_("buffer --batch output")),
diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
new file mode 100755
index 0000000..e466634
--- /dev/null
+++ b/t/t8010-cat-file-filters.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='git cat-file filters support'
+. ./test-lib.sh
+
+test_expect_success 'setup ' '
+	echo "*.txt eol=crlf diff=txt" >.gitattributes &&
+	echo "hello" | append_cr >world.txt &&
+	git add .gitattributes world.txt &&
+	test_tick &&
+	git commit -m "Initial commit"
+'
+
+has_cr () {
+	tr '\015' Q <"$1" | grep Q >/dev/null
+}
+
+test_expect_success 'no filters with `git show`' '
+	git show HEAD:world.txt >actual &&
+	! has_cr actual
+
+'
+
+test_expect_success 'no filters with cat-file' '
+	git cat-file blob HEAD:world.txt >actual &&
+	! has_cr actual
+'
+
+test_expect_success 'cat-file --filters converts to worktree version' '
+	git cat-file --filters HEAD:world.txt >actual &&
+	has_cr actual
+'
+
+test_done
-- 
2.10.0.windows.1.10.g803177d

[PATCH v3 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Johannes Schindelin]

There are circumstances when it is relatively easy to figure out the
object name for a given path, but not the name of the containing tree.
For example, when looking at a diff generated by Git, the object names
are recorded, but not the revision. As a matter of fact, the revisions
from which the diff was generated may not even exist locally.

In such a case, the user would have to generate a fake revision just to
be able to use --textconv or --filters.

Let's simplify this dramatically, because we do not really need that
revision at all: all we care about is that we know the path. In the
scenario described above, we do know the path, and we just want to
specify it separately from the object name.

Example usage:

	git cat-file --textconv --path=main.c 0f1937fd

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-cat-file.txt |  7 ++++++-
 builtin/cat-file.c             | 26 +++++++++++++++++++++-----
 t/t8010-cat-file-filters.sh    | 20 ++++++++++++++++++++
 3 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 537d02c..4fa9041 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -9,7 +9,7 @@ git-cat-file - Provide content or type and size information for repository objec
 SYNOPSIS
 --------
 [verse]
-'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) <object>
+'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) [--path=<path>] <object>
 'git cat-file' (--batch | --batch-check) [--follow-symlinks]
 
 DESCRIPTION
@@ -64,6 +64,11 @@ OPTIONS
 	end-of-line conversion, etc). In this case, <object> has to be of
 	the form <tree-ish>:<path>, or :<path>.
 
+--path=<path>::
+	For use with --textconv or --filters, to allow specifying an object
+	name and a path separately, e.g. when it is difficult to figure out
+	the revision from which the blob came.
+
 --batch::
 --batch=<format>::
 	Print object information and contents for each object provided
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 0b74afa..b648056 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,6 +20,8 @@ struct batch_options {
 	const char *format;
 };
 
+static const char *force_path;
+
 static int filter_object(const char *path, unsigned mode,
 			 const unsigned char *sha1,
 			 char **buf, unsigned long *size)
@@ -58,6 +60,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	struct object_info oi = {NULL};
 	struct strbuf sb = STRBUF_INIT;
 	unsigned flags = LOOKUP_REPLACE_OBJECT;
+	const char *path = force_path;
 
 	if (unknown_type)
 		flags |= LOOKUP_UNKNOWN_OBJECT;
@@ -65,6 +68,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
 		die("Not a valid object name %s", obj_name);
 
+	if (!path)
+		path = obj_context.path;
+	if (obj_context.mode == S_IFINVALID)
+		obj_context.mode = 0100644;
+
 	buf = NULL;
 	switch (opt) {
 	case 't':
@@ -89,21 +97,22 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 		return !has_sha1_file(sha1);
 
 	case 'w':
-		if (!obj_context.path[0])
+		if (!path[0])
 			die("git cat-file --filters %s: <object> must be "
 			    "<sha1:path>", obj_name);
 
-		if (filter_object(obj_context.path, obj_context.mode,
+		if (filter_object(path, obj_context.mode,
 				  sha1, &buf, &size))
 			return -1;
 		break;
 
 	case 'c':
-		if (!obj_context.path[0])
+		if (!path[0])
 			die("git cat-file --textconv %s: <object> must be <sha1:path>",
 			    obj_name);
 
-		if (textconv_object(obj_context.path, obj_context.mode, sha1, 1, &buf, &size))
+		if (textconv_object(path, obj_context.mode,
+				    sha1, 1, &buf, &size))
 			break;
 
 	case 'p':
@@ -477,7 +486,7 @@ static int batch_objects(struct batch_options *opt)
 }
 
 static const char * const cat_file_usage[] = {
-	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) <object>"),
+	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) [--path=<path>] <object>"),
 	N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
 	NULL
 };
@@ -525,6 +534,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 			    N_("for blob objects, run textconv on object's content"), 'c'),
 		OPT_CMDMODE(0, "filters", &opt,
 			    N_("for blob objects, run filters on object's content"), 'w'),
+		OPT_STRING(0, "path", &force_path, N_("blob"),
+			   N_("use a specific path for --textconv/--filters")),
 		OPT_BOOL(0, "allow-unknown-type", &unknown_type,
 			  N_("allow -s and -t to work with broken/corrupt objects")),
 		OPT_BOOL(0, "buffer", &batch.buffer_output, N_("buffer --batch output")),
@@ -567,6 +578,11 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		usage_with_options(cat_file_usage, options);
 	}
 
+	if (force_path && opt != 'c' && opt != 'w') {
+		error("--path=<path> needs --textconv or --filters");
+		usage_with_options(cat_file_usage, options);
+	}
+
 	if (batch.buffer_output < 0)
 		batch.buffer_output = batch.all_objects;
 
diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
index e466634..0d5c33e 100755
--- a/t/t8010-cat-file-filters.sh
+++ b/t/t8010-cat-file-filters.sh
@@ -31,4 +31,24 @@ test_expect_success 'cat-file --filters converts to worktree version' '
 	has_cr actual
 '
 
+test_expect_success 'cat-file --filters --path=<path> works' '
+	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
+	git cat-file --filters --path=world.txt $sha1 >actual &&
+	has_cr actual
+'
+
+test_expect_success 'cat-file --textconv --path=<path> works' '
+	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
+	test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
+	git cat-file --textconv --path=hello.txt $sha1 >rot13 &&
+	test uryyb = "$(cat rot13 | remove_cr)"
+'
+
+test_expect_success '----path=<path> complains without --textconv/--filters' '
+	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
+	test_must_fail git cat-file --path=hello.txt blob $sha1 >actual 2>err &&
+	test ! -s actual &&
+	grep "path.*needs.*filters" err
+'
+
 test_done
-- 
2.10.0.windows.1.10.g803177d

[PATCH v3 4/4] cat-file: support --textconv/--filters in batch mode

[Johannes Schindelin]

With this patch, --batch can be combined with --textconv or --filters.
For this to work, the input needs to have the form

	<object name><single white space><path>

so that the filters can be chosen appropriately.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-cat-file.txt | 18 +++++++++++-----
 builtin/cat-file.c             | 49 +++++++++++++++++++++++++++++++++++++-----
 t/t8010-cat-file-filters.sh    | 10 +++++++++
 3 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 4fa9041..204541c 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) [--path=<path>] <object>
-'git cat-file' (--batch | --batch-check) [--follow-symlinks]
+'git cat-file' (--batch | --batch-check) [ --textconv | --filters ] [--follow-symlinks]
 
 DESCRIPTION
 -----------
@@ -20,7 +20,11 @@ object type, or `-s` is used to find the object size, or `--textconv` or
 `--filters` is used (which imply type "blob").
 
 In the second form, a list of objects (separated by linefeeds) is provided on
-stdin, and the SHA-1, type, and size of each object is printed on stdout.
+stdin, and the SHA-1, type, and size of each object is printed on stdout. The
+output format can be overridden using the optional `<format>` argument. If
+either `--textconv` or `--filters` was specified, the input is expected to
+list the object names followed by the path name, separated by a single white
+space, so that the appropriate drivers can be determined.
 
 OPTIONS
 -------
@@ -72,13 +76,17 @@ OPTIONS
 --batch::
 --batch=<format>::
 	Print object information and contents for each object provided
-	on stdin.  May not be combined with any other options or arguments.
-	See the section `BATCH OUTPUT` below for details.
+	on stdin.  May not be combined with any other options or arguments
+	except `--textconv` or `--filters`, in which case the input lines
+	also need to specify the path, separated by white space.  See the
+	section `BATCH OUTPUT` below for details.
 
 --batch-check::
 --batch-check=<format>::
 	Print object information for each object provided on stdin.  May
-	not be combined with any other options or arguments.  See the
+	not be combined with any other options or arguments except
+	`--textconv` or `--filters`, in which case the input lines also
+	need to specify the path, separated by white space.  See the
 	section `BATCH OUTPUT` below for details.
 
 --batch-all-objects::
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b648056..4461153 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -17,6 +17,7 @@ struct batch_options {
 	int print_contents;
 	int buffer_output;
 	int all_objects;
+	int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
 	const char *format;
 };
 
@@ -285,7 +286,32 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
 	if (data->type == OBJ_BLOB) {
 		if (opt->buffer_output)
 			fflush(stdout);
-		if (stream_blob_to_fd(1, sha1, NULL, 0) < 0)
+		if (opt->cmdmode) {
+			char *contents;
+			unsigned long size;
+
+			if (!data->rest)
+				die("missing path for '%s'", sha1_to_hex(sha1));
+
+			if (opt->cmdmode == 'w') {
+				if (filter_object(data->rest, 0100644, sha1,
+						  &contents, &size))
+					die("could not convert '%s' %s",
+					    sha1_to_hex(sha1), data->rest);
+			} else if (opt->cmdmode == 'c') {
+				enum object_type type;
+				if (!textconv_object(data->rest, 0100644, sha1,
+						     1, &contents, &size))
+					contents = read_sha1_file(sha1, &type,
+								  &size);
+				if (!contents)
+					die("could not convert '%s' %s",
+					    sha1_to_hex(sha1), data->rest);
+			} else
+				die("BUG: invalid cmdmode: %c", opt->cmdmode);
+			batch_write(opt, contents, size);
+			free(contents);
+		} else if (stream_blob_to_fd(1, sha1, NULL, 0) < 0)
 			die("unable to stream %s to stdout", sha1_to_hex(sha1));
 	}
 	else {
@@ -422,6 +448,8 @@ static int batch_objects(struct batch_options *opt)
 	data.mark_query = 1;
 	strbuf_expand(&buf, opt->format, expand_format, &data);
 	data.mark_query = 0;
+	if (opt->cmdmode)
+		data.split_on_whitespace = 1;
 
 	if (opt->all_objects) {
 		struct object_info empty;
@@ -487,7 +515,7 @@ static int batch_objects(struct batch_options *opt)
 
 static const char * const cat_file_usage[] = {
 	N_("git cat-file (-t [--allow-unknown-type]|-s [--allow-unknown-type]|-e|-p|<type>|--textconv|--filters) [--path=<path>] <object>"),
-	N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
+	N_("git cat-file (--batch | --batch-check) [--follow-symlinks] [--textconv|--filters]"),
 	NULL
 };
 
@@ -558,7 +586,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, cat_file_usage, 0);
 
 	if (opt) {
-		if (argc == 1)
+		if (batch.enabled && (opt == 'c' || opt == 'w'))
+			batch.cmdmode = opt;
+		else if (argc == 1)
 			obj_name = argv[0];
 		else
 			usage_with_options(cat_file_usage, options);
@@ -570,8 +600,12 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		} else
 			usage_with_options(cat_file_usage, options);
 	}
-	if (batch.enabled && (opt || argc)) {
-		usage_with_options(cat_file_usage, options);
+	if (batch.enabled) {
+		if (batch.cmdmode != opt || argc)
+			usage_with_options(cat_file_usage, options);
+		if (batch.cmdmode && batch.all_objects)
+			die("--batch-all-objects cannot be combined with "
+			    "--textconv nor with --filters");
 	}
 
 	if ((batch.follow_symlinks || batch.all_objects) && !batch.enabled) {
@@ -583,6 +617,11 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		usage_with_options(cat_file_usage, options);
 	}
 
+	if (force_path && batch.enabled) {
+		error("--path=<path> incompatible with --batch");
+		usage_with_options(cat_file_usage, options);
+	}
+
 	if (batch.buffer_output < 0)
 		batch.buffer_output = batch.all_objects;
 
diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
index 0d5c33e..acdfa09 100755
--- a/t/t8010-cat-file-filters.sh
+++ b/t/t8010-cat-file-filters.sh
@@ -51,4 +51,14 @@ test_expect_success '----path=<path> complains without --textconv/--filters' '
 	grep "path.*needs.*filters" err
 '
 
+test_expect_success 'cat-file --textconv --batch works' '
+	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
+	test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
+	printf "%s hello.txt\n%s hello\n" $sha1 $sha1 |
+	git cat-file --textconv --batch >actual &&
+	printf "%s blob 6\nuryyb\r\n\n%s blob 6\nhello\n\n" \
+		$sha1 $sha1 >expect &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.10.0.windows.1.10.g803177d

Re: [PATCH v3 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> +static int filter_object(const char *path, unsigned mode,
> +			 const unsigned char *sha1,
> +			 char **buf, unsigned long *size)
> +{
> +	enum object_type type;
> +
> +	*buf = read_sha1_file(sha1, &type, size);
> +	if (!*buf)
> +		return error(_("cannot read object %s '%s'"),
> +			sha1_to_hex(sha1), path);
> +	if (type != OBJ_BLOB) {
> +		free(*buf);
> +		return error(_("blob expected for %s '%s'"),
> +			sha1_to_hex(sha1), path);
> +	}
> +	if (S_ISREG(mode)) {
> +		struct strbuf strbuf = STRBUF_INIT;
> +		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
> +			free(*buf);
> +			*size = strbuf.len;
> +			*buf = strbuf_detach(&strbuf, NULL);
> +		}
> +	}

This needs to error out when mode is not ISREG just like it errors
out when type is not BLOB.

Other than that, I think these four patches are good to go.

Thanks.

Re: [PATCH v3 2/4] cat-file: introduce the --filters option

[Johannes Schindelin]

Hi Junio,

On Fri, 9 Sep 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > +static int filter_object(const char *path, unsigned mode,
> > +			 const unsigned char *sha1,
> > +			 char **buf, unsigned long *size)
> > +{
> > +	enum object_type type;
> > +
> > +	*buf = read_sha1_file(sha1, &type, size);
> > +	if (!*buf)
> > +		return error(_("cannot read object %s '%s'"),
> > +			sha1_to_hex(sha1), path);
> > +	if (type != OBJ_BLOB) {
> > +		free(*buf);
> > +		return error(_("blob expected for %s '%s'"),
> > +			sha1_to_hex(sha1), path);
> > +	}
> > +	if (S_ISREG(mode)) {
> > +		struct strbuf strbuf = STRBUF_INIT;
> > +		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
> > +			free(*buf);
> > +			*size = strbuf.len;
> > +			*buf = strbuf_detach(&strbuf, NULL);
> > +		}
> > +	}
> 
> This needs to error out when mode is not ISREG just like it errors
> out when type is not BLOB.

Are you sure that this is desirable in batch mode?

Ciao,
Dscho

Re: [PATCH v3 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> > +	if (type != OBJ_BLOB) {
>> > +		free(*buf);
>> > +		return error(_("blob expected for %s '%s'"),
>> > +			sha1_to_hex(sha1), path);
>> > +	}
>> > +	if (S_ISREG(mode)) {
>> > +		struct strbuf strbuf = STRBUF_INIT;
>> > +		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
>> > +			free(*buf);
>> > +			*size = strbuf.len;
>> > +			*buf = strbuf_detach(&strbuf, NULL);
>> > +		}
>> > +	}
>> 
>> This needs to error out when mode is not ISREG just like it errors
>> out when type is not BLOB.
>
> Are you sure that this is desirable in batch mode?

I do not quite see a reason why we should not diagnose a bad input
that does not produce a filtered result.  In batch mode or not, it
diagnoses when the user feeds it a non-blob, and I think it should
do so for non-regular, too. Both are "you asked me to filter, but
you shouldn't have".

Re: [PATCH v3 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>>> > +	if (type != OBJ_BLOB) {
>>> > +		free(*buf);
>>> > +		return error(_("blob expected for %s '%s'"),
>>> > +			sha1_to_hex(sha1), path);
>>> > +	}
>>> > +	if (S_ISREG(mode)) {
>>> > +		struct strbuf strbuf = STRBUF_INIT;
>>> > +		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
>>> > +			free(*buf);
>>> > +			*size = strbuf.len;
>>> > +			*buf = strbuf_detach(&strbuf, NULL);
>>> > +		}
>>> > +	}
>>> 
>>> This needs to error out when mode is not ISREG just like it errors
>>> out when type is not BLOB.
>>
>> Are you sure that this is desirable in batch mode?
>
> I do not quite see a reason why we should not diagnose a bad input
> that does not produce a filtered result.  In batch mode or not, it
> diagnoses when the user feeds it a non-blob, and I think it should
> do so for non-regular, too. Both are "you asked me to filter, but
> you shouldn't have".

Stepping back a bit, I can also see a use case that would be helped
if this filter_object() function by default gives the contents of
the requested object as-is, unless the object is a regular blob with
a path for which filtering is defined.  Driving such a mechanism via
the batch interface will allow you to first ask about the top-level
tree object (given back to you as-is), and you can iterate over its
entries recursively and get the blobs to be placed in a new working
directory (i.e. "git archive" piped to "tar xf" but regular files
are passed thru convert_to_working_tree()).  In such an application,
after you learn the mode from the containing tree object and know
that RelNotes is a symbolic link blob, you still would want the
contents out of the pipe going to the same batch interface process
that is not filtered.

So I would not mind if we define the semantics of "--filters" as
such (as long as it is clearly documented, of course).  AFAICS, the
batch interface does not call filter_object() for non-blobs, and by
returning successfully without doing anything special for a symbolic
link from filter_object() automatically gives us the "by default
return as-is, but give processed output only for regular file blobs"
semantics to the batch mode.

But for a non-batch mode, it feels somewhat funny to be giving the
as-is output without saying anything to symbolic links; we can argue
that it is being consistent with what we do in the batch mode,
though.

Thanks.

Re: [PATCH v3 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> So I would not mind if we define the semantics of "--filters" as
> such (as long as it is clearly documented, of course).  AFAICS, the
> batch interface does not call filter_object() for non-blobs, and by
> returning successfully without doing anything special for a symbolic
> link from filter_object() automatically gives us the "by default
> return as-is, but give processed output only for regular file blobs"
> semantics to the batch mode.
>
> But for a non-batch mode, it feels somewhat funny to be giving the
> as-is output without saying anything to symbolic links; we can argue
> that it is being consistent with what we do in the batch mode,
> though.

In other words, instead of trying to be consistent by erroring out
in non-regular blob case, I think the attached change on top would
make more sense, by consistently passing the object contents as-is
for all "not filtered" cases, whether it is run from the batch mode
or from the command line.

 builtin/cat-file.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f8a3a08..99cb525 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -33,12 +33,7 @@ static int filter_object(const char *path, unsigned mode,
 	if (!*buf)
 		return error(_("cannot read object %s '%s'"),
 			sha1_to_hex(sha1), path);
-	if (type != OBJ_BLOB) {
-		free(*buf);
-		return error(_("blob expected for %s '%s'"),
-			sha1_to_hex(sha1), path);
-	}
-	if (S_ISREG(mode)) {
+	if ((type == OBJ_BLOB) && S_ISREG(mode)) {
 		struct strbuf strbuf = STRBUF_INIT;
 		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
 			free(*buf);

Re: [PATCH v3 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> +test_expect_success '----path=<path> complains without --textconv/--filters' '

I wonder where this "----path" came from; it wasn't in v2 I queued,
but somehow came back mysteriously.

Will locally amend.

> +	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
> +	test_must_fail git cat-file --path=hello.txt blob $sha1 >actual 2>err &&
> +	test ! -s actual &&
> +	grep "path.*needs.*filters" err
> +'
> +
>  test_done

Re: [PATCH v3 2/4] cat-file: introduce the --filters option

[Johannes Schindelin]

Hi Junio,

On Fri, 9 Sep 2016, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > So I would not mind if we define the semantics of "--filters" as
> > such (as long as it is clearly documented, of course).  AFAICS, the
> > batch interface does not call filter_object() for non-blobs, and by
> > returning successfully without doing anything special for a symbolic
> > link from filter_object() automatically gives us the "by default
> > return as-is, but give processed output only for regular file blobs"
> > semantics to the batch mode.
> >
> > But for a non-batch mode, it feels somewhat funny to be giving the
> > as-is output without saying anything to symbolic links; we can argue
> > that it is being consistent with what we do in the batch mode,
> > though.
> 
> In other words, instead of trying to be consistent by erroring out
> in non-regular blob case, I think the attached change on top would
> make more sense, by consistently passing the object contents as-is
> for all "not filtered" cases, whether it is run from the batch mode
> or from the command line.
> 
>  builtin/cat-file.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index f8a3a08..99cb525 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -33,12 +33,7 @@ static int filter_object(const char *path, unsigned mode,
>  	if (!*buf)
>  		return error(_("cannot read object %s '%s'"),
>  			sha1_to_hex(sha1), path);
> -	if (type != OBJ_BLOB) {
> -		free(*buf);
> -		return error(_("blob expected for %s '%s'"),
> -			sha1_to_hex(sha1), path);
> -	}
> -	if (S_ISREG(mode)) {
> +	if ((type == OBJ_BLOB) && S_ISREG(mode)) {
>  		struct strbuf strbuf = STRBUF_INIT;
>  		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
>  			free(*buf);

Yes, that makes most sense to me, too.

Ciao,
Dscho

Re: [PATCH v3 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Johannes Schindelin]

Hi Junio,

On Fri, 9 Sep 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > +test_expect_success '----path=<path> complains without --textconv/--filters' '
> 
> I wonder where this "----path" came from; it wasn't in v2 I queued,
> but somehow came back mysteriously.

No idea how it crept back in. Sorry.

> Will locally amend.

Thanks!
Dscho

Re: [PATCH v3 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> In other words, instead of trying to be consistent by erroring out
>> in non-regular blob case, I think the attached change on top would
>> make more sense, by consistently passing the object contents as-is
>> for all "not filtered" cases, whether it is run from the batch mode
>> or from the command line.
>>  ...
>> +	if ((type == OBJ_BLOB) && S_ISREG(mode)) {
>>  		struct strbuf strbuf = STRBUF_INIT;
>>  		if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
>>  			free(*buf);
>
> Yes, that makes most sense to me, too.

Alright; will squash it in then before merging it down to 'next'.