git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Torsten Bögershausen" <tboegi@web.de>
To: Jeff King <peff@peff.net>, Michael J Gruber <git@drmicha.warpmail.net>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCHv2] add: ignore only ignored files
Date: Sat, 22 Nov 2014 15:59:12 +0100	[thread overview]
Message-ID: <5470A4C0.3070501@web.de> (raw)
In-Reply-To: <20141121180105.GB26650@peff.net>

>> +test_expect_success 'error out when attempting to add ignored ones but add others' '
>> +	touch a.if &&
>> +	test_must_fail git add a.?? &&
>> +	! (git ls-files | grep "\\.ig") &&
>> +	(git ls-files | grep a.if)
>> +'
> 
> I am somewhat allergic to pipes in our test suite, because they can mask
> errors (especially with a negated grep, because we do not know if they
> correctly produced any output at all). But I guess this is matching the
> surrounding code, and it is quite unlikely for `ls-files` to fail in any
> meaningful way here. So I think it's fine.
> 
> -Peff

2 small comments:
Why the escaped "\\.ig" and the unescaped "a.if"  ?

The other question, this is a more general one, strikes me every time I see
! grep

Should we avoid it by writing "test_must_fail" instead of "!" ?
(The current code base has a mixture of both)

The following came into my mind when working on another grepy thing,
and it may be unnecessary clumsy:

test_expect_success 'error out when attempting to add ignored ones but add others' '
	touch a.if &&
	test_must_fail git add a.?? &&
	git ls-files >files.txt &&
	test_must_fail grep a.ig files.txt >/dev/null &&
	grep a.if files.txt >/dev/null &&
	rm files.txt
'

(It feels as if there should be a "grepnot" ;-)


The 3rd comment:
Thanks for taking this up!

  reply	other threads:[~2014-11-22 14:59 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-19 14:52 [RFD/PATCH] add: ignore only ignored files Michael J Gruber
2014-11-19 18:51 ` Junio C Hamano
2014-11-19 19:15 ` Jeff King
2014-11-19 21:43   ` Junio C Hamano
2014-11-20  9:42     ` Michael J Gruber
2014-11-20 15:56       ` Jeff King
2014-11-20 17:23         ` Junio C Hamano
2014-11-20 18:20           ` Jeff King
2014-11-21 15:39             ` Michael J Gruber
2014-11-21 16:08               ` [PATCHv2] " Michael J Gruber
2014-11-21 18:01                 ` Jeff King
2014-11-22 14:59                   ` Torsten Bögershausen [this message]
2014-11-22 19:19                     ` Jeff King
2014-11-22 21:20                       ` Torsten Bögershausen
2014-11-23 19:50                         ` Jeff King
2014-11-23 18:10                       ` Junio C Hamano
2014-11-23 19:46                         ` Jeff King
2014-11-24 17:41                           ` Junio C Hamano
2014-11-24 20:22                             ` Torsten Bögershausen
2014-11-25  3:57                             ` Jeff King
2014-11-24 10:29                     ` Michael J Gruber
2014-11-24 10:23                   ` Michael J Gruber

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=5470A4C0.3070501@web.de \
    --to=tboegi@web.de \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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).