Re: [PATCH 1/6] test-lib: introduce test_commit_bulk
- From
Jeff King <peff@peff.net>
- Date
- Jun 29, 2019, 00:26 UTC
- Message-ID
- <20190629002617.GA3094@sigill.intra.peff.net>
- In-Reply-To
- <CAPig+cQnvotvJakCp1ic_WP3AMojNmfZw-PoinR=VOODmtNBtw@mail.gmail.com>
On Fri, Jun 28, 2019 at 07:46:02PM -0400, Eric Sunshine wrote:
Show 20 quoted lines
> On Fri, Jun 28, 2019 at 7:04 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > On Fri, Jun 28, 2019 at 05:32:35PM -0400, Eric Sunshine wrote:
> > > On Fri, Jun 28, 2019 at 5:39 AM Jeff King <peff@peff.net> wrote:
> > > > + in_dir=${indir:+-C "$indir"}
> > >
> > > Doesn't this suffer the problem in which some older/broken
> > > shells[1][2][3][4] incorrectly [...]
> >
> > I think we don't need any of those "${indir:+-C "$indir"}" parameter
> > expansions and could simply use 'git -C "$indir" cmd...' everywhere.
> > $indir is set to empty right at the start of the function, and 'git -C
> > "" ...' works and doesn't change the working directory.
>
> I recall the discussion around the meaning of `-C ""` when that
> command line option was introduced. The conclusion was that the
> zero-length argument should mean "this directory" since that's how `cd
> ""` behaves. However, I don't think that behavior ever got documented,
> and it's not necessarily obvious. An alternative would be to default
> 'indir' to ".", which should give the same result and be easily
> understood.Yeah, I had considered that, too. I had mostly just copied the solution from test_commit, thinking nobody would nitpick it. ;) But if everybody likes ".", I think that is a bit more readable.
-Peff