git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Keeping <john@keeping.me.uk>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jonathan Nieder <jrnieder@gmail.com>,
	Jens Lehmann <Jens.Lehmann@web.de>,
	Heiko Voigt <hvoigt@hvoigt.net>,
	Ramkumar Ramachandra <artagnon@gmail.com>
Subject: Re: [PATCH v3 2/2] submodule: drop the top-level requirement
Date: Fri, 19 Apr 2013 08:46:32 +0100	[thread overview]
Message-ID: <20130419074632.GC2278@serenity.lan> (raw)
In-Reply-To: <7vfvyn4g46.fsf@alter.siamese.dyndns.org>

On Thu, Apr 18, 2013 at 03:40:41PM -0700, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> > +relative_path ()
> > +{
> > +	local target curdir result
> > +	target=$1
> > +	curdir=${2-$wt_prefix}
> > +	curdir=${curdir%/}
> > +	result=
> > +
> > +	while test -n "$curdir"
> > +	do
> > +		case "$target" in
> > +		"$curdir/"*)
> > +			target=${target#$curdir/}
> > +			break
> > +			;;
> > +		esac
> 
> Could $curdir have glob wildcard to throw this part of the logic
> off?  It is OK to have limitations like "you cannot have a glob
> characters in your path to submodule working tree" (at least until
> we start rewriting these in C or Perl or Python), but we need to be
> aware of them.

I think the use of "#" instead of "##" saves us here because even with a
wildcard in $curdir the case statement matches literally, so we know
that "$target" starts with "$curdir/", so "${target#$curdir/}" can't
match anything longer than the literal "$curdir" prefix.

> >  module_list()
> >  {
> > +	eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"
> 
> An efficient reuse of "--" ;-)
> 
> > +test_expect_success 'run summary from subdir' '
> > +	mkdir sub &&
> > +	(
> > +		cd sub &&
> > +		git submodule summary >../actual
> > +	) &&
> > +	cat >expected <<-EOF &&
> > +* ../sm1 0000000...$head1 (2):
> > +  > Add foo2
> > +
> > +EOF
> 
> It somewhat looks strange to start with "<<-EOF" and then not to
> indent the body nor EOF.

Yes, but I copied this from the preceding test.  I'd rather leave this
as it is for consistency and let later style fixes change all of the
tests in this file.

  reply	other threads:[~2013-04-19  7:46 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-07 19:55 [RFC/PATCH 0/2] submodule: drop the top-level requirement John Keeping
2013-04-07 19:55 ` [PATCH 1/2] rev-parse: add --filename-prefix option John Keeping
2013-04-07 22:14   ` Jonathan Nieder
2013-04-08  8:31     ` John Keeping
2013-04-08 15:07       ` Junio C Hamano
2013-04-08 17:36         ` John Keeping
2013-04-08 18:11           ` Junio C Hamano
2013-04-07 19:55 ` [PATCH 2/2] submodule: drop the top-level requirement John Keeping
2013-04-07 20:15 ` [RFC/PATCH 0/2] " Jens Lehmann
2013-04-09 20:29 ` [PATCH v2 " John Keeping
2013-04-09 20:29   ` [PATCH v2 1/2] rev-parse: add --filename-prefix option John Keeping
2013-04-09 20:57     ` Junio C Hamano
2013-04-09 21:28       ` John Keeping
2013-04-09 21:33         ` Junio C Hamano
2013-04-18 14:28     ` Ramkumar Ramachandra
2013-04-18 14:42       ` John Keeping
2013-04-09 20:29   ` [PATCH v2 2/2] submodule: drop the top-level requirement John Keeping
2013-04-09 21:00     ` Junio C Hamano
2013-04-09 21:29       ` John Keeping
2013-04-18 14:46     ` Ramkumar Ramachandra
2013-04-18 14:56       ` John Keeping
2013-04-18 19:50   ` [PATCH v3 0/2] " John Keeping
2013-04-18 19:50     ` [PATCH v3 1/2] rev-parse: add --prefix option John Keeping
2013-04-19  9:53       ` Ramkumar Ramachandra
2013-04-19 10:22         ` John Keeping
2013-04-19 11:15           ` Ramkumar Ramachandra
2013-04-19 11:25             ` John Keeping
2013-04-19 11:29               ` Ramkumar Ramachandra
2013-04-18 19:50     ` [PATCH v3 2/2] submodule: drop the top-level requirement John Keeping
2013-04-18 22:40       ` Junio C Hamano
2013-04-19  7:46         ` John Keeping [this message]
2013-04-19 16:45           ` Junio C Hamano
2013-04-19 19:23             ` Johannes Sixt
2013-04-19 21:03               ` Junio C Hamano
2013-04-24  8:15                 ` [PATCH] submodule: fix quoting in relative_path() John Keeping
2013-04-24 16:21                   ` Junio C Hamano
2013-04-24 16:28                     ` John Keeping
2013-04-24 19:12                       ` Johannes Sixt
2013-04-18 23:54       ` [PATCH v3 2/2] submodule: drop the top-level requirement Eric Sunshine

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=20130419074632.GC2278@serenity.lan \
    --to=john@keeping.me.uk \
    --cc=Jens.Lehmann@web.de \
    --cc=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hvoigt@hvoigt.net \
    --cc=jrnieder@gmail.com \
    /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).