Re: [PATCH v2 04/16] ref-filter: modify "%(objectname:short)" to take length

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 15, 2016 at 9:47 AM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
> Add support for %(objectname:short=<length>) which would print the
> abbreviated unique objectname of given length. When no length is
> specified 7 is used. The minimum length is 'MINIMUM_ABBREV'.
>

Isn't the default abbreviation value used here, not hard coded to 7?
Thus user can configure this?

Description should also mention that length may be exceeded if it is
not long enough to be a unique identifier.

> Add tests and documentation for the same.
>

Also I didn't see any update to the documentation here..

> Mentored-by: Christian Couder <christian.couder@xxxxxxxxx>
> Mentored-by: Matthieu Moy <matthieu.moy@xxxxxxxxxxxxxxx>
> Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
> ---
>  ref-filter.c            | 25 +++++++++++++++++++------
>  t/t6300-for-each-ref.sh | 10 ++++++++++
>  2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 857a8b5..17f781d 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -55,7 +55,10 @@ static struct used_atom {
>                         const char *if_equals,
>                                 *not_equals;
>                 } if_then_else;
> -               enum { O_FULL, O_SHORT } objectname;
> +               struct {
> +                       enum { O_FULL, O_LENGTH, O_SHORT } option;
> +                       unsigned int length;
> +               } objectname;
>         } u;
>  } *used_atom;
>  static int used_atom_cnt, need_tagged, need_symref;
> @@ -118,10 +121,17 @@ static void contents_atom_parser(struct used_atom *atom, const char *arg)
>  static void objectname_atom_parser(struct used_atom *atom, const char *arg)
>  {
>         if (!arg)
> -               atom->u.objectname = O_FULL;
> +               atom->u.objectname.option = O_FULL;
>         else if (!strcmp(arg, "short"))
> -               atom->u.objectname = O_SHORT;
> -       else
> +               atom->u.objectname.option = O_SHORT;
> +       else if (skip_prefix(arg, "short=", &arg)) {
> +               atom->u.contents.option = O_LENGTH;
> +               if (strtoul_ui(arg, 10, &atom->u.objectname.length) ||
> +                   atom->u.objectname.length == 0)
> +                       die(_("positive value expected objectname:short=%s"), arg);
> +               if (atom->u.objectname.length < MINIMUM_ABBREV)
> +                       atom->u.objectname.length = MINIMUM_ABBREV;

Should this error instead of accepting and upgrading it to the minimum?

> +       } else
>                 die(_("unrecognized %%(objectname) argument: %s"), arg);
>  }
>
> @@ -591,12 +601,15 @@ static int grab_objectname(const char *name, const unsigned char *sha1,
>                            struct atom_value *v, struct used_atom *atom)
>  {
>         if (starts_with(name, "objectname")) {
> -               if (atom->u.objectname == O_SHORT) {
> +               if (atom->u.objectname.option == O_SHORT) {
>                         v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));

Yes, here we are using DEFAULT_ABBREV, so you should mention that
instead of using 7 in the commit message.

>                         return 1;
> -               } else if (atom->u.objectname == O_FULL) {
> +               } else if (atom->u.objectname.option == O_FULL) {
>                         v->s = xstrdup(sha1_to_hex(sha1));
>                         return 1;
> +               } else if (atom->u.objectname.option == O_LENGTH) {
> +                       v->s = xstrdup(find_unique_abbrev(sha1, atom->u.objectname.length));
> +                       return 1;
>                 } else
>                         die("BUG: unknown %%(objectname) option");
>         }
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 19a2823..2be0a3f 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -60,6 +60,8 @@ test_atom head objecttype commit
>  test_atom head objectsize 171
>  test_atom head objectname $(git rev-parse refs/heads/master)
>  test_atom head objectname:short $(git rev-parse --short refs/heads/master)
> +test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
> +test_atom head objectname:short=10 $(git rev-parse --short=10 refs/heads/master)
>  test_atom head tree $(git rev-parse refs/heads/master^{tree})
>  test_atom head parent ''
>  test_atom head numparent 0
> @@ -99,6 +101,8 @@ test_atom tag objecttype tag
>  test_atom tag objectsize 154
>  test_atom tag objectname $(git rev-parse refs/tags/testtag)
>  test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
> +test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
> +test_atom head objectname:short=10 $(git rev-parse --short=10 refs/heads/master)
>  test_atom tag tree ''
>  test_atom tag parent ''
>  test_atom tag numparent ''
> @@ -164,6 +168,12 @@ test_expect_success 'Check invalid format specifiers are errors' '
>         test_must_fail git for-each-ref --format="%(authordate:INVALID)" refs/heads
>  '
>
> +test_expect_success 'arguments to %(objectname:short=) must be positive integers' '
> +       test_must_fail git for-each-ref --format="%(objectname:short=0)" &&
> +       test_must_fail git for-each-ref --format="%(objectname:short=-1)" &&
> +       test_must_fail git for-each-ref --format="%(objectname:short=foo)"
> +'
> +
>  test_date () {
>         f=$1 &&
>         committer_date=$2 &&
> --
> 2.7.3
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]