Re: [PATCH 2/2] ls-files: add --sparse option

From: Elijah Newren <newren@gmail.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Victoria Dye" <vdye@github.com>,
	"Derrick Stolee" <derrickstolee@github.com>,
	"Derrick Stolee" <dstolee@microsoft.com>
Subject: Re: [PATCH 2/2] ls-files: add --sparse option
Date: Wed, 8 Dec 2021 10:36:09 -0800	[thread overview]
Message-ID: <CABPp-BEyvkRdxJoJZcNF1VoQBcJajq1CVUrNTfHnHG433Q=cNQ@mail.gmail.com> (raw)
In-Reply-To: <d1275b30-b9b0-a416-3300-9809d880eb55@gmail.com>

On Wed, Dec 8, 2021 at 10:23 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 12/8/2021 12:04 PM, Elijah Newren wrote:
> > On Wed, Dec 8, 2021 at 7:14 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> >> Taking your idea and applying it to 'ls-files' (without --stage to
> >> avoid OIDs which would change depending on the hash algorithm), the
> >> start of the test looks like this:
> >>
> >> test_expect_success 'ls-files' '
> >>         init_repos &&
> >>
> >>         # Behavior agrees by default. Sparse index is expanded.
> >>         test_all_match git ls-files &&
> >>
> >>         # With --sparse, the sparse index data changes behavior.
> >>         git -C sparse-index ls-files --stage >out &&
> >>         git -C sparse-index ls-files --stage --sparse >sparse &&
> >>
> >>         cat >expect <<-\EOF &&
> >>          e
> >>          folder1-
> >>          folder1.x
> >>         -folder1/0/0/0
> >>         -folder1/0/1
> >>         -folder1/a
> >>         +folder1/
> >>          folder10
> >>         -folder2/0/0/0
> >>         -folder2/0/1
> >>         -folder2/a
> >>         +folder2/
> >>          g
> >>         -x/a
> >>         +x/
> >>          z
> >>         EOF
> >>
> >>         diff -u out sparse | tail -n 16 >actual &&
> >>         test_cmp expect actual
> >> '
> >
> > This actually looks quite nice, though the magic '16' is kind of
> > annoying.  Could we get rid of that -- perhaps using something to rip
> > out the diff header, or using comm instead?
>
> What I really want is "remove the first two lines of this file"

Is `tail -n +3` portable?  Looks like we have five uses of tail -n +N
in the testsuite, so it should be okay to use.

> but perhaps "tail -n $(wc -l expect)" would suffice to avoid a
> magic number?

That works too.

> > Also, perhaps 'dense' rather than 'out'?
>
> Sounds good.
>
> Thanks,
> -Stolee