From: Junio C Hamano <hidden> Date: 2016-06-15 22:57:13
Michael J Gruber [off-list ref] writes:
Adding to that:
Somehow I still feel I should introduce a new attribute "show" (or a
better name) similar to "diff" so that you can specifiy a diff driver to
use for showing a blob (or grepping it), which may or may not be the
same you use for "diff". This would be a much more fine-grained and
systematic way of setting a default for "--textconv" for blobs.
Of course, some driver attributes would just not matter for coverting
blobs, but that doesn't hurt.
I'm just wondering whether it's worth the effort and whether I should
distinguish between "show" and grep".
Haven't thought things through, but my gut feeling is that it is on
the other side of the line. We could of course add more features and
over-engineered mechanisms, and the implementation may end up to be
even modular and clean, but I cannot answer "Yes" with a confidence
to the question "Does such a fine grained control help the users?"
and cannot answer "If so in what way?" myself.
From: Jeff King <hidden> Date: 2016-06-15 22:57:14
On Sun, May 12, 2013 at 10:01:38PM -0700, Junio C Hamano wrote:
Michael J Gruber [off-list ref] writes:
quoted
Adding to that:
Somehow I still feel I should introduce a new attribute "show" (or a
better name) similar to "diff" so that you can specifiy a diff driver to
use for showing a blob (or grepping it), which may or may not be the
same you use for "diff". This would be a much more fine-grained and
systematic way of setting a default for "--textconv" for blobs.
Of course, some driver attributes would just not matter for coverting
blobs, but that doesn't hurt.
I'm just wondering whether it's worth the effort and whether I should
distinguish between "show" and grep".
Haven't thought things through, but my gut feeling is that it is on
the other side of the line. We could of course add more features and
over-engineered mechanisms, and the implementation may end up to be
even modular and clean, but I cannot answer "Yes" with a confidence
to the question "Does such a fine grained control help the users?"
and cannot answer "If so in what way?" myself.
Yeah, I think the _most_ flexible thing is going to look something like:
$ cat .gitattributes
*.pdf diff=pdf show=pdf
$ cat ~/.gitconfig
[diff "pdf"]
textconv = ...
[show "pdf"]
textconv = ...
But that obviously sucks, because in the common case that you want to
use the same command, you are repeating yourself in the config. You
could assume that the "show" attribute points us at a "diff" block. And
that makes sense for textconv, but what does it mean if you have
"show=foo" and "diff.foo.command" set?
If the _only_ thing you would want to do with such a "show" mechanism is
to display converted contents on show/grep, then we could lose the
flexibility and say that "show" is a single-bit: do we respect diff
textconv for show/grep in this case, or not? And that leaves only the
question of where to put it: is it a gitattribute, or does it go in the
config?
I don't think that it is a property of the file itself. That is, you do
not say "foo files are inherently uninteresting to git-show, and
therefore we always convert them, whereas bar files do not have that
property'. You say "in my workflows, I expect to see converted results
from grep/show". And the latter points to using config, like either
"diff.*.showConverted" (to allow per-type setting), or even
"grep.useTextconv" and "show.textConv" (to allow setting it per-user for
all types).
And of course for any workflow-oriented config, you will sometimes want
to override it for a particular operation. But that is why we have a
command-line escape hatch, and that part is already implemented.
-Peff
From: Michael J Gruber <hidden> Date: 2016-06-15 22:57:14
Jeff King venit, vidit, dixit 13.05.2013 13:55:
On Sun, May 12, 2013 at 10:01:38PM -0700, Junio C Hamano wrote:
quoted
Michael J Gruber [off-list ref] writes:
quoted
Adding to that:
Somehow I still feel I should introduce a new attribute "show" (or a
better name) similar to "diff" so that you can specifiy a diff driver to
use for showing a blob (or grepping it), which may or may not be the
same you use for "diff". This would be a much more fine-grained and
systematic way of setting a default for "--textconv" for blobs.
Of course, some driver attributes would just not matter for coverting
blobs, but that doesn't hurt.
I'm just wondering whether it's worth the effort and whether I should
distinguish between "show" and grep".
Haven't thought things through, but my gut feeling is that it is on
the other side of the line. We could of course add more features and
over-engineered mechanisms, and the implementation may end up to be
even modular and clean, but I cannot answer "Yes" with a confidence
to the question "Does such a fine grained control help the users?"
and cannot answer "If so in what way?" myself.
Yeah, I think the _most_ flexible thing is going to look something like:
$ cat .gitattributes
*.pdf diff=pdf show=pdf
$ cat ~/.gitconfig
[diff "pdf"]
textconv = ...
[show "pdf"]
textconv = ...
But that obviously sucks, because in the common case that you want to
use the same command, you are repeating yourself in the config. You
could assume that the "show" attribute points us at a "diff" block. And
that makes sense for textconv, but what does it mean if you have
"show=foo" and "diff.foo.command" set?
I don't propose "show drivers". In your example above, you would point
to the same diff driver.
If you use a diff driver just with the "show" attribute then only its
textconv config will be relevant.
But you do have the possibility to use different drivers for diff and
show. For example, for showing a file some sort of automatic pagination
or line numbering can be helpful whereas it would hurt the diff case.
If the _only_ thing you would want to do with such a "show" mechanism is
to display converted contents on show/grep, then we could lose the
flexibility and say that "show" is a single-bit: do we respect diff
textconv for show/grep in this case, or not? And that leaves only the
question of where to put it: is it a gitattribute, or does it go in the
config?
I don't think that it is a property of the file itself. That is, you do
not say "foo files are inherently uninteresting to git-show, and
therefore we always convert them, whereas bar files do not have that
property'. You say "in my workflows, I expect to see converted results
from grep/show". And the latter points to using config, like either
"diff.*.showConverted" (to allow per-type setting), or even
"grep.useTextconv" and "show.textConv" (to allow setting it per-user for
all types).
I strongly disagree here. I have textconv filters for pdf, gpg, odf,
xls, doc, xoj... I know, ugly. At least some of them would benefit from
different filteres or different settings.
The way I propose it, a user would just have to add "show=foo" to the
"diff=foo" lines without having to ad an extra filter, but with the
flexibility to do so.
And of course for any workflow-oriented config, you will sometimes want
to override it for a particular operation. But that is why we have a
command-line escape hatch, and that part is already implemented.
One may ask what a purely ui output oriented setting like "show" has to
do in .gitattributes, of course, but that applies to "diff" as well.
Separating the two (one in attributes, one in config) looks artificial
to me.
Michael
From: Jeff King <hidden> Date: 2016-06-15 22:57:16
On Mon, May 13, 2013 at 04:57:55PM +0200, Michael J Gruber wrote:
quoted
I don't think that it is a property of the file itself. That is, you do
not say "foo files are inherently uninteresting to git-show, and
therefore we always convert them, whereas bar files do not have that
property'. You say "in my workflows, I expect to see converted results
from grep/show". And the latter points to using config, like either
"diff.*.showConverted" (to allow per-type setting), or even
"grep.useTextconv" and "show.textConv" (to allow setting it per-user for
all types).
I strongly disagree here. I have textconv filters for pdf, gpg, odf,
xls, doc, xoj... I know, ugly. At least some of them would benefit from
different filteres or different settings.
OK. I was speaking mostly from intuition, and I suspect you have more
real-world experience here. So I am willing to admit that my "you do not
say..." above was a strawman. :)
The way I propose it, a user would just have to add "show=foo" to the
"diff=foo" lines without having to ad an extra filter, but with the
flexibility to do so.
Yes, I think that would work OK. The only problem is that it is a bit
weird to pointing "show=foo" to "diff.foo.*", especially when most of
the driver options are ignored. But if we can accept that wrinkle in the
UI, I think it would otherwise do what users want.
One may ask what a purely ui output oriented setting like "show" has to
do in .gitattributes, of course, but that applies to "diff" as well.
Separating the two (one in attributes, one in config) looks artificial
to me.
I think the point is that the attribute says "a property of this path is
that it has type X". And then the config says "when you see type X, do
this thing with it".
So arguably "diff=X" is wrong in the first place. It should be "type=X",
and we should have "diff.X", "merge.X", etc in the config. And
diff.*.textconv is potentially misplaced; it is not really about diffing
at all, but rather about creating a human-readable presentation for the
file. I don't think it is so bad that it is worth the pain of fixing it
now, though. It is a historical weirdness that "diff=X" means "present
the path according to the rules in X", but we can live with that.
But if we think of it that way, then automatically respecting textconv
for "git show" is a sensible thing to do. Hmph. Now I may have convinced
myself that flipping the default is the right thing. :)
So if it is not clear, I am pretty on the fence about how the defaults
should be handled, or what would surprise users the least. Either way,
though, it would probably make sense to have a configurable option. And
with the reasoning above for the split between attributes/config, it
would make sense to me for that option to be a boolean
"diff.X.showtextconv". Which seems totally odd and broken (we are not
doing a diff at all!), but that is where the textconv config lives, for
historical reasons.
-Peff