Re: [RFC PATCH] disable complete ignorance of submodules for index <-> HEAD diff

[Sergey Sharybin <sergey.vfx@gmail.com>]

Hi,

Tested the patch. `git status` now shows the changes to the
submodules, which is nice :)

However, is it possible to make it so `git commit` lists submodules in
"changes to be committed" section, so you'll see what's gonna to be in
the commit while typing the commit message as well?

On Sat, Nov 23, 2013 at 7:11 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> If the value of ignore for submodules is set to "all" we would not show
> whats actually committed during status or diff. This can result in the
> user committing unexpected submodule references. Lets be nicer and always
> show whats in the index.
>
> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
> ---
> This probably needs splitting up into two patches one for the
> refactoring and one for the actual fix. It is also missing tests, but I
> would first like to know what you think about this approach.
>
>  builtin/diff.c | 43 +++++++++++++++++++++++++++----------------
>  diff.h         |  2 +-
>  submodule.c    |  6 ++++--
>  wt-status.c    |  3 +++
>  4 files changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/builtin/diff.c b/builtin/diff.c
> index adb93a9..e9a356c 100644
> --- a/builtin/diff.c
> +++ b/builtin/diff.c
> @@ -249,6 +249,21 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv
>         return run_diff_files(revs, options);
>  }
>
> +static int have_cached_option(int argc, const char **argv)
> +{
> +       int i;
> +       for (i = 1; i < argc; i++) {
> +               const char *arg = argv[i];
> +               if (!strcmp(arg, "--"))
> +                       return 0;
> +               else if (!strcmp(arg, "--cached") ||
> +                        !strcmp(arg, "--staged")) {
> +                       return 1;
> +               }
> +       }
> +       return 0;
> +}
> +
>  int cmd_diff(int argc, const char **argv, const char *prefix)
>  {
>         int i;
> @@ -259,6 +274,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
>         struct blobinfo blob[2];
>         int nongit;
>         int result = 0;
> +       int have_cached;
>
>         /*
>          * We could get N tree-ish in the rev.pending_objects list.
> @@ -305,6 +321,11 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
>
>         if (nongit)
>                 die(_("Not a git repository"));
> +
> +       have_cached = have_cached_option(argc, argv);
> +       if (have_cached)
> +               DIFF_OPT_SET(&rev.diffopt, NO_IGNORE_SUBMODULE);
> +
>         argc = setup_revisions(argc, argv, &rev, NULL);
>         if (!rev.diffopt.output_format) {
>                 rev.diffopt.output_format = DIFF_FORMAT_PATCH;
> @@ -319,22 +340,12 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
>          * Do we have --cached and not have a pending object, then
>          * default to HEAD by hand.  Eek.
>          */
> -       if (!rev.pending.nr) {
> -               int i;
> -               for (i = 1; i < argc; i++) {
> -                       const char *arg = argv[i];
> -                       if (!strcmp(arg, "--"))
> -                               break;
> -                       else if (!strcmp(arg, "--cached") ||
> -                                !strcmp(arg, "--staged")) {
> -                               add_head_to_pending(&rev);
> -                               if (!rev.pending.nr) {
> -                                       struct tree *tree;
> -                                       tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
> -                                       add_pending_object(&rev, &tree->object, "HEAD");
> -                               }
> -                               break;
> -                       }
> +       if (!rev.pending.nr && have_cached) {
> +               add_head_to_pending(&rev);
> +               if (!rev.pending.nr) {
> +                       struct tree *tree;
> +                       tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
> +                       add_pending_object(&rev, &tree->object, "HEAD");
>                 }
>         }
>
> diff --git a/diff.h b/diff.h
> index e342325..81561b3 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -64,7 +64,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
>  #define DIFF_OPT_FIND_COPIES_HARDER  (1 <<  6)
>  #define DIFF_OPT_FOLLOW_RENAMES      (1 <<  7)
>  #define DIFF_OPT_RENAME_EMPTY        (1 <<  8)
> -/* (1 <<  9) unused */
> +#define DIFF_OPT_NO_IGNORE_SUBMODULE (1 <<  9)
>  #define DIFF_OPT_HAS_CHANGES         (1 << 10)
>  #define DIFF_OPT_QUICK               (1 << 11)
>  #define DIFF_OPT_NO_INDEX            (1 << 12)
> diff --git a/submodule.c b/submodule.c
> index 1905d75..9d81712 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -301,9 +301,11 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,
>         DIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
>         DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);
>
> -       if (!strcmp(arg, "all"))
> +       if (!strcmp(arg, "all")) {
> +               if (DIFF_OPT_TST(diffopt, NO_IGNORE_SUBMODULE))
> +                       return;
>                 DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
> -       else if (!strcmp(arg, "untracked"))
> +       } else if (!strcmp(arg, "untracked"))
>                 DIFF_OPT_SET(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
>         else if (!strcmp(arg, "dirty"))
>                 DIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);
> diff --git a/wt-status.c b/wt-status.c
> index b4e44ba..34be1cc 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -462,6 +462,9 @@ static void wt_status_collect_changes_index(struct wt_status *s)
>                 handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
>         }
>
> +       /* for the index we need to disable complete ignorance of submodules */
> +       DIFF_OPT_SET(&rev.diffopt, NO_IGNORE_SUBMODULE);
> +
>         rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
>         rev.diffopt.format_callback = wt_status_collect_updated_cb;
>         rev.diffopt.format_callback_data = s;
> --
> 1.8.5.rc3.1.gcd6363f
>



-- 
With best regards, Sergey Sharybin