Re: [PATCH 1/3] fetch-pack: move code to report unmatched refs to a function

[Matt McCutchen]

On Wed, 2017-02-22 at 09:11 -0800, Junio C Hamano wrote:
> Matt McCutchen  writes:
> 
> > We're preparing to reuse this code in transport.c for "git fetch".
> > 
> > While I'm here, internationalize the existing error message.
> > ---
> 
> Sounds good.  Please just say it is OK for me to forge your sign-off
> ;-)

Oops.  Given the other issue below, I'll just regenerate the patch
series.

> > diff --git a/fetch-pack.h b/fetch-pack.h
> > index c912e3d..fd4d80e 100644
> > --- a/fetch-pack.h
> > +++ b/fetch-pack.h
> > @@ -45,4 +45,13 @@ struct ref *fetch_pack(struct fetch_pack_args
> > *args,
> >        struct sha1_array *shallow,
> >        char **pack_lockfile);
> >  
> > +/*
> > + * Print an appropriate error message for each sought ref that
> > wasn't
> > + * matched.  Return 0 if all sought refs were matched, otherwise
> > 1.
> > + *
> > + * The type of "sought" should be "const struct ref *const *" but
> > for
> > + * http://stackoverflow.com/questions/5055655/double-pointer-const
> > -correctness-warnings-in-c .
> > + */
> 
> This is an unfinished sentence, but I wonder if we even need to have
> it here?  I'd be surprised if this function was unique in the
> codebase that takes an array pointer whose type is looser than
> necessary because of well-known language rules.

You're probably right.  I'm in the habit of documenting things that
were unknown to me, but I'll take your word for what's well-known to
the average git developer.  I'll remove the remark.

Matt

Re: [PATCH 1/3] fetch-pack: move code to report unmatched refs to a function

[Junio C Hamano]

Matt McCutchen  writes:

> We're preparing to reuse this code in transport.c for "git fetch".
>
> While I'm here, internationalize the existing error message.
> ---

Sounds good.  Please just say it is OK for me to forge your sign-off ;-)

> diff --git a/fetch-pack.h b/fetch-pack.h
> index c912e3d..fd4d80e 100644
> --- a/fetch-pack.h
> +++ b/fetch-pack.h
> @@ -45,4 +45,13 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
>  struct sha1_array *shallow,
>  char **pack_lockfile);
>  
> +/*
> + * Print an appropriate error message for each sought ref that wasn't
> + * matched.  Return 0 if all sought refs were matched, otherwise 1.
> + *
> + * The type of "sought" should be "const struct ref *const *" but for
> + * 
> http://stackoverflow.com/questions/5055655/double-pointer-const-correctness-warnings-in-c
>  .
> + */

This is an unfinished sentence, but I wonder if we even need to have
it here?  I'd be surprised if this function was unique in the
codebase that takes an array pointer whose type is looser than
necessary because of well-known language rules.

[PATCH 1/3] fetch-pack: move code to report unmatched refs to a function

[Matt McCutchen]

We're preparing to reuse this code in transport.c for "git fetch".

While I'm here, internationalize the existing error message.
---
 builtin/fetch-pack.c  |  7 +--
 fetch-pack.c  | 13 +
 fetch-pack.h  |  9 +
 t/t5500-fetch-pack.sh |  6 +++---
 4 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index cfe9e44..2a1c1c2 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -219,12 +219,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char 
*prefix)
 * remote no-such-ref' would silently succeed without issuing
 * an error.
 */
-   for (i = 0; i < nr_sought; i++) {
-   if (!sought[i] || sought[i]->matched)
-   continue;
-   error("no such remote ref %s", sought[i]->name);
-   ret = 1;
-   }
+   ret |= report_unmatched_refs(sought, nr_sought);
 
while (ref) {
printf("%s %s\n",
diff --git a/fetch-pack.c b/fetch-pack.c
index 601f077..7c8d44c 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1094,3 +1094,16 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
clear_shallow_info();
return ref_cpy;
 }
+
+int report_unmatched_refs(struct ref **sought, int nr_sought)
+{
+   int i, ret = 0;
+
+   for (i = 0; i < nr_sought; i++) {
+   if (!sought[i] || sought[i]->matched)
+   continue;
+   error(_("no such remote ref %s"), sought[i]->name);
+   ret = 1;
+   }
+   return ret;
+}
diff --git a/fetch-pack.h b/fetch-pack.h
index c912e3d..fd4d80e 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -45,4 +45,13 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
   struct sha1_array *shallow,
   char **pack_lockfile);
 
+/*
+ * Print an appropriate error message for each sought ref that wasn't
+ * matched.  Return 0 if all sought refs were matched, otherwise 1.
+ *
+ * The type of "sought" should be "const struct ref *const *" but for
+ * 
http://stackoverflow.com/questions/5055655/double-pointer-const-correctness-warnings-in-c
 .
+ */
+int report_unmatched_refs(struct ref **sought, int nr_sought);
+
 #endif
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 505e1b4..b5865b3 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -484,7 +484,7 @@ test_expect_success 'test lonely missing ref' '
cd client &&
test_must_fail git fetch-pack --no-progress .. refs/heads/xyzzy
) >/dev/null 2>error-m &&
-   test_cmp expect-error error-m
+   test_i18ncmp expect-error error-m
 '
 
 test_expect_success 'test missing ref after existing' '
@@ -492,7 +492,7 @@ test_expect_success 'test missing ref after existing' '
cd client &&
test_must_fail git fetch-pack --no-progress .. refs/heads/A 
refs/heads/xyzzy
) >/dev/null 2>error-em &&
-   test_cmp expect-error error-em
+   test_i18ncmp expect-error error-em
 '
 
 test_expect_success 'test missing ref before existing' '
@@ -500,7 +500,7 @@ test_expect_success 'test missing ref before existing' '
cd client &&
test_must_fail git fetch-pack --no-progress .. refs/heads/xyzzy 
refs/heads/A
) >/dev/null 2>error-me &&
-   test_cmp expect-error error-me
+   test_i18ncmp expect-error error-me
 '
 
 test_expect_success 'test --all, --depth, and explicit head' '
-- 
2.9.3