Re: [PATCH] commit: make default of "cleanup" option configurable

Hi,

2013/1/9 Jonathan Nieder [off-list ref]:

Hi,

Ralf Thielow wrote:

quoted

The default of the "cleanup" option in "git commit"
is not configurable. Users who don't want to use the
default have to pass this option on every commit since
there's no way to configure it.

Could you give an example?  I'm trying to get a sense of whether these
habitual --cleanup= passers would use --cleanup=verbatim or
--cleanup=strip.

It's actually my own usecase :). The bugtracker I'm using is able
to create relationships between issues and related commits. It
expects that a part of the commit message contains the issue number
in format "#<issueId>". So I need to use a cleanup mode different
from "default" to keep the commentary. The mode I'd use is "whitespace",
"verbatim" is also possible.

I'm a bit worried that a setting like 'commit.cleanup = strip' would
be likely to break scripts.  Yes, scripts using "git commit" instead
of commit-tree while caring about detailed semantics are asking for
trouble, but I'm worried about importers, for example, which are
sometimes written by new git users.  Some scripts might assume that
"git commit" preserves the entire change description from their source
data, even when some lines happen to start by listing the ways the
author is #1.

When a user uses a script/importer which expects that the "default" option
is used without setting it explicitly, and then the user changes the default,
isn't it the users fault if that would break things?

I don't think that necessarily rules out this change, but:

 1. We need more of an explanation of the purpose than "this lets
    people type less".  What workflow does setting this option fit
    into?

 2. I would prefer to see a little thought about whether it's possible
    to avoid silently impacting scripts.  E.g., depending on the
    answer to (1), maybe supporting "verbatim" but not "strip" would be
    ok?  Or alternatively, maybe a search of public code repositories
    would reveal that new git users tend to write their importers in a
    way that this doesn't break.

As a side effect, the information gathered to answer (1) and (2) could
have the potential to make the user-level documentation more useful,
by adding context to the description of the configuration item.

I'll add a sentence of my bugtracker example to it. I think many developers
are using such a tool, so it'd makes sense.

[...]

quoted

--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -103,7 +103,7 @@ static enum {
      CLEANUP_NONE,
      CLEANUP_ALL
 } cleanup_mode;
-static char *cleanup_arg;
+const static char *cleanup_arg;

Style nit: storage class comes first, then the type.  Otherwise the
typename "const char *" is split up.

Thanks.
I'll send a new version of the patch including changes of your and
Junios comments.

Ralf