Re: [PATCH 3/6] cat-file: do not die on --textconv without textconv filters
Jeff King venit, vidit, dixit 20.04.2013 06:17:
On Fri, Apr 19, 2013 at 06:44:46PM +0200, Michael J Gruber wrote:
-die(git cat-file --textconv: unable to run textconv on
%s,
-obj_name);
-break;
+if (textconv_object(obj_context.path, obj_context.mode, sha1,
1, buf, size))
+break;
+
+/* otherwise expect a blob */
+exp_type = blob;
case 0:
if (type_from_string(exp_type) == OBJ_BLOB) {
I'm not sure this is right. What happens with:
git cat-file --textconv HEAD:Documentation
We have failed to textconv, but should we be expecting a blob?
Very true, thanks. I'll reorder so that the --textconv case continues
(without break) into the -p case. I think it makes sense to consider
--textconv to be at least as pretty as -p.
Michael
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] cat-file: do not die on --textconv without textconv filters
Michael J Gruber g...@drmicha.warpmail.net writes: When a command is supposed to use textconv filters (by default or with --textconv) and none are configured then the blob is output without conversion; the only exception to this rule is cat-file --textconv. I am of two minds. Because cat-file is mostly a low-level plumbing, I do not necessarily think it is a bad behaviour for it to error out when it was asked to apply textconv where there is no filter or when the filter fails to produce an output. On the other hand, it certainly makes it more convenient for callers that do not care too deeply, taking textconv as a mere hint just like Porcelains do. But assuming that this is the direction we would want to go... diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 40f87b4..dd4e063 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -146,10 +146,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name) 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)) - die(git cat-file --textconv: unable to run textconv on %s, - obj_name); - break; + if (textconv_object(obj_context.path, obj_context.mode, sha1, 1, buf, size)) + break; + + /* otherwise expect a blob */ + exp_type = blob; Please use the constant string blob_type that is available for all callers including this one. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] cat-file: do not die on --textconv without textconv filters
On Fri, Apr 19, 2013 at 06:44:46PM +0200, Michael J Gruber wrote:
- die(git cat-file --textconv: unable to run textconv on
%s,
- obj_name);
- break;
+ if (textconv_object(obj_context.path, obj_context.mode, sha1,
1, buf, size))
+ break;
+
+ /* otherwise expect a blob */
+ exp_type = blob;
case 0:
if (type_from_string(exp_type) == OBJ_BLOB) {
I'm not sure this is right. What happens with:
git cat-file --textconv HEAD:Documentation
We have failed to textconv, but should we be expecting a blob?
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
