Merge pull request #679 from mih/bf-637

Elevate `next-status` and `iter_gitstatus()`

changelog.d/20240507_190704_michael.hanke_bf_637.md

@@ -0,0 +1,23 @@
+### 🐛 Bug Fixes
+
+- `next-status -r mono` now reports on new commits in submodules.
+  Previously this was ignored, leading to the impression of
+  clean datasets despite unsaved changes.
+  Fixes https://github.com/datalad/datalad-next/issues/645 via
+  https://github.com/datalad/datalad-next/pull/679 (by @mih)
+
+### 💫 Enhancements and new features
+
+- `next-status` and `iter_gitstatus()` have been improved to
+  report on further modifications after a file addition has been
+  originally staged.
+  Fixes https://github.com/datalad/datalad-next/issues/637 via
+  https://github.com/datalad/datalad-next/pull/679 (by @mih)
+- `next-status` result rendering has been updated to be more markedly
+  different than git-status's. Coloring is now exclusively
+  determined by the nature of a change, rather than being partially
+  similar to git-status's index-updated annotation. This reduces
+  the chance for misinterpretations, and does not create an undesirable
+  focus on the Git index (which is largely ignored by DataLad).
+  Fixes https://github.com/datalad/datalad-next/issues/640 via
+  https://github.com/datalad/datalad-next/pull/679 (by @mih)

datalad_next/commands/status.py

@@ -56,15 +56,6 @@ class StatusState(Enum):
     unknown = 'unknown'
 
 
-STATE_COLOR_MAP = {
-    StatusState.added: ac.GREEN,
-    StatusState.modified: ac.RED,
-    StatusState.deleted: ac.RED,
-    StatusState.untracked: ac.RED,
-    StatusState.unknown: ac.YELLOW,
-}
-
-
 diffstatus2resultstate_map = {
     GitDiffStatus.addition: StatusState.added,
     GitDiffStatus.copy: StatusState.added,
@@ -358,7 +349,7 @@ def custom_result_renderer(res, **kwargs):
             fill=' ' * max(0, max_len - len(state)),
             state=ac.color_word(
                 res.state.value,
-                STATE_COLOR_MAP.get(res.state)),
+                _get_result_status_render_color(res)),
             path=path,
             type_=' ({})'.format(ac.color_word(type_, ac.MAGENTA))
             if type_ else '',
@@ -371,3 +362,14 @@ def custom_result_summary_renderer(results):
         # no reports, no changes
         if len(results) == 0:
             ui.message("nothing to save, working tree clean")
+
+
+def _get_result_status_render_color(res):
+    if res.state == StatusState.deleted:
+        return ac.RED
+    elif res.state == StatusState.modified:
+        return ac.CYAN
+    elif res.state == StatusState.added:
+        return ac.GREEN
+    else:
+        return ac.BOLD

datalad_next/iter_collections/gitdiff.py

@@ -91,6 +91,10 @@ class GitDiffItem(GitTreeItem):
     """Qualifiers for modification types of container-type
     items (directories, submodules)."""
 
+    def __post_init__(self):
+        if self.status == GitDiffStatus.addition and self.gitsha is None:
+            self.add_modification_type(GitContainerModificationType.modified_content)
+
     @cached_property
     def prev_path(self) -> PurePosixPath | None:
         """Returns the item ``prev_name`` as a ``PurePosixPath``

datalad_next/iter_collections/gitstatus.py

@@ -316,12 +316,14 @@ def _yield_hierarchy_items(
         # TODO do we need to adjust the eval mode here for the diff recmodes?
         eval_submodule_state=eval_submodule_state,
     ):
-        # we get to see any submodule item passing through here, and can simply
-        # call this function again for a subpath
+        # there is nothing else to do for any non-submodule item
         if item.gittype != GitTreeItemType.submodule:
             yield item
             continue
 
+        # we get to see any submodule item passing through here, and can simply
+        # call this function again for a subpath
+
         # submodule recursion
         # the .path of a GitTreeItem is always POSIX
         sm_path = path / item.path
@@ -342,6 +344,14 @@ def _yield_hierarchy_items(
             # repository, hence no submodule item is emitted
             sm_head = item.gitsha or item.prev_gitsha
 
+            if GitContainerModificationType.new_commits in item.modification_types:
+                # this is a submodule that has new commits compared to
+                # its state in the parent dataset. We need to yield this
+                # item, even if nothing else is modified, because otherwise
+                # this (unsafed) changed would go unnoticed
+                # https://github.com/datalad/datalad-next/issues/645
+                yield item
+
         for i in _yield_hierarchy_items(
             head=sm_head,
             path=sm_path,

datalad_next/iter_collections/tests/test_itergitstatus.py

@@ -69,7 +69,12 @@ def test_status_homogeneity(modified_dataset):
 
         # anything modified is labeled as such
         assert all(
+            # either directly
             i.status == GitDiffStatus.modification
+            # or as an addition with a modification on top
+            or (i.status == GitDiffStatus.addition
+                and GitContainerModificationType.modified_content
+                    in i.modification_types)
             for i in st.values()
             if 'm' in i.path.name.split('_')[1]
         )
@@ -213,11 +218,15 @@ def test_status_monorec(modified_dataset):
     # the submodules
     _assert_testcases(
         st,
-        # repository and recursive test cases, minus any direct submodule
-        # items
+        # repository and recursive test cases
         [c for c in chain(test_cases_repository_recursion,
                           test_cases_submodule_recursion)
-         if not c['name'].split('/')[-1].split('_')[0] == 'sm'])
+         # minus any submodule that have no new commits
+         # (this only thing that is not attributable to individual
+         # content changes)
+         if not c['name'].split('/')[-1] in (
+             'sm_m', 'sm_mu', 'sm_u',
+         )])
 
 
 def test_status_gitinit(tmp_path):

datalad_next/tests/fixtures.py

@@ -309,6 +309,7 @@ def modified_dataset(tmp_path_factory):
           (use "git restore --staged <file>..." to unstage)
                 new file:   dir_m/file_a
                 new file:   file_a
+                new file:   file_am
 
         Changes not staged for commit:
           (use "git add/rm <file>..." to update what will be committed)
@@ -324,6 +325,7 @@ def modified_dataset(tmp_path_factory):
                 modified:   dir_sm/sm_nm (new commits, modified content)
                 modified:   dir_sm/sm_nmu (new commits, modified content, untracked content)
                 modified:   dir_sm/sm_u (untracked content)
+                modified:   file_am
                 deleted:    file_d
                 modified:   file_m
 
@@ -334,6 +336,7 @@ def modified_dataset(tmp_path_factory):
                 dir_u/file_u
                 file_u
 
+
     Suffix indicates the ought-to state (multiple possible):
 
     a - added
@@ -406,6 +409,11 @@ def modified_dataset(tmp_path_factory):
         pobj = obj.pathobj if isinstance(obj, Dataset) else obj
         (pobj / 'file_a').write_text('added')
         assert call_git_success(['add', 'file_a'], cwd=pobj)
+    # added and then modified file
+    file_am_obj = ds.pathobj / 'file_am'
+    file_am_obj.write_text('added')
+    assert call_git_success(['add', 'file_am'], cwd=ds.pathobj)
+    file_am_obj.write_text('modified')
 
     # record git-status output as a reference
     status_start = call_git_lines(['status'], cwd=ds.pathobj)