On Sat, Dec 17, 2022 at 12:45 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > > > ... to optionally allow the users > > to check attributes against paths from older commits. > > The above makes it sounds as if attributes are taken from places > that are unrelated to the "older commits" and the point of the > change allows "paths in an older commit" to be inspected. I do not > think that is what is going on, though. Isn't the point of the patch > to take attributes definitions from arbitrary tree-ish? > > Also, "older commits" is misleading. You may be chasing a bug in > older codebase and you have a checkout of an old commit, but you > know you have corrected the attributes definition since that old > version. In such a case, you may want to take the attributes from > the latest release and apply to the currently checked out working > tree or commit that is older. That is checking attributes taken > from newer commit. > > ... to check attributes taken from a commit other than HEAD > against paths. > > or something, perhaps? > I agree with your wording, it's much better. I'll stick to it. > > Add a new flag `--revision`/`-r` which will allow users to check the > > attributes against a tree-ish revision. > > "tree-ish revision" sounds a bit strange. We used to use "revision" > very loosely to mean an "object name", but we weaned ourselves off > of such a loose terminology over time. These days, the word > "revision" only refers to a commit (or commit-ish). > > I would understand "... against a tree-ish." If you feared that > "tree-ish" is not widely understood (which is a valid concern), then > "... against a commit (actually any tree-ish would do)" is probably > what I would write. > Thanks for explaining, I somehow keep associating revision to be the universal set, which consists of tree, commit. I'll use your wording though. > > > Since we use a tree-ish object, the user can pass "-r HEAD:subdirectory" > > and all the attributes will be looked up as if subdirectory was the root > > directory of the repository. > > Is this meant to explain a feature, or a misfeature? In other > words, when would this be useful? I would omit this paragraph if I > were writing it. It's a misfeature to be honest, I think it was called out in the previous version of the series. I'm happy to drop it, because I initially didn't include it. https://lore.kernel.org/git/674caf56-940b-8130-4a5e-ea8dc4783e81@xxxxxxxxxxxxx/ > > > We cannot use the `<rev>:<path>` syntax like the one used in `git show` > > because any non-flag parameter before `--` is treated as an attribute > > and any parameter after `--` is treated as a pathname. > > I do not see what this one wants to say. <rev>:<path> is an > established way to name an object that is sitting at the <path> in > the tree-ish whose name is <rev>. The user can use "-r > <rev>:<path>" and if the <path> in <rev> is a tree, then all the > attributes will be looked up as if <path> were the root. So the > users can use the <rev>:<path> syntax, cannot they? > Yes ofcourse, this one merely is stating why we cannot use `<rev>:<path>` directly (i.e. without the --revision flag). I'll correct it to: We cannot simply use the `<rev>:<path>` syntax without the `--revision` flag, similar to how it is used in `git show` because any non-flag parameter before `--` is treated as an attribute and any parameter after `--` is treated as a pathname. > > Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx> > > Co-authored-by: toon@xxxxxxxxx > > Co-authoring is fine, but as one of the copyright holder, the other > author needs to sign it off, too. Can I simply add this, or does Toon need to provide his approval on this list? -- - Karthik