Re: [PATCH v1 1/1] dir: teach status to show ignored directories

From: Stefan Beller <sbeller@google.com>
Date: Aug 10, 2017, 20:03 UTC
Message-ID: <CAGZ79kb8cWR1P=1FSkfZ72Ai63T0WapwtCCN3gwqrWE_=MzOLg@mail.gmail.com>
In-Reply-To: <20170810184936.239542-2-jamill@microsoft.com>

On Thu, Aug 10, 2017 at 11:49 AM, Jameson Miller <jameson.miller81@gmail.com> wrote:

Welcome to the Git mailing list. :)

Show 6 quoted lines

> Teach Git to optionally show ignored directories when showing all
> untracked files. The git status command exposes the options to report
> ignored and/or untracked files. However, when reporting all untracked
> files (--untracked-files=all), all individual ignored files are reported
> as well. It is not currently possible to get the reporting behavior of
> the --ignored flag, while also reporting all untracked files.

Trying to understand this based off the documentation for --untracked=all and --ignored, I realize that the documentation for --ignored seems to be lacking as I do not understand what the --ignored behavior is in combination with --untracked=[all, normal, no]

> This
> change exposes a flag to report all untracked files while not showing
> individual files in ignored directories.

By the description up to here, it sounds as if you want to introduce mode for --untracked, e.g. "normal-adjusted-for-ignored" (it's a bad suggestion)? However the patch seems to add an orthogonal flag, such that

  status --no-ignored --untracked=no --show-ignored-directory

would also be possible. What is a reasonable expectation for the output of such?

> Motivation:
> Our application (Visual Studio) needs all untracked files listed
> individually, but does not need all ignored files listed individually.

For parsing output, I would strongly recommend --porcelain[=2], but that is a tangent.

Show 16 quoted lines

> Reporting all ignored files can affect the time it takes for status
> to run. For a representative repository, here are some measurements
> showing a large perf improvement for this scenario:
>
> | Command | Reported ignored entries | Time (s) |
> | ------- | ------------------------ | -------- |
> | 1       | 0                        | 1.3      |
> | 2       | 1024                     | 4.2      |
> | 3       | 174904                   | 7.5      |
> | 4       | 1046                     | 1.6      |
>
> Commands:
>  1) status
>  2) status --ignored
>  3) status --ignored --untracked-files=all
>  4) status --ignored --untracked-files=all --show-ignored-directory

(2) is --untracked-files=normal I'd presume, such that this flag can be understood as a tweak to "normal" based on the similar size between 2 and 4? (The timing improvement from 2 to 4 is huge though).

> This changes exposes a --show-ignored-directory flag to the git status

s/changes/change/

Show 12 quoted lines

> command. This flag is utilized when running git status with the
> --ignored and --untracked-files options to not list ignored individual
> ignored files contained in directories that match an ignore pattern.
>
> Part of the perf improvement comes from the tweak to
> read_directory_recursive to stop scanning the file system after it
> encounters the first file. When a directory is ignored, all it needs to
> determine is if the directory is empty or not. The logic currently keeps
> scanning the file system until it finds an untracked file. However, as
> the directory is ignored, all the contained contents are also marked
> excluded. For ignored directories that contain a large number of files,
> this can take some time.

I think it is possible to ignore a directory and still track files in it, what are the implications of this flag on a tracked (and changed) file in an ignored dir?

What happens to empty directories that match an ignore pattern?

Show 5 quoted lines

> @@ -1362,6 +1363,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)
>                   N_("ignore changes to submodules, optional when: all, dirty, untracked. (Default: all)"),
>                   PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
>                 OPT_COLUMN(0, "column", &s.colopts, N_("list untracked files in columns")),
> +               OPT_BOOL(0, "show-ignored-directory", &show_ignored_directory,

Is it possible to directly read into  s.show_ignored_directory here?

Show 7 quoted lines

> +test_expect_success 'Verify behavior of status on folders with ignored files' '
> +       test_when_finished "git clean -fdx" &&
> +       git status --porcelain=v2 --ignored --untracked-files=all --show-ignored-directory >output &&
> +       test_i18ncmp expect output
> +'
> +
> +# Test status bahavior on folder with tracked and ignored files

behavior

Show 8 quoted lines

> +cat >expect <<\EOF
> +? expect
> +? output
> +! dir/tracked_ignored/ignored_1.ign
> +! dir/tracked_ignored/ignored_2.ign
> +! tracked_ignored/ignored_1.ign
> +! tracked_ignored/ignored_2.ign
> +EOF

I think our latest 'best style' is to include these heredocs into the test.