On Mon, Dec 14, 2009 at 03:31:38PM -0800, Junio C Hamano wrote:
> The change to do so looks like this; it has a few side effects:
>
> - If somebody else were relying on the fact that 'nkf -w' names the
> entire command, it now will run 'nkf' command with '-w' as an argument,
> and it will break such a set-up. IOW, command that has an IFS white
> space in its path will now need to be quoted from the shell.
>
> You can see the fallout from this in the damage made to t/ hierarchy in
> the attached patch.
>
> - You can now use $HOME and other environment variables your shell
> expands when defining your textconv command.
>
> Overall I think it is a good direction to go, but we need to be careful
> about how we transition the existing repositories that use the old
> semantics.
I agree that this is a good change. My only reservation would be that
spawning a shell would be slightly slower (think "git log -p"). However:
1. textconv is dog-slow already, as it has to dump each blob to a
tempfile[1].
2. There is an obvious optimization, which is that you can skip the
shell if there are no metacharacters (in fact, we seem to be doing
that already in launch_editor).
> We might need to introduce diff.*.xtextconv or something.
I am torn on this. I don't like introducing behavior changes that might
hurt people (and really, I think we are just talking about people with
textconv pointing to a program by its full path that has a space in it).
But I also hate carrying around baggage crap like xtextconv forever. It
makes git harder to explain why there are two variables that do almost
the same thing, and the one they want to use is probably the one with
the crappier, unlikely-to-be-guessed name.
So I am somewhat inclined to call it a bug fix, but I don't feel very
strongly.
-Peff
[1] The current textconv interface is really nice for things like just
using "antiword" out of the box. But I wrote a new interface which can
be much faster: it calls the textconv filter with the blob name, and
then the filter is responsible for using cat-file to get at the blob.
This means the filter can look at only part of a blob (e.g., if we are
interested in the metadata tags at the beginning of a large media file),
and it can cache answers as it sees fit, avoiding access to the blob
entirely.
I need to polish the code a bit and submit it. Obviously this is not
meant to replace the existing textconv, but rather to supplement it, for
when "fast and inconvenient" is better than "slow and simple". What is
the best way to configure this? I can imagine "diff.*.textconvType =
fast", or also "diff.*.fastTextconv".
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html