Re: [PATCH v3 0/2] check-attr: add support to work with revisions

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Karthik Nayak <karthik.188@gmail.com>
Cc: git@vger.kernel.org, toon@iotcl.com
Subject: Re: [PATCH v3 0/2] check-attr: add support to work with revisions
Date: Fri, 16 Dec 2022 17:17:48 +0100	[thread overview]
Message-ID: <221216.86k02r1fcl.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <20221216093552.3171319-1-karthik.188@gmail.com>

On Fri, Dec 16 2022, Karthik Nayak wrote:

> v1: https://lore.kernel.org/git/20221206103736.53909-1-karthik.188@gmail.com/
> v2: https://lore.kernel.org/git/CAOLa=ZSsFGBw3ta1jWN8cmUch2ca=zTEjp1xMA6Linafx9W53g@mail.gmail.com/T/#t

Could you please set the In-Reply-To header appropriately in the future,
so that each version of this series isn't in its own disconnected
thread?

> This series aims to add a new flag `-r|--revisions` to git-check-attr(1) which
> allows us to read gitattributes from the specified revision.

I didn't look at the v2, but expected at least the short form to be gone
here re
https://lore.kernel.org/git/CAOLa=ZTSzUh2Ma_EMHHWcDunGyKMaUW9BaG=QdegtMqLd+69Wg@mail.gmail.com/;

I'm still more partial to the alternate suggestion I had in
https://lore.kernel.org/git/221207.86lenja0zi.gmgdl@evledraar.gmail.com/;
I'm not sure what you meant in your reply at
https://lore.kernel.org/git/CAOLa=ZQua8TfApCdzoK06_2fkWb4ZCfWewXKOSaXno1fqFSq2A@mail.gmail.com/
(sorry about not following up at the time) with:

	"when being consistent we need to be fully consistent,
	i.e. <revision>:<path>, tweaking this slightly to be
	<revision>:<attr> is worse than breaking consistency."

Yes, it would, but isn't that by definition the case with any
proposal?

We don't have a way to refer to an attribute (or all attributes for -a)
for a given revision/path, the task of this series is to invent such a
syntax.

So we could invent that as this series currently does with:

	git check-attrs --revision <rev> <attr>... <path>...

Or, as I suggested:

        git check-attr [<rev>:]<attr>... -- <path>...

Or whatever. Here I'm not saying that one is better than the other, but
advocating for one on the basis of consistency doesn't make sense to me,
this is new syntax.

I think what you mean is that because the log family uses "<rev>:<path>"
we should not come up with a syntax that looks anything like
"<lhs>:<rhs>"., as the "<lhs>" in the mind of some users is going to be
"<rev>", and the "<rhs>" is "<path>", so it would be confusing to have
it be "<attr>" here, and have the "<path>..." come after the "--".

I'm not convinced by that. From refspecs to e.g. "git log"'s own "-L" we
have little mini-syntaxes in various places that use this sort of colon
notation. I find it more elegant than "--revision".

It's fine if you disagree, I'm just trying to understand the basis of
the disagreement.