Commit log message typos and spell mistakes

Commit log message typos and spell mistakes

[Steven Drake]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 551 bytes --]

On Fri, 26 Feb 2010, Alex Riesen wrote:
> On Fri, Feb 26, 2010 at 05:00, Steven Drake <sdrake@xnet.co.nz> wrote:
> > It is not a good Idea to give a config_error for _any_ keyword without a
> > value as it means that
> >
> >        [section]
> >                keyword
> >
> > sytle can not be using for setting bool type keyword.
> 
> Typo: sytle (style?)

Yes dammit, time to teach git to do spell checking!

-- 
Steven
  1: Linux - will work for fish.
  2: The Linux penguin - looks stuffed to the brim with herring.
  ( make your own conclusions )

[PATCH 1/1] Add commit log message spell checking feature.

[Steven Drake]

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'.

The spell checker shell command to run is specified by the
`commit.spellcommand` configuration variable and defaults to `ispell`.

Because it is assumed that the spell checker is an interactive command spell
checking is only done if stdio is a terminal.

Signed-off-by: Steven Drake <sdrake@xnet.co.nz>
---
The above commit log message was spell check using this feature.

 Documentation/config.txt     |    9 +++++++++
 Documentation/git-commit.txt |    8 ++++++++
 builtin/commit.c             |   33 ++++++++++++++++++++++++++++++++-
 3 files changed, 49 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 793c9a8..f977a45 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -737,6 +737,15 @@ color.ui::
 	terminal. When more specific variables of color.* are set, they always
 	take precedence over this setting. Defaults to false.
 
+commit.spell::
+	A boolean to enable/disable running of the spell checker on commit
+	log message. Defaults to false.
+
+commit.spellcommand::
+	Shell command to run as the spell checker. First argument given to
+	the command is the name of the file containing the commit log message.
+	Defaults to `ispell`.  See 'ispell(1)'.
+
 commit.status::
 	A boolean to enable/disable inclusion of status information in the
 	commit message template when using an editor to prepare the commit
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 64fb458..d68106f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -120,6 +120,14 @@ OPTIONS
 	Add Signed-off-by line by the committer at the end of the commit
 	log message.
 
+--spell::
+	Run the spell checker on commit log message. This overrides the
+	`commit.spell` configuration variable.
+
+--no-spell::
+	Do not run the spell checker on commit log message. This overrides the
+	`commit.spell` configuration variable.
+
 -n::
 --no-verify::
 	This option bypasses the pre-commit and commit-msg hooks.
diff --git a/builtin/commit.c b/builtin/commit.c
index 8dd104e..272faa7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -66,7 +66,8 @@ static char *edit_message, *use_message;
 static char *author_name, *author_email, *author_date;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
-static int no_post_rewrite;
+static int no_post_rewrite, spell_check;
+static const char *spell_command = (const char*)"ispell";
 static char *untracked_files_arg, *force_date;
 /*
  * The default commit message cleanup mode will remove the lines
@@ -93,6 +94,11 @@ static enum {
 	STATUS_FORMAT_PORCELAIN,
 } status_format = STATUS_FORMAT_LONG;
 
+static inline int have_terminal(void)
+{
+	return (isatty(0) && isatty(1) && isatty(2));
+}
+
 static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 {
 	struct strbuf *buf = opt->value;
@@ -118,6 +124,7 @@ static struct option builtin_commit_options[] = {
 	OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
 	OPT_BOOLEAN(0, "reset-author", &renew_authorship, "the commit is authored by me now (used with -C-c/--amend)"),
 	OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
+	OPT_BOOLEAN(0, "spell", &spell_check, "run spell checker on commit message"),
 	OPT_FILENAME('t', "template", &template_file, "use specified template file"),
 	OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
 	OPT_STRING(0, "cleanup", &cleanup_arg, "default", "how to strip spaces and #comments from message"),
@@ -525,6 +532,21 @@ static int ends_rfc2822_footer(struct strbuf *sb)
 	return 1;
 }
 
+static int spell_checker(const char *commit_editmsg)
+{
+	const char *terminal = getenv("TERM");
+	int terminal_is_dumb = !terminal || !strcmp(terminal, "dumb");
+
+	if (terminal_is_dumb || !have_terminal()) {
+		error("No Terminal, can not run spell checker");
+		return 0;
+	}
+
+	const char *args[] = { spell_command, commit_editmsg, NULL };
+
+	return run_command_v_opt(args, RUN_USING_SHELL);
+}
+
 static int prepare_to_commit(const char *index_file, const char *prefix,
 			     struct wt_status *s)
 {
@@ -726,6 +748,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		}
 	}
 
+	if (spell_check && spell_checker(git_path(commit_editmsg)))
+		return 0;
+
 	if (!no_verify &&
 	    run_hook(index_file, "commit-msg", git_path(commit_editmsg), NULL)) {
 		return 0;
@@ -1154,6 +1179,12 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 
 	if (!strcmp(k, "commit.template"))
 		return git_config_pathname(&template_file, k, v);
+	if (!strcmp(k, "commit.spell")) {
+		spell_check = git_config_bool(k, v);
+		return 0;
+	}
+	if (!strcmp(k, "commit.spellcommand"))
+		return git_config_string(&spell_command, k, v);
 	if (!strcmp(k, "commit.status")) {
 		include_status = git_config_bool(k, v);
 		return 0;
-- 
1.7.0.323.g7e21d

Re: [PATCH 1/1] Add commit log message spell checking feature.

[Jeff King]

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? Though
personally I would probably just invoke interactive spell-checking from
the editor.

-Peff

Re: [PATCH 1/1] Add commit log message spell checking feature.

[Steven Drake]

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!).

> Though personally I would probably just invoke interactive spell-checking
> from the editor.

I would probably forget to.

-- 
Steven
I'm such a prick!  --- The Bastard Operator from Hell

Re: [PATCH 1/1] Add commit log message spell checking feature.

[Steven Drake]

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?

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!

-- 
Steven
  1: Linux - will work for fish.
  2: The Linux penguin - looks stuffed to the brim with herring.
  ( make your own conclusions )

Re: [PATCH 1/1] Add commit log message spell checking feature.

[Teemu Likonen]

* 2010-02-28 11:33 (-0500), 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?
> Though personally I would probably just invoke interactive
> spell-checking from the editor.

I agree that it's a job for user's text editor.

Re: [PATCH 1/1] Add commit log message spell checking feature.

[Jeff King]

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