Re: [PATCH v4 2/2] attr: add flag `--revision` to work with revisions

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 21, 2022, 21:13 UTC
Message-ID: <CAPig+cTHJzHsUum0RnyCcqxj1qhxJR9EcKRjoShCznQ_oXYHOA@mail.gmail.com>
In-Reply-To: <cc2e5f1e-8f53-ad83-1f5d-ae910ab60c53@dunelm.org.uk>

On Wed, Dec 21, 2022 at 3:58 PM Phillip Wood <phillip.wood123@gmail.com> wrote:

Show 8 quoted lines

> On 21/12/2022 13:47, Karthik Nayak wrote:
> > Since we use a tree-ish object, the user can pass "--revision
> > HEAD:subdirectory" and all the attributes will be looked up as if
> > subdirectory was the root directory of the repository.
>
> We should be clear in the documentation and option help that --revision
> takes a tree-ish (i.e. --revision=<tree-ish>). Maybe calling the option
> --tree would be clearer.

If that's the case, shouldn't this be called --source (or some variation thereof) for consistency[1] with git-restore?

[1]: https://lore.kernel.org/git/CAPig+cT_xz3m-3kkL_Scf0opNXkikU94kSF46VO8KHkNMWbGzg@mail.gmail.com/

Show 5 quoted lines

> > +attr_check_revision () {
> > +     path="$1" expect="$2" revision="$3" git_opts="$4" &&
>
> Is that line valid posix shell? I know it works with local (which is not
> in posix) but is it allowed for global variables?

This is fine as-is; it's POSIX.