Re: [PATCH v4 00/16] pathspec cleanup

From: Duy Nguyen <pclouds@gmail.com>
Date: Jan 4, 2017, 13:56 UTC
Message-ID: <CACsJy8C=SC+gE1HQeGdE5z3+S8RHwiM-4ZXBzuwvN=+COUGNzg@mail.gmail.com>
In-Reply-To: <20170103184241.128409-1-bmwill@google.com>

On Wed, Jan 4, 2017 at 1:42 AM, Brandon Williams <bmwill@google.com> wrote:

Show 16 quoted lines

> diff --git a/dir.c b/dir.c
> index 15f7c9993..e8ddd7f8a 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1353,6 +1353,15 @@ static int simplify_away(const char *path, int pathlen,
>  {
>         int i;
>
> +       if (pathspec)
> +               guard_pathspec(pathspec,
> +                              pathspec_fromtop |
> +                              pathspec_maxdepth |
> +                              pathspec_literal |
> +                              pathspec_glob |
> +                              pathspec_icase |
> +                              pathspec_exclude);

You have done some magic (or your MTA/editor did) to lower case GUARD_PATHSPEC and all the flags. The real patch looks good though, so no problem.

> +
>         if (!pathspec || !pathspec->nr)
>                 return 0;

Super tiny nit, if GUARD_PATHSPEC is placed after this line, then we don't have to check if pathspec is non-NULL. Probably not worth a re-roll unless somebody else finds something else.

>                 if (m->mnemonic)
> -                       strbuf_addf(&sb, "'(%c)%s'", m->mnemonic, m->name);
> +                       strbuf_addf(&sb, "'%s' (mnemonic: '%c')",
> +                                   m->name, m->mnemonic);

.. and that somebody might be me :) we need to mark "mnemonic" for translation. Putting _() around the string would do.

Ideally I would give the translators the whole sentence so they can have good context (and good translation as a result). But since we potentially concatenate multiple unsupported magic in the string, there's no way to provide one (or a few) fixed string(s) at compile time. So let's just _() it and leave it at that.

-- 
Duy