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

Subsystems: the rest

2 messages, 2 authors, 2016-09-10 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-09-09 17:26:27

Junio C Hamano [off-list ref] 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 2/4] cat-file: introduce the --filters option

From: Johannes Schindelin <hidden>
Date: 2016-09-10 07:57:55

Hi Junio,

On Fri, 9 Sep 2016, Junio C Hamano wrote:
quoted hunk
Junio C Hamano [off-list ref] writes:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help