Re: [PATCH/FINAL] status: contextually notify user about an initial commit

[Kaartic Sivaraam]

On Wed, 2017-06-21 at 19:10 -0700, Junio C Hamano wrote:
> You can check by downloading what you sent out (I showed you how in
> the other thread).
> 
> It seems that there are funny non-breaking spaces in the additional
> text below "---" but before the diffstat, but they are not part of
> patch text anyway.
> 
> You seem to havespelled "Ævar" differently (perhaps difference
> between NFD vs NFC ???) which seems to confuse mailinfo, but I don't
> have time to dig into it myself (it is quicker for me to edit your
> Signed-off-by: while queuing).
> 
> Ah, wait... it's not like Ævar is relaying your work; it's more like
> some code / tests were given by him to you to incorporate into this,
> so I suspect that two S-o-b: from you two should be in the reverse
> order.  I'll swap them while queuing.
> 
> Thanks.
So, from this I conclude the following,

"Never user e-mail clients to send patches. They seem to be bringing in
all sorts of surprising changes."

This is just a consequence of not following the advice in
Documentation/SubmittingPatches. This won't repeat in future. 
(expecting, I could hold to that statement :))

-- 
Regards,
Kaartic Sivaraam 

Re: [PATCH/FINAL] status: contextually notify user about an initial commit

[Junio C Hamano]

Kaartic Sivaraam  writes:

> The existing message, "Initial commit", makes sense for the commit template
> notifying users that it's their initial commit, but is confusing when
> merely checking the status of a fresh repository (or orphan branch)
> without having any commits yet.
>
> Change the output of "status" to say "No commits yet" when "git
> status" is run on a fresh repo (or orphan branch), while retaining the
> current "Initial commit" message displayed in the template that's
> displayed in the editor when the initial commit is being authored.
>
> Correspondingly change the output of "short status" to "No commits yet
> on " when "git status -sb" is run on a fresh repo (or orphan branch).
>
> A few alternatives considered were,
>
>  * Waiting for initial commit
>  * Your current branch does not have any commits
>  * Current branch waiting for initial commit
>
> The most succint one among the alternatives was chosen.
>
> Helped-by: Junio C Hamano 
> Signed-off-by: Kaartic Sivaraam 
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>  Change(s): 
>  * Added the corresponding change to short status that was
>missing in the previous patch.
>  * Fixed broken tests
>
>
>  Note: This is my last attempt to try sending patches using my
>  email client in case this one is also line wrapped, please let
>  me so that I could avoid sending through my email-client altogether.
>
>  I'm trying this because I forgot to turn off line wrapping in my 
>  email-client while sending previous patches. Hope it works!

You can check by downloading what you sent out (I showed you how in
the other thread).

It seems that there are funny non-breaking spaces in the additional
text below "---" but before the diffstat, but they are not part of
patch text anyway.

You seem to havespelled "Ævar" differently (perhaps difference
between NFD vs NFC ???) which seems to confuse mailinfo, but I don't
have time to dig into it myself (it is quicker for me to edit your
Signed-off-by: while queuing).

Ah, wait... it's not like Ævar is relaying your work; it's more like
some code / tests were given by him to you to incorporate into this,
so I suspect that two S-o-b: from you two should be in the reverse
order.  I'll swap them while queuing.

Thanks.

Re: [PATCH/FINAL] status: contextually notify user about an initial commit

[Junio C Hamano]

Kaartic Sivaraam  writes:

> On Wed, 2017-06-21 at 16:52 +0200, Ævar Arnfjörð Bjarmason wrote:
>> No, this is a bug in your patch, the test suite should pass under
>> poison.
>> 
>> The issue is that you changed the test code I gave you (to also add
>> more
>> tests, yay) along the way to do:
>> 
>> test_must_fail test_i18ngrep ...
>> 
>> Instead of the correct form:
>> 
>> test_i18ngrep ! ...
>> 
> Yeah, I did it after reading info about 'test_must_fail' in 't/README'.
> I thought it should be used for tests that fail which seemed to be a
> misinterpretation. Thanks for pointing it out. Fixed it!

Actually, test_must_fail _is_ to be used to expect that the command
being tested to fail.  The issue is with i18ngrep, where it is
rendered to a glorified "true" under the poison build.  By writing 

test_must_fail test_i18ngrep ...

you are saying that you expect test_i18ngrep to fail, but the point
of i18ngrep is not to fail under the poison build, so...

[PATCH/FINAL] status: contextually notify user about an initial commit

[Kaartic Sivaraam]

The existing message, "Initial commit", makes sense for the commit template
notifying users that it's their initial commit, but is confusing when
merely checking the status of a fresh repository (or orphan branch)
without having any commits yet.

Change the output of "status" to say "No commits yet" when "git
status" is run on a fresh repo (or orphan branch), while retaining the
current "Initial commit" message displayed in the template that's
displayed in the editor when the initial commit is being authored.

Correspondingly change the output of "short status" to "No commits yet
on " when "git status -sb" is run on a fresh repo (or orphan branch).

A few alternatives considered were,

 * Waiting for initial commit
 * Your current branch does not have any commits
 * Current branch waiting for initial commit

The most succint one among the alternatives was chosen.

Helped-by: Junio C Hamano 
Signed-off-by: Kaartic Sivaraam 
Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Change(s): 
 * Added the corresponding change to short status that was
   missing in the previous patch.
 * Fixed broken tests


 Note: This is my last attempt to try sending patches using my
 email client in case this one is also line wrapped, please let
 me so that I could avoid sending through my email-client altogether.

 I'm trying this because I forgot to turn off line wrapping in my 
 email-client while sending previous patches. Hope it works!

 builtin/commit.c  |  1 +
 t/t7501-commit.sh |  2 +-
 t/t7508-status.sh | 30 ++
 wt-status.c   |  7 +--
 wt-status.h   |  1 +
 5 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 8d1cac062..3d614a2ac 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1648,6 +1648,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
usage_with_options(builtin_commit_usage, 
builtin_commit_options);
 
status_init_config(, git_commit_config);
+   s.commit_template = 1;
status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
s.colopts = 0;
 
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 0b6da7ae1..fa61b1a4e 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -18,7 +18,7 @@ test_expect_success 'initial status' '
echo bongo bongo >file &&
git add file &&
git status >actual &&
-   test_i18ngrep "Initial commit" actual
+   test_i18ngrep "No commits yet" actual
 '
 
 test_expect_success 'fail initial amend' '
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 5edcc6edf..db709048c 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1499,4 +1499,34 @@ test_expect_success 'git commit -m will commit a staged 
but ignored submodule' '
git config -f .gitmodules  --remove-section submodule.subname
 '
 
+test_expect_success '"No commits yet" should be noted in status output' '
+   git checkout --orphan empty-branch-1 &&
+   git status >output &&
+   test_i18ngrep "No commits yet" output
+'
+
+test_expect_success '"No commits yet" should not be noted in status output' '
+   git checkout --orphan empty-branch-2 &&
+   test_commit test-commit-1 &&
+   git status >output &&
+   test_i18ngrep ! "No commits yet" output
+'
+
+test_expect_success '"Initial commit" should be noted in commit template' '
+   git checkout --orphan empty-branch-3 &&
+   touch to_be_committed_1 &&
+   git add to_be_committed_1 &&
+   git commit --dry-run >output &&
+   test_i18ngrep "Initial commit" output
+'
+
+test_expect_success '"Initial commit" should not be noted in commit template' '
+   git checkout --orphan empty-branch-4 &&
+   test_commit test-commit-2 &&
+   touch to_be_committed_2 &&
+   git add to_be_committed_2 &&
+   git commit --dry-run >output &&
+   test_i18ngrep ! "Initial commit" output
+'
+
 test_done
diff --git a/wt-status.c b/wt-status.c
index 068de38b5..c711ef86e 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1578,7 +1578,10 @@ static void wt_longstatus_print(struct wt_status *s)
 
if (s->is_initial) {
status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
-   status_printf_ln(s, color(WT_STATUS_HEADER, s), _("Initial 
commit"));
+   status_printf_ln(s, color(WT_STATUS_HEADER, s),
+s->commit_template
+? _("Initial commit")
+: _("No commits yet"));
status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
}
 
@@ -1748,7 +1751,7 @@ static void wt_shortstatus_print_tracking(struct 
wt_status *s)
 #define LABEL(string) (s->no_gettext ? (string) : _(string))
 
if (s->is_initial)
-   color_fprintf(s->fp, header_color, LABEL(N_("Initial commit on 
")));
+   

Re: [PATCH/FINAL] status: contextually notify user about an initial commit

[Kaartic Sivaraam]

On Wed, 2017-06-21 at 16:52 +0200, Ævar Arnfjörð Bjarmason wrote:
> No, this is a bug in your patch, the test suite should pass under
> poison.
> 
> The issue is that you changed the test code I gave you (to also add
> more
> tests, yay) along the way to do:
> 
> test_must_fail test_i18ngrep ...
> 
> Instead of the correct form:
> 
> test_i18ngrep ! ...
> 
Yeah, I did it after reading info about 'test_must_fail' in 't/README'.
I thought it should be used for tests that fail which seemed to be a
misinterpretation. Thanks for pointing it out. Fixed it!

-- 
Regards,
Kaartic Sivaraam 

Re: [PATCH/FINAL] status: contextually notify user about an initial commit

[Ævar Arnfjörð Bjarmason]

On Wed, Jun 21 2017, Kaartic Sivaraam jotted:

> On Wed, 2017-06-21 at 08:07 +0530, Kaartic Sivaraam wrote:
>> The existing message, "Initial commit", makes sense for the commit
>> template
>> notifying users that it's their initial commit, but is confusing when
>> merely checking the status of a fresh repository (or orphan branch)
>> without having any commits yet.
>>
>> Change the output of "status" to say "No commits yet" when "git
>> status" is run on a fresh repo (or orphan branch), while retaining
>> the
>> current "Initial commit" message displayed in the template that's
>> displayed in the editor when the initial commit is being authored.
>>
>> A few alternatives considered were,
>>
>> * Waiting for initial commit
>> * Your current branch does not have any commits
>> * Current branch waiting for initial commit
>>
>> The most succint one among the alternatives was chosen.
>>
>> Helped-by: Junio C Hamano 
>> Signed-off-by: Kaartic Sivaraam 
>> Signed-off-by: Ævar Arnfjörð Bjarmason 
>> ---
>>
>> The 'FINAL' part in the subject is just my opinion about
>> this patch
>>
> Just for the note, the tests passed locally and all travis-ci builds
> jobs succeeded except for the one in which the 'GITTEXT_POISON'
> environment variable is enabled. I guess that isn't an issue, from what
> I came to know while digging about it.

No, this is a bug in your patch, the test suite should pass under
poison.

The issue is that you changed the test code I gave you (to also add more
tests, yay) along the way to do:

test_must_fail test_i18ngrep ...

Instead of the correct form:

test_i18ngrep ! ...

This fixup for your patch makes it work again:

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index e0d2c9e581..b3743ff0a8 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1618,7 +1618,7 @@ test_expect_success '"No commits yet" should not be 
noted in status output' '
git checkout --orphan empty-branch-2 &&
test_commit test-commit-1 &&
git status >output &&
-   test_must_fail test_i18ngrep "No commits yet" output
+   test_i18ngrep ! "No commits yet" output
 '
 
 test_expect_success '"Initial commit" should be noted in commit template' '
@@ -1635,7 +1635,7 @@ test_expect_success '"Initial commit" should not be 
noted in commit template' '
touch to_be_committed_2 &&
git add to_be_committed_2 &&
git commit --dry-run >output &&
-   test_must_fail test_i18ngrep "Initial commit" output
+   test_i18ngrep ! "Initial commit" output
 '
 
 test_done

Re: [PATCH/FINAL] status: contextually notify user about an initial commit

[Kaartic Sivaraam]

On Wed, 2017-06-21 at 08:07 +0530, Kaartic Sivaraam wrote:
> The existing message, "Initial commit", makes sense for the commit
> template
> notifying users that it's their initial commit, but is confusing when
> merely checking the status of a fresh repository (or orphan branch)
> without having any commits yet.
> 
> Change the output of "status" to say "No commits yet" when "git
> status" is run on a fresh repo (or orphan branch), while retaining
> the
> current "Initial commit" message displayed in the template that's
> displayed in the editor when the initial commit is being authored.
> 
> A few alternatives considered were,
> 
>  * Waiting for initial commit
>  * Your current branch does not have any commits
>  * Current branch waiting for initial commit
> 
> The most succint one among the alternatives was chosen.
> 
> Helped-by: Junio C Hamano 
> Signed-off-by: Kaartic Sivaraam 
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
> 
>  The 'FINAL' part in the subject is just my opinion about 
>  this patch
> 
Just for the note, the tests passed locally and all travis-ci builds
jobs succeeded except for the one in which the 'GITTEXT_POISON'
environment variable is enabled. I guess that isn't an issue, from what
I came to know while digging about it.

-- 
Regards,
Kaartic Sivaraam 

[PATCH/FINAL] status: contextually notify user about an initial commit

[Kaartic Sivaraam]

The existing message, "Initial commit", makes sense for the commit template
notifying users that it's their initial commit, but is confusing when
merely checking the status of a fresh repository (or orphan branch)
without having any commits yet.

Change the output of "status" to say "No commits yet" when "git
status" is run on a fresh repo (or orphan branch), while retaining the
current "Initial commit" message displayed in the template that's
displayed in the editor when the initial commit is being authored.

A few alternatives considered were,

 * Waiting for initial commit
 * Your current branch does not have any commits
 * Current branch waiting for initial commit

The most succint one among the alternatives was chosen.

Helped-by: Junio C Hamano 
Signed-off-by: Kaartic Sivaraam 
Signed-off-by: Ævar Arnfjörð Bjarmason 
---

 The 'FINAL' part in the subject is just my opinion about 
 this patch

 builtin/commit.c  |  1 +
 t/t7501-commit.sh |  2 +-
 t/t7508-status.sh | 30 ++
 wt-status.c   |  5 -
 wt-status.h   |  1 +
 5 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 8d1cac062..3d614a2ac 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1648,6 +1648,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
usage_with_options(builtin_commit_usage, 
builtin_commit_options);
 
status_init_config(, git_commit_config);
+   s.commit_template = 1;
status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
s.colopts = 0;
 
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 0b6da7ae1..fa61b1a4e 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -18,7 +18,7 @@ test_expect_success 'initial status' '
echo bongo bongo >file &&
git add file &&
git status >actual &&
-   test_i18ngrep "Initial commit" actual
+   test_i18ngrep "No commits yet" actual
 '
 
 test_expect_success 'fail initial amend' '
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 5edcc6edf..0ffa585e2 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1499,4 +1499,34 @@ test_expect_success 'git commit -m will commit a staged 
but ignored submodule' '
git config -f .gitmodules  --remove-section submodule.subname
 '
 
+test_expect_success '"No commits yet" should be noted in status output' '
+   git checkout --orphan empty-branch-1 &&
+   git status >output &&
+   test_i18ngrep "No commits yet" output
+'
+
+test_expect_success '"No commits yet" should not be noted in status output' '
+   git checkout --orphan empty-branch-2 &&
+   test_commit test-commit-1 &&
+   git status >output &&
+   test_must_fail test_i18ngrep "No commits yet" output
+'
+
+test_expect_success '"Initial commit" should be noted in commit template' '
+   git checkout --orphan empty-branch-3 &&
+   touch to_be_committed_1 &&
+   git add to_be_committed_1 &&
+   git commit --dry-run >output &&
+   test_i18ngrep "Initial commit" output
+'
+
+test_expect_success '"Initial commit" should not be noted in commit template' '
+   git checkout --orphan empty-branch-4 &&
+   test_commit test-commit-2 &&
+   touch to_be_committed_2 &&
+   git add to_be_committed_2 &&
+   git commit --dry-run >output &&
+   test_must_fail test_i18ngrep "Initial commit" output
+'
+
 test_done
diff --git a/wt-status.c b/wt-status.c
index 068de38b5..a2e294bb2 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1578,7 +1578,10 @@ static void wt_longstatus_print(struct wt_status *s)
 
if (s->is_initial) {
status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
-   status_printf_ln(s, color(WT_STATUS_HEADER, s), _("Initial 
commit"));
+   status_printf_ln(s, color(WT_STATUS_HEADER, s),
+s->commit_template
+? _("Initial commit")
+: _("No commits yet"));
status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
}
 
diff --git a/wt-status.h b/wt-status.h
index 8a3864783..2389f0839 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -76,6 +76,7 @@ struct wt_status {
char color_palette[WT_STATUS_MAXSLOT][COLOR_MAXLEN];
unsigned colopts;
int null_termination;
+   int commit_template;
int show_branch;
int hints;
 
-- 
2.11.0