All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Nguyen Thai Ngoc Duy <pclouds@gmail.com>,
	Michael J Gruber <git@drmicha.warpmail.net>
Subject: Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration
Date: Fri, 08 Apr 2011 16:18:46 -0700	[thread overview]
Message-ID: <7vaag01gdl.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <7vei5c1iat.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Fri, 08 Apr 2011 15:37:14 -0700")

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> ... I thought there was interest in full-tree grep
>> (OK, _I_ had some interst in it).  But the same transition pain
>> arguments apply there, and we should be able to do "git grep pattern :/"
>> soon, right?
>
> I never tested it myself, but the earlier "support :/ at a wrong level
> get_pathspec()" patch should take care of "git grep" as well.  It is
> equivalent to the "alternative approach" Michael posted as RFC as a
> follow-up to his earlier "grep --full-tree" thread.

It appears that we might want to further tweak the code that tries to
disambiguate between revs and paths (we error out when argv[i] does not
name a rev and lstat(argv[i]) fails), but other than that, this command
sequence

    $ cd Documentation
    $ git grep -e purple -- :

seems to hit ../contrib/emacs/git.el and ../gitk-git/gitk correctly.

Of course, from the same directory:

    $ git grep -e purple -- :/*.el

hits ../contrib/emacs/git.el as expected.

The following patch will apply on top of 8a42c98 (magic pathspec: add
tentative ":/path/from/top/level" pathspec support, 2011-04-06).

Per our discussion, I think 'add -u' migration topics should be scrapped
for now, and rethought after giving time for people to get familiar with
the new :/ facility.

Thanks.

-- >8 --
Subject: [PATCH] magic pathspec: futureproof shorthand form

The earlier design was to take whatever non-alnum that the short format
parser happens to support, leaving the rest as part of the pattern, so a
version of git that knows '*' magic and a version that does not would have
behaved differently when given ":*Makefile".  The former would have
applied the '*' magic to the pattern "Makefile", while the latter would
used no magic to the pattern "*Makefile".

Instead, just reserve all non-alnum ASCII letters that are neither glob
nor regexp special as potential magic signature, and when we see a magic
that is not supported, die with an error message, just like the longhand
codepath does.

With this, ":%#!*Makefile" will always mean "%#!" magic applied to the
pattern "*Makefile", no matter what version of git is used (it is a
different matter if the version of git supports all of these three magic
matching rules).

Also make ':' without anything else to mean "there is no pathspec".  This
would allow differences between "git log" and "git log ." run from the top
level of the working tree (the latter simplifies no-op commits away from
the history) to be expressed from a subdirectory by saying "git log :".

Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 ctype.c           |   15 ++++++++-------
 git-compat-util.h |    2 ++
 setup.c           |    9 ++++++++-
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/ctype.c b/ctype.c
index de60027..b5d856f 100644
--- a/ctype.c
+++ b/ctype.c
@@ -10,17 +10,18 @@ enum {
 	A = GIT_ALPHA,
 	D = GIT_DIGIT,
 	G = GIT_GLOB_SPECIAL,	/* *, ?, [, \\ */
-	R = GIT_REGEX_SPECIAL	/* $, (, ), +, ., ^, {, | */
+	R = GIT_REGEX_SPECIAL,	/* $, (, ), +, ., ^, {, | */
+	P = GIT_PATHSPEC_MAGIC  /* other non-alnum, except for ] and } */
 };
 
 unsigned char sane_ctype[256] = {
 	0, 0, 0, 0, 0, 0, 0, 0, 0, S, S, 0, 0, S, 0, 0,		/*   0.. 15 */
 	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,		/*  16.. 31 */
-	S, 0, 0, 0, R, 0, 0, 0, R, R, G, R, 0, 0, R, 0,		/*  32.. 47 */
-	D, D, D, D, D, D, D, D, D, D, 0, 0, 0, 0, 0, G,		/*  48.. 63 */
-	0, A, A, A, A, A, A, A, A, A, A, A, A, A, A, A,		/*  64.. 79 */
-	A, A, A, A, A, A, A, A, A, A, A, G, G, 0, R, 0,		/*  80.. 95 */
-	0, A, A, A, A, A, A, A, A, A, A, A, A, A, A, A,		/*  96..111 */
-	A, A, A, A, A, A, A, A, A, A, A, R, R, 0, 0, 0,		/* 112..127 */
+	S, P, P, P, R, P, P, P, R, R, G, R, P, P, R, P,		/*  32.. 47 */
+	D, D, D, D, D, D, D, D, D, D, P, P, P, P, P, G,		/*  48.. 63 */
+	P, A, A, A, A, A, A, A, A, A, A, A, A, A, A, A,		/*  64.. 79 */
+	A, A, A, A, A, A, A, A, A, A, A, G, G, 0, R, P,		/*  80.. 95 */
+	P, A, A, A, A, A, A, A, A, A, A, A, A, A, A, A,		/*  96..111 */
+	A, A, A, A, A, A, A, A, A, A, A, R, R, 0, P, 0,		/* 112..127 */
 	/* Nothing in the 128.. range */
 };
diff --git a/git-compat-util.h b/git-compat-util.h
index 49b50ee..d88cf8a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -462,6 +462,7 @@ extern unsigned char sane_ctype[256];
 #define GIT_ALPHA 0x04
 #define GIT_GLOB_SPECIAL 0x08
 #define GIT_REGEX_SPECIAL 0x10
+#define GIT_PATHSPEC_MAGIC 0x20
 #define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0)
 #define isascii(x) (((x) & ~0x7f) == 0)
 #define isspace(x) sane_istest(x,GIT_SPACE)
@@ -472,6 +473,7 @@ extern unsigned char sane_ctype[256];
 #define is_regex_special(x) sane_istest(x,GIT_GLOB_SPECIAL | GIT_REGEX_SPECIAL)
 #define tolower(x) sane_case((unsigned char)(x), 0x20)
 #define toupper(x) sane_case((unsigned char)(x), 0)
+#define is_pathspec_magic(x) sane_istest(x,GIT_PATHSPEC_MAGIC)
 
 static inline int sane_case(int x, int high)
 {
diff --git a/setup.c b/setup.c
index 820ed05..5048252 100644
--- a/setup.c
+++ b/setup.c
@@ -197,19 +197,26 @@ const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt)
 		}
 		if (*copyfrom == ')')
 			copyfrom++;
+	} else if (!elt[1]) {
+		/* Just ':' -- no element! */
+		return NULL;
 	} else {
 		/* shorthand */
 		for (copyfrom = elt + 1;
 		     *copyfrom && *copyfrom != ':';
 		     copyfrom++) {
 			char ch = *copyfrom;
+
+			if (!is_pathspec_magic(ch))
+				break;
 			for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++)
 				if (pathspec_magic[i].mnemonic == ch) {
 					magic |= pathspec_magic[i].bit;
 					break;
 				}
 			if (ARRAY_SIZE(pathspec_magic) <= i)
-				break;
+				die("Unimplemented pathspec magic '%c' in '%s'",
+				    ch, elt);
 		}
 		if (*copyfrom == ':')
 			copyfrom++;
-- 
1.7.5.rc1

  reply	other threads:[~2011-04-08 23:19 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-07  1:16 [PATCH 0/4] Redoing the "add -u" migration plan Junio C Hamano
2011-04-07  1:16 ` [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support Junio C Hamano
2011-04-07  1:40   ` Junio C Hamano
2011-04-07 13:09     ` Nguyen Thai Ngoc Duy
2011-04-07 18:28       ` Junio C Hamano
2011-04-08 11:39         ` Nguyen Thai Ngoc Duy
2011-04-07 13:23   ` Nguyen Thai Ngoc Duy
2011-04-07 16:18     ` Junio C Hamano
2011-04-08 12:00       ` Nguyen Thai Ngoc Duy
2011-04-08 15:05         ` Junio C Hamano
2011-04-08 15:39           ` Nguyen Thai Ngoc Duy
2011-04-08 16:37           ` Junio C Hamano
2011-04-08 17:02             ` Nguyen Thai Ngoc Duy
2011-04-07  1:16 ` [PATCH 2/4] add -u: get rid of "treewideupdate" configuration Junio C Hamano
2011-04-08 17:54   ` Jeff King
2011-04-08 19:27     ` Junio C Hamano
2011-04-08 20:24       ` Jeff King
2011-04-08 22:22         ` Junio C Hamano
2011-04-08 22:32           ` Jeff King
2011-04-08 22:37             ` Junio C Hamano
2011-04-08 23:18               ` Junio C Hamano [this message]
2011-04-09  4:38                 ` Nguyen Thai Ngoc Duy
2011-04-09  4:56                   ` Junio C Hamano
2011-04-09  5:05                     ` Nguyen Thai Ngoc Duy
2011-04-09 21:34                       ` Junio C Hamano
2011-04-09  4:58                 ` Nguyen Thai Ngoc Duy
2011-04-09  5:20                   ` Junio C Hamano
2011-04-09 10:15                     ` Nguyen Thai Ngoc Duy
2011-04-09 11:24                   ` Nguyen Thai Ngoc Duy
2011-04-09 21:38                     ` Junio C Hamano
2011-05-03  7:52                 ` Nguyen Thai Ngoc Duy
2011-05-03 15:01                   ` Junio C Hamano
2011-05-03 16:17                     ` Nguyen Thai Ngoc Duy
2011-04-07  1:16 ` [PATCH 3/4] add: make "add -u/-A" update full tree without pathspec (step 2) Junio C Hamano
2011-04-07  1:16 ` [PATCH 4/4] add: make "add -u/-A" update full tree without pathspec (step 3) Junio C Hamano

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=7vaag01gdl.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.