git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Kirill Smelkov <kirr@landau.phys.spbu.ru>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Axel Bonnet <axel.bonnet@ensimag.imag.fr>,
	Cl??ment Poulain <clement.poulain@ensimag.imag.fr>,
	Diane Gasselin <diane.gasselin@ensimag.imag.fr>,
	Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Subject: Re: [PATCH v3 1/3] tests: Prepare --textconv tests for correctly-failing conversion program
Date: Tue, 28 Sep 2010 09:23:57 -0400	[thread overview]
Message-ID: <20100928132356.GA5829@sigill.intra.peff.net> (raw)
In-Reply-To: <20100928120722.GA29525@landau.phys.spbu.ru>

On Tue, Sep 28, 2010 at 04:07:22PM +0400, Kirill Smelkov wrote:

> > Granted, echo is forgiving and will concatenate the arguments it gets with
> > a space in between, but you would either want to either:
> > 
> >  (1) make it more explicit that helper gets only one argument, by saying
> >      "$1" instead of "$@", in all places in the helper script; or
> > 
> >  (2) if you are planning to make 'helper' capable of handling multiple
> >      input files, show the error message for the ones that are not binary
> >      (you would probably need a loop for that).
> > 
> > I think (1) would be sufficient in this case.
> 
> I too think (1) is right. It was just that originally there was $@
> (which I now understand was wrong). So ok to apply the following patch
> on top of this series? (I assume it's ok, and will repost the whole
> thing)

No, "helper" is supposed to be able to take multiple arguments, at least
in t4042. See the "changing textconv invalidates cache" test. The extra
argument comes from the user, not from git.

>  t/t4042-diff-textconv-caching.sh |    4 ++--

Why are we touching t4042 at all in this series? We are not actually
adding any tests to it, AFAICT.

>  t/t8006-blame-textconv.sh        |    4 ++--
>  t/t8007-cat-file-textconv.sh     |    4 ++--

These ones simply copied the helper script, and could be switched to use
$1.

-Peff

  parent reply	other threads:[~2010-09-28 13:24 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-24 18:24 [BUG, PATCH v3 0/3] Fix {blame,cat-file} --textconv for cases with symlinks Kirill Smelkov
2010-09-24 18:24 ` [PATCH v3 1/3] tests: Prepare --textconv tests for correctly-failing conversion program Kirill Smelkov
2010-09-27 18:23   ` Junio C Hamano
2010-09-28 12:07     ` Kirill Smelkov
2010-09-28 12:20       ` Kirill Smelkov
2010-09-28 12:41         ` Kirill Smelkov
2010-09-28 13:23       ` Jeff King [this message]
2010-09-28 14:35         ` Kirill Smelkov
2010-09-28 14:39           ` Jeff King
2010-09-28 15:09             ` Kirill Smelkov
2010-09-28 15:36       ` Junio C Hamano
2010-09-28 15:58         ` Jeff King
2010-09-28 16:12           ` Kirill Smelkov
2010-09-29 11:44             ` Kirill Smelkov
2010-09-24 18:24 ` [PATCH v3 2/3] blame,cat-file: Demonstrate --textconv is wrongly running converter on symlinks Kirill Smelkov
2010-09-27 18:27   ` Junio C Hamano
2010-09-28 12:07     ` Kirill Smelkov
2010-09-24 18:24 ` [PATCH v3 3/3] blame,cat-file --textconv: Don't assume mode is ``S_IFREF | 0664'' Kirill Smelkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100928132356.GA5829@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=axel.bonnet@ensimag.imag.fr \
    --cc=clement.poulain@ensimag.imag.fr \
    --cc=diane.gasselin@ensimag.imag.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kirr@landau.phys.spbu.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).