On Wed, Mar 03, 2010 at 07:50:34PM +1300, Steven Drake wrote: > On Sun, 28 Feb 2010, Jeff King wrote: > > On Sun, Feb 28, 2010 at 01:03:00PM +1300, Steven Drake wrote: > > > > > Add 'git commit --spell' to run a spell checker on commit log message. > > > The `commit.spell` configuration variable can be used to enable the spell > > > checker by default and can be turned off by '--no-spell'. > > > > Isn't this exactly the sort of thing the commit-msg hook is for? > > Accept then there would be no way of having '--spell'/'--no-spell' (Yet!). You would have to spell it: NO_SPELL=1 git commit which admittedly isn't as nice. But I'm not sure why you would want --no-spell. I guess for rebases and such where you aren't writing the message directly? But in that case, shouldn't it be passing --no-verify already (and if we go with your patch, should --no-verify perhaps imply --no-spell)? > Plus as I have just found out all hooks are run with stdin as '/dev/null' so > there is no way of running an interactive command like 'ispell' from a hook! Yes, you have to do "ispell </dev/tty". Though both that and your original suffer from somebody running "git gui" or similar in a terminal that the user is no longer looking at. I see you check for isatty(), but I don't know if that is enough for git gui (or any of the other graphical commit helpers). > > Though personally I would probably just invoke interactive spell-checking > > from the editor. > > I would probably forget to. Sure, but that is a problem with --spell, too. And you have already solved it here, with commit.spell configuration. So why does that technique not apply to configuring your editor? I'm not 100% against your patch. I'm just concerned that it is adding code and complexity for a feature that nobody will use, because everybody else is already doing the same thing through their editor, which is cleaner (e.g., we don't have to worry about handling --no-spell for scripts). -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