On Fri, Dec 16, 2022 at 5:30 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > > > On Fri, Dec 16 2022, Karthik Nayak wrote: > > > v1: https://lore.kernel.org/git/20221206103736.53909-1-karthik.188@xxxxxxxxx/ > > v2: https://lore.kernel.org/git/CAOLa=ZSsFGBw3ta1jWN8cmUch2ca=zTEjp1xMA6Linafx9W53g@xxxxxxxxxxxxxx/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? > I didn't know, will do this from next time! > > 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@xxxxxxxxxxxxxx/; > Right, I was open to it, but since there wasn't any confirmation, I didn't go forward with it. Will remove it from the next version. > I'm still more partial to the alternate suggestion I had in > https://lore.kernel.org/git/221207.86lenja0zi.gmgdl@xxxxxxxxxxxxxxxxxxx/; > I'm not sure what you meant in your reply at > https://lore.kernel.org/git/CAOLa=ZQua8TfApCdzoK06_2fkWb4ZCfWewXKOSaXno1fqFSq2A@xxxxxxxxxxxxxx/ > (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? > I'm not opposing the proposal, rather stating my opinion on it. To go over my reply I'm only saying that most users of Git are accustomed to the `<revision>:<path>` syntax and now breaking that only in one command to be `<revision>:<attr>` seems a bit odd, from the user experience point of view. > 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 see what you mean, but I was referring to consistency around how different options are used in other git commands. Mainly that most commands treat the second section after `<rev>:` to be a path, now adding a new option where the section after `<rev>:` to be an attribute, might be a bit confusing. > 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 "--". > Exactly. > 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. > I don't disagree. I think it's healthy to have this discussion, especially since we're adding a new option and this is the right time. I'm all ears and finally want to get the best solution. -- - Karthik
