Re: [PATCH 14/16] pathspec: create strip submodule slash helpers

[Brandon Williams]

On 12/09, Stefan Beller wrote:
> On Fri, Dec 9, 2016 at 11:18 AM, Brandon Williams  wrote:
> > Factor out the logic responsible for stripping the trailing slash on
> > pathspecs referencing submodules into its own function.
> >
> > Change-Id: Icad62647c04b4195309def0e3db416203d14f9e4
> 
> I think we should come up with a solution to wipe out change ids
> before sending emails. ;)

Darn! Yeah maybe a hook or something :D

-- 
Brandon Williams

Re: [PATCH 14/16] pathspec: create strip submodule slash helpers

[Stefan Beller]

On Fri, Dec 9, 2016 at 11:18 AM, Brandon Williams  wrote:
> Factor out the logic responsible for stripping the trailing slash on
> pathspecs referencing submodules into its own function.
>
> Change-Id: Icad62647c04b4195309def0e3db416203d14f9e4

I think we should come up with a solution to wipe out change ids
before sending emails. ;)

> Signed-off-by: Brandon Williams 
> ---
>  pathspec.c | 68 
> ++
>  1 file changed, 42 insertions(+), 26 deletions(-)
>
> diff --git a/pathspec.c b/pathspec.c
> index 84a57cf..4d9a6a0 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -258,6 +258,44 @@ static const char *parse_element_magic(unsigned *magic, 
> int *prefix_len,
> return parse_short_magic(magic, elem);
>  }
>
> +static void strip_submodule_slash_cheap(struct pathspec_item *item)
> +{
> +   if (item->len >= 1 && item->match[item->len - 1] == '/') {
> +   int i = cache_name_pos(item->match, item->len - 1);
> +
> +   if (i >= 0 && S_ISGITLINK(active_cache[i]->ce_mode)) {
> +   item->len--;
> +   item->match[item->len] = '\0';
> +   }
> +   }
> +}
> +
> +static void strip_submodule_slash_expensive(struct pathspec_item *item)
> +{
> +   int i;
> +
> +   for (i = 0; i < active_nr; i++) {
> +   struct cache_entry *ce = active_cache[i];
> +   int ce_len = ce_namelen(ce);
> +
> +   if (!S_ISGITLINK(ce->ce_mode))
> +   continue;
> +
> +   if (item->len <= ce_len || item->match[ce_len] != '/' ||
> +   memcmp(ce->name, item->match, ce_len))
> +   continue;
> +
> +   if (item->len == ce_len + 1) {
> +   /* strip trailing slash */
> +   item->len--;
> +   item->match[item->len] = '\0';
> +   } else {
> +   die(_("Pathspec '%s' is in submodule '%.*s'"),
> +   item->original, ce_len, ce->name);
> +   }
> +   }
> +}
> +
>  /*
>   * Take an element of a pathspec and check for magic signatures.
>   * Append the result to the prefix. Return the magic bitmap.
> @@ -278,7 +316,7 @@ static unsigned prefix_pathspec(struct pathspec_item 
> *item, unsigned flags,
> unsigned magic = 0, element_magic = 0;
> const char *copyfrom = elt;
> char *match;
> -   int i, pathspec_prefix = -1;
> +   int pathspec_prefix = -1;
>
> /* PATHSPEC_LITERAL_PATH ignores magic */
> if (flags & PATHSPEC_LITERAL_PATH) {
> @@ -329,33 +367,11 @@ static unsigned prefix_pathspec(struct pathspec_item 
> *item, unsigned flags,
> item->len = strlen(item->match);
> item->prefix = prefixlen;
>
> -   if ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) &&
> -   (item->len >= 1 && item->match[item->len - 1] == '/') &&
> -   (i = cache_name_pos(item->match, item->len - 1)) >= 0 &&
> -   S_ISGITLINK(active_cache[i]->ce_mode)) {
> -   item->len--;
> -   match[item->len] = '\0';
> -   }
> +   if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)
> +   strip_submodule_slash_cheap(item);
>
> if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)
> -   for (i = 0; i < active_nr; i++) {
> -   struct cache_entry *ce = active_cache[i];
> -   int ce_len = ce_namelen(ce);
> -
> -   if (!S_ISGITLINK(ce->ce_mode))
> -   continue;
> -
> -   if (item->len <= ce_len || match[ce_len] != '/' ||
> -   memcmp(ce->name, match, ce_len))
> -   continue;
> -   if (item->len == ce_len + 1) {
> -   /* strip trailing slash */
> -   item->len--;
> -   match[item->len] = '\0';
> -   } else
> -   die (_("Pathspec '%s' is in submodule 
> '%.*s'"),
> -elt, ce_len, ce->name);
> -   }
> +   strip_submodule_slash_expensive(item);
>
> if (magic & PATHSPEC_LITERAL)
> item->nowildcard_len = item->len;
> --
> 2.8.0.rc3.226.g39d4020
>

[PATCH 14/16] pathspec: create strip submodule slash helpers

[Brandon Williams]

Factor out the logic responsible for stripping the trailing slash on
pathspecs referencing submodules into its own function.

Change-Id: Icad62647c04b4195309def0e3db416203d14f9e4
Signed-off-by: Brandon Williams 
---
 pathspec.c | 68 ++
 1 file changed, 42 insertions(+), 26 deletions(-)

diff --git a/pathspec.c b/pathspec.c
index 84a57cf..4d9a6a0 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -258,6 +258,44 @@ static const char *parse_element_magic(unsigned *magic, 
int *prefix_len,
return parse_short_magic(magic, elem);
 }
 
+static void strip_submodule_slash_cheap(struct pathspec_item *item)
+{
+   if (item->len >= 1 && item->match[item->len - 1] == '/') {
+   int i = cache_name_pos(item->match, item->len - 1);
+
+   if (i >= 0 && S_ISGITLINK(active_cache[i]->ce_mode)) {
+   item->len--;
+   item->match[item->len] = '\0';
+   }
+   }
+}
+
+static void strip_submodule_slash_expensive(struct pathspec_item *item)
+{
+   int i;
+
+   for (i = 0; i < active_nr; i++) {
+   struct cache_entry *ce = active_cache[i];
+   int ce_len = ce_namelen(ce);
+
+   if (!S_ISGITLINK(ce->ce_mode))
+   continue;
+
+   if (item->len <= ce_len || item->match[ce_len] != '/' ||
+   memcmp(ce->name, item->match, ce_len))
+   continue;
+
+   if (item->len == ce_len + 1) {
+   /* strip trailing slash */
+   item->len--;
+   item->match[item->len] = '\0';
+   } else {
+   die(_("Pathspec '%s' is in submodule '%.*s'"),
+   item->original, ce_len, ce->name);
+   }
+   }
+}
+
 /*
  * Take an element of a pathspec and check for magic signatures.
  * Append the result to the prefix. Return the magic bitmap.
@@ -278,7 +316,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item, 
unsigned flags,
unsigned magic = 0, element_magic = 0;
const char *copyfrom = elt;
char *match;
-   int i, pathspec_prefix = -1;
+   int pathspec_prefix = -1;
 
/* PATHSPEC_LITERAL_PATH ignores magic */
if (flags & PATHSPEC_LITERAL_PATH) {
@@ -329,33 +367,11 @@ static unsigned prefix_pathspec(struct pathspec_item 
*item, unsigned flags,
item->len = strlen(item->match);
item->prefix = prefixlen;
 
-   if ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) &&
-   (item->len >= 1 && item->match[item->len - 1] == '/') &&
-   (i = cache_name_pos(item->match, item->len - 1)) >= 0 &&
-   S_ISGITLINK(active_cache[i]->ce_mode)) {
-   item->len--;
-   match[item->len] = '\0';
-   }
+   if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)
+   strip_submodule_slash_cheap(item);
 
if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)
-   for (i = 0; i < active_nr; i++) {
-   struct cache_entry *ce = active_cache[i];
-   int ce_len = ce_namelen(ce);
-
-   if (!S_ISGITLINK(ce->ce_mode))
-   continue;
-
-   if (item->len <= ce_len || match[ce_len] != '/' ||
-   memcmp(ce->name, match, ce_len))
-   continue;
-   if (item->len == ce_len + 1) {
-   /* strip trailing slash */
-   item->len--;
-   match[item->len] = '\0';
-   } else
-   die (_("Pathspec '%s' is in submodule '%.*s'"),
-elt, ce_len, ce->name);
-   }
+   strip_submodule_slash_expensive(item);
 
if (magic & PATHSPEC_LITERAL)
item->nowildcard_len = item->len;
-- 
2.8.0.rc3.226.g39d4020