Re: [PATCH v3 2/4] cat-file: introduce the --filters option
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Sep 10, 2016, 07:57 UTC
- Message-ID
- <alpine.DEB.2.20.1609100957040.129229@virtualbox>
- In-Reply-To
- <xmqqr38tc7d1.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 9 Sep 2016, Junio C Hamano wrote:
Show 42 quoted lines
> 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