Re: [PATCH v3 2/2] attr: add flag `-r|--revisions` to work with revisions

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> 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?

> 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.

> When the user uses this flag, we
> go through the stack of .gitattributes files but instead of checking the
> current working tree and/or in the index, we check the blobs from the
> provided tree-ish object. This allows the command to also be used in
> bare repositories.

Good.

> 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.

> 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?

> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> Co-authored-by: toon@iotcl.com

Co-authoring is fine, but as one of the copyright holder, the other
author needs to sign it off, too.