Re: [RFC PATCH] short status: improve reporting for submodule changes

[Stefan Beller]

On Wed, Mar 15, 2017 at 6:31 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> While we already have a porcelain2 layer for git-status, that is accurate
>> for submodules, users still like the way they are are used to of
>> 'status -s'.
>>
>> As a submodule has more state than a file potentially, we'll look at all
>> cases:
>>
>>-- new submodule commits
>>  /  - modified files
>>  | /   -- untracked files
>>  | |  /
>>  | | |   current / proposed reporting
>>  0 0 0 "  " "  "
>>  0 0 1 " M" " ?"
>>  0 1 0 " M" " m"
>>  0 1 1 " M" " m"
>>  1 0 0 " M" " M"
>>  1 0 1 " M" " M"
>>  1 1 0 " M" " M"
>>  1 1 1 " M" " M"
>
> You are essentialy saying that there are three levels, 1. with
> commit level difference, 2. the same commit with local mods, 3. no
> mods but with crufts, and instead of wasting 8 letters to express
> all combinations, the highest level is reported, right?  That sounds
> OK to me.  I am not sure if "?" is a good letter to use (doesn't it
> usually mean it is an untracked cruft?), though.

ok. it helped me, though, to picture all possibilities to come up with
what I consider best for each case. Yes in the end it can be described
as 'report highest bit'.

>
> Does the commit level difference really mean "has new commits"?  It
> probably is not new problem but an old mistake inherited from the
> current code, but I suspect that you're just comparing the commit
> bound in the index of the superproject and the HEAD commit in the
> submodule, in which case "newness" does not matter a bit---"is it
> the same or different?" is the question you are asking, I would
> think.

yes, I agree. That is the actual question asked.


>
>

Re: [RFC PATCH] short status: improve reporting for submodule changes

[Junio C Hamano]

Stefan Beller  writes:

> While we already have a porcelain2 layer for git-status, that is accurate
> for submodules, users still like the way they are are used to of
> 'status -s'.
>
> As a submodule has more state than a file potentially, we'll look at all
> cases:
>
>-- new submodule commits
>  /  - modified files
>  | /   -- untracked files
>  | |  /
>  | | |   current / proposed reporting
>  0 0 0 "  " "  "
>  0 0 1 " M" " ?"
>  0 1 0 " M" " m"
>  0 1 1 " M" " m"
>  1 0 0 " M" " M"
>  1 0 1 " M" " M"
>  1 1 0 " M" " M"
>  1 1 1 " M" " M"

You are essentialy saying that there are three levels, 1. with
commit level difference, 2. the same commit with local mods, 3. no
mods but with crufts, and instead of wasting 8 letters to express
all combinations, the highest level is reported, right?  That sounds
OK to me.  I am not sure if "?" is a good letter to use (doesn't it
usually mean it is an untracked cruft?), though.

Does the commit level difference really mean "has new commits"?  It
probably is not new problem but an old mistake inherited from the
current code, but I suspect that you're just comparing the commit
bound in the index of the superproject and the HEAD commit in the
submodule, in which case "newness" does not matter a bit---"is it
the same or different?" is the question you are asking, I would
think.

[RFC PATCH] short status: improve reporting for submodule changes

[Stefan Beller]

While we already have a porcelain2 layer for git-status, that is accurate
for submodules, users still like the way they are are used to of
'status -s'.

As a submodule has more state than a file potentially, we'll look at all
cases:

   -- new submodule commits
 /  - modified files
 | /   -- untracked files
 | |  /
 | | |   current / proposed reporting
 0 0 0 "  " "  "
 0 0 1 " M" " ?"
 0 1 0 " M" " m"
 0 1 1 " M" " m"
 1 0 0 " M" " M"
 1 0 1 " M" " M"
 1 1 0 " M" " M"
 1 1 1 " M" " M"

So when there is no new commits in the submodule, we'd want to tell
what actually happend to the submodule. The lower case 'm' indicates
to the user that there is need to give a --recurse-submodules option
for e.g. adding or committing these changes. The " ?" also differentiates
an untracked file in the submodule from a regular untracked file.

While making these changes, we need to make sure to not break porcelain
level 1, which uses the same code as the short printing function.

Signed-off-by: Stefan Beller 
---
 Documentation/git-status.txt |  8 
 wt-status.c  | 15 +--
 wt-status.h  |  1 +
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index ba873657cf..26c8d832cd 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -181,6 +181,12 @@ in which case `XY` are `!!`.
 !   !ignored
 -
 
+Submodules have more state to it, such that it reports instead:
+   Mthe submodule has new commits
+   mthe submodule has modified content
+   ?the submodule has untracked files
+
+
 If -b is used the short-format status is preceded by a line
 
 ## branchname tracking info
@@ -210,6 +216,8 @@ field from the first filename).  Third, filenames 
containing special
 characters are not specially formatted; no quoting or
 backslash-escaping is performed.
 
+Any submodule changes are reported as modified `M` instead of `m` or single 
`?`.
+
 Porcelain Format Version 2
 ~~
 
diff --git a/wt-status.c b/wt-status.c
index a52d342695..080d97f272 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1664,9 +1664,19 @@ static void wt_shortstatus_status(struct 
string_list_item *it,
color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%c", 
d->index_status);
else
putchar(' ');
-   if (d->worktree_status)
+   if (d->worktree_status) {
+   if (!s->submodule_porcelain1 &&
+   (d->dirty_submodule || d->new_submodule_commits)) {
+   /* It is a submodule, and we're not in plumbing mode. */
+   if (d->new_submodule_commits)
+   d->worktree_status = 'M';
+   else if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
+   d->worktree_status = 'm';
+   else if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
+   d->worktree_status = '?';
+   }
color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%c", 
d->worktree_status);
-   else
+   } else
putchar(' ');
putchar(' ');
if (s->null_termination) {
@@ -1811,6 +1821,7 @@ static void wt_porcelain_print(struct wt_status *s)
s->relative_paths = 0;
s->prefix = NULL;
s->no_gettext = 1;
+   s->submodule_porcelain1 = 1;
wt_shortstatus_print(s);
 }
 
diff --git a/wt-status.h b/wt-status.h
index 54fec77032..620e4d2ae4 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -70,6 +70,7 @@ struct wt_status {
int display_comment_prefix;
int relative_paths;
int submodule_summary;
+   int submodule_porcelain1;
int show_ignored_files;
enum untracked_status_type show_untracked_files;
const char *ignore_submodule_arg;
-- 
2.12.0.269.g1a05a5734c.dirty