On Mon, Jan 26, 2009 at 12:28:55PM +0100, Johannes Schindelin wrote:
> > Are you aware that gitweb no longer calls "git diff", exactly because
> > of problems caused by calling a porcelain from a script?
>
> As I said: do you really expect people not to forget to upgrade gitweb
> manually when they do "sudo make install" with a new Git version?
Yes.
But my point is that gitweb was _already_ broken, because it was calling
a porcelain, and there were _already_ features that could cause serious
breakage.
So yes, adding a new feature that a user can trigger causes one more
opportunity for breakage. But the solution isn't to never ever add more
features to "git diff". It's to close the avenue by which the new _and_
old breakages are triggered.
> > I don't want to break existing setups, either. But at some point you
> > have to say "this is porcelain, so don't rely on there not being any
> > user-triggered effects in its behavior". If porcelain is cast in stone,
> > then what is the point in differentiating plumbing from porcelain?
>
> Two points there:
>
> - with gitweb, we were the offenders ourselves. So we should give the
> users of gitweb at least _some_ slack.
I'm not sure I agree. I always assumed that since gitweb, git-gui, and
gitk are bundled with git during release that we have _more_ leeway in
making matching changes between them.
Are you sure that you can run random versions of gitweb with random
versions of git in the first place?
> - Concretely for the "porcelain" git diff: This workflow
>
> git diff > my-patch
> <attach and send to somebody>
>
> is probably pretty wide spread. And it is okay, a user is not a script,
> they are very much allowed to use porcelain. And we _would_ break
> expectations there.
Sorry, but what in the world are we supposed to do? Never ever allow the
user to specify diff options to a porcelain because they might impact
the output? A user who sets a config option or a command line option to
impact the output of "git diff" is responsible for how they use "git
diff".
There are already options like this in "git diff". I don't see how one
more changes anything.
> Now, I have another two, fundamental problems with the diff options
> defaults: you are restricting the thing to _one_ set of options, and when
> somebody wants to run without those options, she has to actively _undo_
> them.
Yep, that's what defaults are. And guess what: we _already_ have the
same thing. I have diff.renames set in my ~/.gitconfig. That does
_exactly_ what
git config --global diff.primer -M
would do. It's just a syntax that saves us from having to introduce a
boatload of new variables, one per command line option.
> Remember, sometimes you need another set of options. Like, when I send
> mail to a Git user, I want "-M -C -C", when I send mail to a non-Git user,
> I do not want any additional options (and try to undo "-M -C -C" on the
> command line, good luck), and sometimes it is much easier to see what
> happened with a word diff.
This is a strawman. You have described a scenario where an alias or a
wrapper script is a better fit. Great, then use that mechanism in this
scenario. But that doesn't mean there aren't other scenarios where a
different setup makes more sense (I think Keith's original goal was to
use "-w").
> So what I need are three different sets of diff options.
>
> Guess how well that works with aliases -- we are talking command line
> here after all, right?
Personally, I have always found the suggestion that users simply put
their preferences into an alias like "mydiff" to be a silly one: git has
already taken the obvious good names, so now I am stuck using "git
mydiff" forever and forgetting that "git diff" even exists.
But then, I don't have your "three sets of options" scenario. I just
want one set of defaults. So I don't have a need to name each one, and
having to choose a different name becomes a detriment rather than an
advantage.
However, there are two other drawbacks of aliases that I can think of:
1. They are tied to a specific command, whereas diff options are tied
to the concept of diffing. So now I have to write an alias (with a
new name) for each command:
git config alias.mylog 'log -w'
git config alias.mydiff 'diff -w'
git config alias.myshow 'show -w'
2. They can't change defaults based on the file to be diffed. One of
the things Keith mentioned (and I don't remember if this was
implemented in his patch series) was supporting this for
gitattributes diff drivers. How do I do
git config diff.tex.primer -w
using aliases?
But now you have me defending Keith's proposal, which he should be doing
himself ;P I actually am not that excited about it, and will probably
not use it for anything myself. But I think:
- it lets the user accomplish useful things that would not
otherwise be possible
- supporting it in "git diff" does not create any danger that was not
already there
which means that I have no objection to a clean version being applied.
-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
