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