Re: [PATCH 16/17] pathspec: small readability changes

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

 



On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@xxxxxxxxxx> wrote:
> A few small changes to improve readability.  This is done by grouping related
> assignments, adding blank lines, ensuring lines are <80 characters, etc.
>
> Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx>
> ---
>  pathspec.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/pathspec.c b/pathspec.c
> index 41aa213..8a07b02 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -334,6 +334,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>         if ((magic & PATHSPEC_LITERAL) && (magic & PATHSPEC_GLOB))
>                 die(_("%s: 'literal' and 'glob' are incompatible"), elt);
>
> +       /* Create match string which will be used for pathspec matching */
>         if (pathspec_prefix >= 0) {
>                 match = xstrdup(copyfrom);
>                 prefixlen = pathspec_prefix;
> @@ -341,11 +342,16 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>                 match = xstrdup(copyfrom);
>                 prefixlen = 0;
>         } else {
> -               match = prefix_path_gently(prefix, prefixlen, &prefixlen, copyfrom);
> +               match = prefix_path_gently(prefix, prefixlen,
> +                                          &prefixlen, copyfrom);
>                 if (!match)
>                         die(_("%s: '%s' is outside repository"), elt, copyfrom);
>         }
> +
>         item->match = match;
> +       item->len = strlen(item->match);
> +       item->prefix = prefixlen;
> +
>         /*
>          * Prefix the pathspec (keep all magic) and assign to
>          * original. Useful for passing to another command.
> @@ -362,8 +368,6 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>         } else {
>                 item->original = xstrdup(elt);
>         }
> -       item->len = strlen(item->match);
> -       item->prefix = prefixlen;
>
>         if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)
>             strip_submodule_slash_cheap(item);
> @@ -371,13 +375,14 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>         if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)
>             strip_submodule_slash_expensive(item);
>
> -       if (magic & PATHSPEC_LITERAL)
> +       if (magic & PATHSPEC_LITERAL) {
>                 item->nowildcard_len = item->len;
> -       else {
> +       } else {
>                 item->nowildcard_len = simple_length(item->match);
>                 if (item->nowildcard_len < prefixlen)
>                         item->nowildcard_len = prefixlen;
>         }
> +
>         item->flags = 0;

You probably can move this line up with the others too.

And since you have broken this function down so nicely, it made me see
that we could do

item->magic = magic instead of returning "magic" at the end, which is
assigned to item->magic anyway by the caller.
-- 
Duy



[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]