git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Sangeeta <sangunb09@gmail.com>
To: Git List <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [Outreachy][PATCH v7] diff: do not show submodule with untracked files as "-dirty"
Date: Tue, 8 Dec 2020 19:06:57 +0530	[thread overview]
Message-ID: <CAHjREB7_BScE3zZVUZho4xi0OTdpsJTqLAbqhS0Tcd_xF6gqHA@mail.gmail.com> (raw)
In-Reply-To: <20201110083900.88161-1-sangunb09@gmail.com>

Hey Junio,

In the "What's cooking in git.git" you mentioned that this patch is
left on doc update and some adjustments in "git status".

The above patch does adds the `ignore-submodule=none` as the default
behavior for git status and also adds documentation too.
Am I missing something?

Thanks and regards,
Sangeeta

Sangeeta

On Tue, Nov 10, 2020 at 2:09 PM Sangeeta Jain <sangunb09@gmail.com> wrote:
>
> Git diff reports a submodule directory as -dirty even when there are
> only untracked files in the submodule directory. This is inconsistent
> with what `git describe --dirty` says when run in the submodule
> directory in that state.
>
> Make `--ignore-submodules=untracked` the default for `git diff` when
> there is no configuration variable or command line option, so that the
> command would not give '-dirty' suffix to a submodule whose working
> tree has untracked files, to make it consistent with `git
> describe --dirty` that is run in the submodule working tree.
>
> And also make `--ignore-submodules=none` the default for `git status`
> so that the user doesn't end up deleting a submodule that has
> uncommitted (untracked) files.
>
> Signed-off-by: Sangeeta Jain <sangunb09@gmail.com>
> ---
>  Documentation/config/diff.txt                |  2 ++
>  diff.c                                       |  3 +++
>  diff.h                                       |  1 +
>  submodule.c                                  |  1 +
>  t/t3701-add-interactive.sh                   |  6 ++++++
>  t/t4027-diff-submodule.sh                    | 12 ++++++++++--
>  t/t4041-diff-submodule-option.sh             | 16 ++++++++--------
>  t/t4060-diff-submodule-option-diff-format.sh | 16 ++++++++--------
>  wt-status.c                                  |  4 +++-
>  9 files changed, 42 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
> index c3ae136eba..2d3331f55c 100644
> --- a/Documentation/config/diff.txt
> +++ b/Documentation/config/diff.txt
> @@ -85,6 +85,8 @@ diff.ignoreSubmodules::
>         and 'git status' when `status.submoduleSummary` is set unless it is
>         overridden by using the --ignore-submodules command-line option.
>         The 'git submodule' commands are not affected by this setting.
> +       By default this is set to untracked so that any untracked
> +       submodules are ignored.
>
>  diff.mnemonicPrefix::
>         If set, 'git diff' uses a prefix pair that is different from the
> diff --git a/diff.c b/diff.c
> index 2bb2f8f57e..5a80695da8 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4585,6 +4585,9 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
>                 DIFF_XDL_SET(options, INDENT_HEURISTIC);
>
>         options->orderfile = diff_order_file_cfg;
> +
> +       if (!options->flags.ignore_submodule_set)
> +               options->flags.ignore_untracked_in_submodules = 1;
>
>         if (diff_no_prefix) {
>                 options->a_prefix = options->b_prefix = "";
> diff --git a/diff.h b/diff.h
> index 11de52e9e9..1e18e6b1c3 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -178,6 +178,7 @@ struct diff_flags {
>         unsigned diff_from_contents;
>         unsigned dirty_submodules;
>         unsigned ignore_untracked_in_submodules;
> +       unsigned ignore_submodule_set;
>         unsigned ignore_dirty_submodules;
>         unsigned override_submodule_config;
>         unsigned dirstat_by_line;
> diff --git a/submodule.c b/submodule.c
> index b3bb59f066..8f6227c993 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -420,6 +420,7 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy
>  void handle_ignore_submodules_arg(struct diff_options *diffopt,
>                                   const char *arg)
>  {
> +       diffopt->flags.ignore_submodule_set = 1;
>         diffopt->flags.ignore_submodules = 0;
>         diffopt->flags.ignore_untracked_in_submodules = 0;
>         diffopt->flags.ignore_dirty_submodules = 0;
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index ca04fac417..9a2489cde0 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -765,6 +765,12 @@ test_expect_success 'setup different kinds of dirty submodules' '
>         cat >expected <<-\EOF &&
>         dirty-both-ways
>         dirty-head
> +       EOF
> +       test_cmp expected actual &&
> +       git -C for-submodules diff-files --name-only --ignore-submodules=none >actual &&
> +       cat >expected <<-\EOF &&
> +       dirty-both-ways
> +       dirty-head
>         dirty-otherwise
>         EOF
>         test_cmp expected actual &&
> diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
> index d7145ccca4..894a11b224 100755
> --- a/t/t4027-diff-submodule.sh
> +++ b/t/t4027-diff-submodule.sh
> @@ -93,6 +93,14 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked)' '
>         ) &&
>         git diff HEAD >actual &&
>         sed -e "1,/^@@/d" actual >actual.body &&
> +       expect_from_to >expect.body $subtip $subprev &&
> +       test_cmp expect.body actual.body
> +'
> +
> +test_expect_success 'git diff HEAD with dirty submodule (untracked) (none ignored)' '
> +       test_config diff.ignoreSubmodules none &&
> +       git diff HEAD >actual &&
> +       sed -e "1,/^@@/d" actual >actual.body &&
>         expect_from_to >expect.body $subtip $subprev-dirty &&
>         test_cmp expect.body actual.body
>  '
> @@ -168,13 +176,13 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)'
>                 git clean -qfdx &&
>                 >cruft
>         ) &&
> -       git diff HEAD >actual &&
> +       git diff --ignore-submodules=none HEAD >actual &&
>         sed -e "1,/^@@/d" actual >actual.body &&
>         expect_from_to >expect.body $subprev $subprev-dirty &&
>         test_cmp expect.body actual.body &&
>         git diff --ignore-submodules=all HEAD >actual2 &&
>         test_must_be_empty actual2 &&
> -       git diff --ignore-submodules=untracked HEAD >actual3 &&
> +       git diff HEAD >actual3 &&
>         test_must_be_empty actual3 &&
>         git diff --ignore-submodules=dirty HEAD >actual4 &&
>         test_must_be_empty actual4
> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> index f852136585..b3a7b7acaa 100755
> --- a/t/t4041-diff-submodule-option.sh
> +++ b/t/t4041-diff-submodule-option.sh
> @@ -262,7 +262,7 @@ test_expect_success 'submodule is up to date' '
>
>  test_expect_success 'submodule contains untracked content' '
>         echo new > sm1/new-file &&
> -       git diff-index -p --submodule=log HEAD >actual &&
> +       git diff-index -p --ignore-submodules=none --submodule=log HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 contains untracked content
>         EOF
> @@ -270,7 +270,7 @@ test_expect_success 'submodule contains untracked content' '
>  '
>
>  test_expect_success 'submodule contains untracked content (untracked ignored)' '
> -       git diff-index -p --ignore-submodules=untracked --submodule=log HEAD >actual &&
> +       git diff-index -p --submodule=log HEAD >actual &&
>         test_must_be_empty actual
>  '
>
> @@ -286,7 +286,7 @@ test_expect_success 'submodule contains untracked content (all ignored)' '
>
>  test_expect_success 'submodule contains untracked and modified content' '
>         echo new > sm1/foo6 &&
> -       git diff-index -p --submodule=log HEAD >actual &&
> +       git diff-index -p --ignore-submodules=none --submodule=log HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 contains untracked content
>         Submodule sm1 contains modified content
> @@ -296,7 +296,7 @@ test_expect_success 'submodule contains untracked and modified content' '
>
>  test_expect_success 'submodule contains untracked and modified content (untracked ignored)' '
>         echo new > sm1/foo6 &&
> -       git diff-index -p --ignore-submodules=untracked --submodule=log HEAD >actual &&
> +       git diff-index -p --submodule=log HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 contains modified content
>         EOF
> @@ -337,7 +337,7 @@ test_expect_success 'submodule is modified' '
>
>  test_expect_success 'modified submodule contains untracked content' '
>         echo new > sm1/new-file &&
> -       git diff-index -p --submodule=log HEAD >actual &&
> +       git diff-index -p  --ignore-submodules=none --submodule=log HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 contains untracked content
>         Submodule sm1 $head6..$head8:
> @@ -347,7 +347,7 @@ test_expect_success 'modified submodule contains untracked content' '
>  '
>
>  test_expect_success 'modified submodule contains untracked content (untracked ignored)' '
> -       git diff-index -p --ignore-submodules=untracked --submodule=log HEAD >actual &&
> +       git diff-index -p --submodule=log HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 $head6..$head8:
>           > change
> @@ -371,7 +371,7 @@ test_expect_success 'modified submodule contains untracked content (all ignored)
>
>  test_expect_success 'modified submodule contains untracked and modified content' '
>         echo modification >> sm1/foo6 &&
> -       git diff-index -p --submodule=log HEAD >actual &&
> +       git diff-index -p --ignore-submodules=none --submodule=log HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 contains untracked content
>         Submodule sm1 contains modified content
> @@ -383,7 +383,7 @@ test_expect_success 'modified submodule contains untracked and modified content'
>
>  test_expect_success 'modified submodule contains untracked and modified content (untracked ignored)' '
>         echo modification >> sm1/foo6 &&
> -       git diff-index -p --ignore-submodules=untracked --submodule=log HEAD >actual &&
> +       git diff-index -p --submodule=log HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 contains modified content
>         Submodule sm1 $head6..$head8:
> diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh
> index fc8229c726..dc7b242697 100755
> --- a/t/t4060-diff-submodule-option-diff-format.sh
> +++ b/t/t4060-diff-submodule-option-diff-format.sh
> @@ -409,7 +409,7 @@ test_expect_success 'submodule is up to date' '
>
>  test_expect_success 'submodule contains untracked content' '
>         echo new > sm1/new-file &&
> -       git diff-index -p --submodule=diff HEAD >actual &&
> +       git diff-index -p --ignore-submodules=none --submodule=diff HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 contains untracked content
>         EOF
> @@ -417,7 +417,7 @@ test_expect_success 'submodule contains untracked content' '
>  '
>
>  test_expect_success 'submodule contains untracked content (untracked ignored)' '
> -       git diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&
> +       git diff-index -p --submodule=diff HEAD >actual &&
>         test_must_be_empty actual
>  '
>
> @@ -433,7 +433,7 @@ test_expect_success 'submodule contains untracked content (all ignored)' '
>
>  test_expect_success 'submodule contains untracked and modified content' '
>         echo new > sm1/foo6 &&
> -       git diff-index -p --submodule=diff HEAD >actual &&
> +       git diff-index -p --ignore-submodules=none --submodule=diff HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 contains untracked content
>         Submodule sm1 contains modified content
> @@ -451,7 +451,7 @@ test_expect_success 'submodule contains untracked and modified content' '
>  # NOT OK
>  test_expect_success 'submodule contains untracked and modified content (untracked ignored)' '
>         echo new > sm1/foo6 &&
> -       git diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&
> +       git diff-index -p --submodule=diff HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 contains modified content
>         diff --git a/sm1/foo6 b/sm1/foo6
> @@ -512,7 +512,7 @@ test_expect_success 'submodule is modified' '
>
>  test_expect_success 'modified submodule contains untracked content' '
>         echo new > sm1/new-file &&
> -       git diff-index -p --submodule=diff HEAD >actual &&
> +       git diff-index -p --ignore-submodules=none --submodule=diff HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 contains untracked content
>         Submodule sm1 $head7..$head8:
> @@ -528,7 +528,7 @@ test_expect_success 'modified submodule contains untracked content' '
>  '
>
>  test_expect_success 'modified submodule contains untracked content (untracked ignored)' '
> -       git diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&
> +       git diff-index -p --submodule=diff HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 $head7..$head8:
>         diff --git a/sm1/foo6 b/sm1/foo6
> @@ -564,7 +564,7 @@ test_expect_success 'modified submodule contains untracked content (all ignored)
>
>  test_expect_success 'modified submodule contains untracked and modified content' '
>         echo modification >> sm1/foo6 &&
> -       git diff-index -p --submodule=diff HEAD >actual &&
> +       git diff-index -p --ignore-submodules=none --submodule=diff HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 contains untracked content
>         Submodule sm1 contains modified content
> @@ -583,7 +583,7 @@ test_expect_success 'modified submodule contains untracked and modified content'
>
>  test_expect_success 'modified submodule contains untracked and modified content (untracked ignored)' '
>         echo modification >> sm1/foo6 &&
> -       git diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&
> +       git diff-index -p --submodule=diff HEAD >actual &&
>         cat >expected <<-EOF &&
>         Submodule sm1 contains modified content
>         Submodule sm1 $head7..$head8:
> diff --git a/wt-status.c b/wt-status.c
> index 7074bbdd53..83d418647d 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -606,7 +606,9 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
>         if (s->ignore_submodule_arg) {
>                 rev.diffopt.flags.override_submodule_config = 1;
>                 handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
> -       }
> +       } else if (!rev.diffopt.flags.ignore_submodule_set &&
> +                       s->show_untracked_files != SHOW_NO_UNTRACKED_FILES)
> +               handle_ignore_submodules_arg(&rev.diffopt, "none");
>         rev.diffopt.format_callback = wt_status_collect_changed_cb;
>         rev.diffopt.format_callback_data = s;
>         rev.diffopt.detect_rename = s->detect_rename >= 0 ? s->detect_rename : rev.diffopt.detect_rename;
> --
> 2.21.1 (Apple Git-122.3)
>

  parent reply	other threads:[~2020-12-08 13:41 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-15 17:08 [PATCH] diff: do not show submodule with untracked files as "-dirty" Sangeeta via GitGitGadget
2020-10-20 13:38 ` [OUTREACHY][PATCH] " Phillip Wood
2020-10-20 18:10   ` Sangeeta NB
2020-10-21 11:28     ` Phillip Wood
2020-10-21 13:10 ` [Outreachy] [PATCH v2] " Sangeeta Jain
2020-10-21 17:43   ` Eric Sunshine
2020-10-21 19:40     ` Sangeeta NB
2020-10-21 23:04       ` Eric Sunshine
2020-10-22 11:22 ` [Outreachy] [PATCH v3] " Sangeeta Jain
2020-10-22 18:07   ` Junio C Hamano
2020-10-23  5:23     ` Sangeeta NB
2020-10-23 15:19       ` Junio C Hamano
2020-10-23 18:17         ` Sangeeta NB
2020-10-23 18:55           ` Junio C Hamano
2020-10-23 19:08             ` Sangeeta NB
2020-10-23 11:17 ` [PATCH v4] " Sangeeta Jain
2020-10-23 15:56   ` Junio C Hamano
2020-10-23 18:32     ` Sangeeta NB
2020-10-23 20:22       ` Junio C Hamano
2020-10-23 11:18 ` [Outreachy] " Sangeeta Jain
2020-10-23 21:28   ` Junio C Hamano
2020-10-25 10:23     ` Sangeeta NB
2020-10-26 17:36       ` Junio C Hamano
2020-10-23 19:29 ` [Outreachy] [PATCH v5] " Sangeeta Jain
2020-10-26 17:57 ` [Outreachy][PATCH v6] " Sangeeta Jain
2020-11-03 10:46   ` Sangeeta
2020-11-03 17:55     ` Junio C Hamano
2020-11-07 10:47       ` Sangeeta
2020-12-08 21:02         ` Junio C Hamano
2020-11-07 11:10   ` Đoàn Trần Công Danh
2020-11-09 15:19     ` Sangeeta
2020-11-09 17:01       ` Junio C Hamano
2020-11-10  8:39 ` [Outreachy][PATCH v7] " Sangeeta Jain
2020-11-10 17:09   ` Đoàn Trần Công Danh
2020-12-08 13:36   ` Sangeeta [this message]
2020-12-08 22:26     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHjREB7_BScE3zZVUZho4xi0OTdpsJTqLAbqhS0Tcd_xF6gqHA@mail.gmail.com \
    --to=sangunb09@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).