[PATCH] Make commit help text more accurate for --verbose

[PATCH] Make commit help text more accurate for --verbose

[Ramana Kumar]

Commit message help text says lines starting with '#' will be ignored.
The diff lines added by --verbose are also ignored, although they don't
start with '#'s. Extend help text to describe behavior on diff lines
when relevant.

Signed-off-by: Ramana Kumar <ramana.kumar@gmail.com>
---
 builtin/commit.c |   25 ++++++++++++++++++-------
 1 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 66fdd22..b49fd7a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -658,17 +658,28 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		fprintf(fp,
 			"\n"
 			"# Please enter the commit message for your changes.");
-		if (cleanup_mode == CLEANUP_ALL)
+		if (cleanup_mode == CLEANUP_ALL && !verbose)
 			fprintf(fp,
 				" Lines starting\n"
 				"# with '#' will be ignored, and an empty"
 				" message aborts the commit.\n");
-		else /* CLEANUP_SPACE, that is. */
-			fprintf(fp,
-				" Lines starting\n"
-				"# with '#' will be kept; you may remove them"
-				" yourself if you want to.\n"
-				"# An empty message aborts the commit.\n");
+    else {
+      if (cleanup_mode == CLEANUP_ALL)
+        fprintf(fp,
+          " Lines starting\n"
+          "# with '#' will be ignored, as will"
+          " the diff and anything below it.\n");
+      else { /* CLEANUP_SPACE, that is. */
+        fprintf(fp,
+          " Lines starting\n"
+          "# with '#' will be kept; you may remove them"
+          " yourself if you want to.\n");
+        if (verbose)
+          fprintf(fp,
+            "# The diff and anything below it will be ignored.\n");
+      }
+      fprintf(fp, "# An empty message aborts the commit.\n");
+    }
 		if (only_include_assumed)
 			fprintf(fp, "# %s\n", only_include_assumed);
 
-- 
1.7.2.2

Re: [PATCH] Make commit help text more accurate for --verbose

[Thiago Farina]

Hi Ramana,

Some *style* comments below.
On Sun, Sep 5, 2010 at 6:53 AM, Ramana Kumar <ramana.kumar@gmail.com> wrote:
>                fprintf(fp,
>                        "\n"
>                        "# Please enter the commit message for your changes.");
> -               if (cleanup_mode == CLEANUP_ALL)
> +               if (cleanup_mode == CLEANUP_ALL && !verbose)
>                        fprintf(fp,
>                                " Lines starting\n"
>                                "# with '#' will be ignored, and an empty"
>                                " message aborts the commit.\n");
The Documentation/CodingStyle says to avoid using braces unncessary.
But since this is not a single line statement, like:
if (foo)
  foo = x;

Could you wrapp this into { } ?

> -               else /* CLEANUP_SPACE, that is. */
> -                       fprintf(fp,
> -                               " Lines starting\n"
> -                               "# with '#' will be kept; you may remove them"
> -                               " yourself if you want to.\n"
> -                               "# An empty message aborts the commit.\n");
> +    else {
> +      if (cleanup_mode == CLEANUP_ALL)
> +        fprintf(fp,
> +          " Lines starting\n"
> +          "# with '#' will be ignored, as will"
> +          " the diff and anything below it.\n");

Same thing here (and below) as pointed above.

> +      else { /* CLEANUP_SPACE, that is. */
> +        fprintf(fp,
> +          " Lines starting\n"
> +          "# with '#' will be kept; you may remove them"
> +          " yourself if you want to.\n");
> +        if (verbose)
> +          fprintf(fp,
> +            "# The diff and anything below it will be ignored.\n");
> +      }
> +      fprintf(fp, "# An empty message aborts the commit.\n");
> +    }
>                if (only_include_assumed)
>                        fprintf(fp, "# %s\n", only_include_assumed);
>

Re: [PATCH] Make commit help text more accurate for --verbose

[Ævar Arnfjörð Bjarmason]

On Sun, Sep 5, 2010 at 15:06, Thiago Farina <tfransosi@gmail.com> wrote:
> Hi Ramana,
>
> Some *style* comments below.
> On Sun, Sep 5, 2010 at 6:53 AM, Ramana Kumar <ramana.kumar@gmail.com> wrote:
>>                fprintf(fp,
>>                        "\n"
>>                        "# Please enter the commit message for your changes.");
>> -               if (cleanup_mode == CLEANUP_ALL)
>> +               if (cleanup_mode == CLEANUP_ALL && !verbose)
>>                        fprintf(fp,
>>                                " Lines starting\n"
>>                                "# with '#' will be ignored, and an empty"
>>                                " message aborts the commit.\n");
> The Documentation/CodingStyle says to avoid using braces unncessary.
> But since this is not a single line statement, like:
> if (foo)
>  foo = x;
>
> Could you wrapp this into { } ?

The braces are still unncessary if the function call is spread across
a few lines. I haven't seen anything in the source that indicates that
we prefer braces when a braceless if/else has an associated statement
that exceeds 1 line.

Re: [PATCH] Make commit help text more accurate for --verbose

[Ramana Kumar]

On Mon, Sep 6, 2010 at 2:30 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Sun, Sep 5, 2010 at 15:06, Thiago Farina <tfransosi@gmail.com> wrote:
>> Hi Ramana,
>>
>> Some *style* comments below.
>> On Sun, Sep 5, 2010 at 6:53 AM, Ramana Kumar <ramana.kumar@gmail.com> wrote:
>>>                fprintf(fp,
>>>                        "\n"
>>>                        "# Please enter the commit message for your changes.");
>>> -               if (cleanup_mode == CLEANUP_ALL)
>>> +               if (cleanup_mode == CLEANUP_ALL && !verbose)
>>>                        fprintf(fp,
>>>                                " Lines starting\n"
>>>                                "# with '#' will be ignored, and an empty"
>>>                                " message aborts the commit.\n");
>> The Documentation/CodingStyle says to avoid using braces unncessary.
>> But since this is not a single line statement, like:
>> if (foo)
>>  foo = x;
>>
>> Could you wrapp this into { } ?
>
> The braces are still unncessary if the function call is spread across
> a few lines. I haven't seen anything in the source that indicates that
> we prefer braces when a braceless if/else has an associated statement
> that exceeds 1 line.

Any other suggestions to improve this patch?