On 01/04, Duy Nguyen wrote: > On Wed, Jan 4, 2017 at 1:42 AM, Brandon Williams <bmwill@xxxxxxxxxx> wrote: > > 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. That's really odd, I was sure I just did a cut and paste. I'll fix that :) > > > + > > 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. I saw this after I sent out the series again, and I agree. I'll move the guard to be after this check. > > > 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. Will do! -- Brandon Williams