From: Phillip Wood <phillip.wood123@gmail.com>
Date: Thu, 15 Oct 2020 13:39:47 GMT
Subject: Re: [Outreachy] Introduction
Message-ID: <95c90e7a-8f4c-32b9-2b0e-9efce2acfb8e@gmail.com>
In-Reply-To: <CAHjREB7xhBWHUCJ+T78i6dDGQr055sMAaYzG4gaQSr0T88cseg@mail.gmail.com>

Hi Sangeeta

On 15/10/2020 11:18, Sangeeta NB wrote:
> Hey,
> 
>> I've got a fixup which I'll post after this which gets rid of the global
>> flag and instead uses a flag in struct diff_options.
> 
> Thanks for the patch, I was thinking about something on similar lines
> but couldn't come up with anything.

As we store the config options in default_diff_options and then copy 
them across at the beginning of repo_setup_diff() we can use a flag in 
struct diff_options which is set by handle_ignore_submodule_arg() to 
tell if we need to initialize opts->flags.ignore_untracked_in_submodules 
in repo_setup_diff()

> Also, one thing I observed that when I add a printf statement in
> wt-status.c, something like this:
> 
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -601,11 +601,11 @@ static void
> wt_status_collect_changes_worktree(struct wt_status *s)
>          rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
>          rev.diffopt.flags.dirty_submodules = 1;
>          rev.diffopt.ita_invisible_in_index = 1;
> +       printf("a printf statement\n");
>          if (!s->show_untracked_files)
> 
> then git status shows output:
> 
> sh-3.2$ git status -s -uno --ignore-submodules=none
> a printf statement
>   m submod
> 
> which is what is expected. But when I comment out the printf statement
> it again gives no output. I couldn't understand why this is taking
> place and how can a printf line modify the behavior of git status.

Are you adding the printf and then running t3600? If so then the extra 
line of output breaks a lot of tests which in turn breaks to setup for 
the test that was failing so there are uncommitted changes. 
Unfortunately it is hard to run a subset of tests in a lot the test 
scripts as there are implicit dependencies between the individual tests 
them.

>> I thinking it would be worth considering if
>> some of them should instead be changed to pass --ignore-submodules=none
>> rather than changing the expected result.
> 
> Ya, that's a good suggestion. Would look at those tests again and see
> if I can pass the --ignore-submodules=none option.

For the diff tests I think we want to test the new default and check 
that --ignore-submodules=none works. I think for the other tests we 
probably want to just add --ignore-submodules=none

I'm afraid I'm still no closer to figuring out why that test in t3600 fails

Phillip

> Thanks
>