All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: Jeff King <peff@peff.net>
Cc: Matthieu Moy <Matthieu.Moy@imag.fr>, git@vger.kernel.org
Subject: Re: [PATCH 0/4] diff text conversion filter
Date: Tue, 07 Oct 2008 18:15:24 +0200	[thread overview]
Message-ID: <48EB8B1C.9060201@viscovery.net> (raw)
In-Reply-To: <20081007154646.GB26531@coredump.intra.peff.net>

Jeff King schrieb:
> On Tue, Oct 07, 2008 at 08:15:45AM +0200, Matthieu Moy wrote:
> 
>> One proposal: have a diff.<driver>.activate with several values:
>>
>> * "always": activate the diff driver in any porcelain
>> * "diff": activate it only for "git diff", as currently
>> * "singlefile": Johannes's heuristic proposal
>>
>> That way, one could say easily "activate exiftags filter all the time,
>> but MS-Word only when I request a diff for a single file", and this
>> leaves room for other values if the need be. Well, there's no room for
>> "use MS-Word native diff tool in git-gui but antiword/catdoc +
>> textconv in 'git log -p'" here, but do we want it?
>>
>> Or is all that just overkill?
> 
> Actually, as soon as I read the first part of your mail I thought of the
> "ms-word vs antiword" situation. The example you gave seems plausible.
> And it seems to me that it is really a superset of the problem we are
> discussing. That is, gitattributes is really just saying "this is a
> 'foo' type file". And if we have a mechanism flexible enough to say "in
> this situation, this is how you handle 'foo' type files", then that
> would work as the basis for implementing these heuristics.
> 
> For example, maybe you could set up some mapping like:
> 
>   git diff --diffdriver=foo,bar
> 
> where a file with gitattribute diff=x would look for the driver config
> for x.foo, then x.bar, and then finally fall back to just x. Then you
> could easily have (ignoring the fact that I'm not sure about the config
> syntax for having _3_ section parts):
> 
>   git config diff.doc.graphical.command ms-word-diff-script
>   git config diff.doc.textconv antiword
> 
> and then git-gui would be configured to diff with:
> 
>   git diff --diffdriver=graphical
> 
> whereas a regular "git diff" would always fall back to
> "diff.doc.textconv". Make sense?

I think you are complicating things. We already have

   diff.doc.command ms-word-diff-script

and with your 4-patch-series-under-discussion we would have

   diff.doc.texconv antiword

and that should be sufficient. I'm proposing this heuristics:

 * If only textconv is given, all porcelains pick it.
 * If only command is given, all porcelains pick it.
 * If both are given, then
   - git log picks textconv.
   - git show and git diff:
     . if exactly one pathspec was given, pick command;
     . otherwise pick textconv

Plumbing never picks any of them, just like today, nor should git
format-patch. The are other porcelains that could be sorted into this
list, like git blame and (the summary line of) git commit.

BTW, please don't take git-gui as an example that would lauch MS-Word on
each diff. (Neither would gitk.) Both rely on plumbing, and that's good.
gitk has a menu entry "External diff", where the diff.doc.command could be
hooked into.

-- Hannes

  reply	other threads:[~2008-10-07 16:16 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-28  2:06 Implementation of a "textconv" filter for easy custom diff Matthieu Moy
2008-09-28  2:06 ` [PATCH] Facility to have multiple kinds of drivers for diff Matthieu Moy
2008-09-28  2:06   ` [PATCH] Implement run_command_to_buf (spawn a process and reads its stdout) Matthieu Moy
2008-09-28  2:06     ` [PATCH] Implement a textconv filter for "git diff" Matthieu Moy
2008-09-28  2:06       ` [PATCH] Document the textconv filter Matthieu Moy
2008-09-28  2:06         ` [PATCH] Add a basic test for " Matthieu Moy
2008-09-28 11:07         ` [PATCH] Document " Johannes Sixt
2008-09-28 12:29           ` Matthieu Moy
2008-09-28  4:15       ` [PATCH] Implement a textconv filter for "git diff" Jeff King
2008-09-28 10:00         ` Matthieu Moy
2008-09-28 16:12           ` Jeff King
2008-09-28  4:10 ` Implementation of a "textconv" filter for easy custom diff Jeff King
2008-09-28  9:57   ` Matthieu Moy
2008-09-28 16:11     ` Jeff King
2008-09-30 15:19       ` Matthieu Moy
2008-09-30 16:45         ` Jeff King
2008-10-05 21:41           ` [PATCH 0/4] diff text conversion filter Jeff King
2008-10-05 21:42             ` [PATCH 1/4] t4012: use test_cmp instead of cmp Jeff King
2008-10-05 21:43             ` [PATCH 2/4] diff: unify external diff and funcname parsing code Jeff King
2008-10-05 21:43             ` [PATCH 3/4] diff: introduce diff.<driver>.binary Jeff King
2008-10-07 15:17               ` Johannes Sixt
2008-10-07 15:35                 ` Jeff King
2008-10-07 15:54                   ` Johannes Sixt
2008-10-12  5:24                   ` Junio C Hamano
2008-10-13  1:23                     ` Jeff King
2008-10-13  4:00                       ` Junio C Hamano
2008-10-13  4:15                         ` Jeff King
2008-10-13  6:10                           ` Johannes Sixt
2008-10-13 13:54                           ` Junio C Hamano
2008-10-13  8:12                         ` Matthieu Moy
2008-10-24  2:46               ` Jeff King
2008-10-24  2:48                 ` [PATCH 1/5] diff: add missing static declaration Jeff King
2008-10-24  2:50                 ` [PATCH 2/5] add userdiff textconv tests Jeff King
2008-10-24  2:53                 ` [PATCH 3/5] refactor userdiff textconv code Jeff King
2008-10-24  7:15                   ` Johannes Sixt
2008-10-24 12:40                     ` Jeff King
2008-10-24 13:51                     ` Jeff King
2008-10-24 14:01                       ` Johannes Sixt
2008-10-24 14:08                         ` Jeff King
2008-10-24 21:12                   ` Junio C Hamano
2008-10-24 22:50                     ` Jeff King
2008-10-24 22:56                       ` Jeff King
2008-10-25  0:48                         ` Jeff King
2008-10-25  0:50                           ` [PATCH 1/7] diff: add missing static declaration Jeff King
2008-10-25  0:51                           ` [PATCH 2/7] add userdiff textconv tests Jeff King
2008-10-25  0:52                           ` [PATCH 3/7] textconv: assume text-converted contents are not binary Jeff King
2008-10-25  0:52                           ` [PATCH 4/7] textconv: don't convert for every operation Jeff King
2008-10-25  5:41                             ` Junio C Hamano
2008-10-25  7:19                               ` Jeff King
2008-10-25 18:32                                 ` Junio C Hamano
2008-10-25 19:35                                   ` Jeff King
2008-10-25 23:35                                     ` Junio C Hamano
2008-10-25 23:48                                     ` Junio C Hamano
2008-10-26  4:52                                       ` Jeff King
2008-10-26  4:38                                     ` Jeff King
2008-10-26  4:41                                       ` [PATCH v3 1/8] diff: add missing static declaration Jeff King
2008-10-26  4:41                                       ` [PATCH v3 2/8] document the diff driver textconv feature Jeff King
2008-10-26  4:42                                       ` [PATCH v3 3/8] add userdiff textconv tests Jeff King
2008-10-26  4:44                                       ` [PATCH v3 4/8] refactor userdiff textconv code Jeff King
2008-10-26  4:45                                       ` [PATCH v3 5/8] userdiff: require explicitly allowing textconv Jeff King
2008-10-26  4:46                                       ` [PATCH v3 6/8] only textconv regular files Jeff King
2008-10-26  4:49                                       ` [PATCH v3 7/8] wt-status: load diff ui config Jeff King
2008-10-27  5:30                                         ` Junio C Hamano
2008-10-27  8:23                                           ` Jeff King
2008-10-26  4:50                                       ` [PATCH v3 8/8] enable textconv for diff in verbose status/commit Jeff King
2008-10-25  0:54                           ` [PATCH 5/7] userdiff: require explicitly allowing textconv Jeff King
2008-10-25  0:54                           ` [PATCH 6/7] document the diff driver textconv feature Jeff King
2008-10-25  0:55                           ` [PATCH 7/7] only textconv regular files Jeff King
2008-10-24  2:55                 ` [PATCH 4/5] userdiff: require explicitly allowing textconv Jeff King
2008-10-24  7:04                   ` Johannes Sixt
2008-10-24  2:56                 ` [PATCH 5/5] document the diff driver textconv feature Jeff King
2008-10-24  7:02                 ` [PATCH 3/4] diff: introduce diff.<driver>.binary Johannes Sixt
2008-10-05 21:43             ` [PATCH 4/4] diff: add filter for converting binary to text Jeff King
2008-10-05 22:03             ` [PATCH 0/4] diff text conversion filter Jakub Narebski
2008-10-06  6:29             ` Johannes Sixt
2008-10-06  6:52               ` Jeff King
2008-10-06  8:55                 ` Johannes Sixt
2008-10-06 15:15               ` Matthieu Moy
2008-10-07  1:20                 ` Jeff King
2008-10-07  5:52                   ` Johannes Sixt
2008-10-07  6:00                     ` Jeff King
2008-10-07  6:15                       ` Matthieu Moy
2008-10-07 15:46                         ` Jeff King
2008-10-07 16:15                           ` Johannes Sixt [this message]
2008-10-13  1:29                             ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48EB8B1C.9060201@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=Matthieu.Moy@imag.fr \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.