All of lore.kernel.org
 help / color / mirror / Atom feed
From: ZheNing Hu <adlternative@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com>,
	"Git List" <git@vger.kernel.org>,
	"Christian Couder" <christian.couder@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Torsten Bögershausen" <tboegi@web.de>
Subject: Re: [PATCH v8] ls-files: introduce "--format" option
Date: Sun, 24 Jul 2022 19:08:00 +0800	[thread overview]
Message-ID: <CAOLTT8RqMU-k85LmcpY0wATHSDoWDEQLnPtfuZ2OC2nWN9305A@mail.gmail.com> (raw)
In-Reply-To: <xmqqsfmr8ygp.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> 于2022年7月24日周日 02:40写道:
>
> ZheNing Hu <adlternative@gmail.com> writes:
>
> >> But is this testing the right thing?
> >
> > Yes, I am sure about that cut can do the same thing as awk, and it can
> > specify its delimiter.
>
> That is not an answer to "is this testing the right thing?"
> question, though ;-)
>
> >> > +test_expect_success 'git ls-files --format objectmode v.s. -s' '
> >> > +     git ls-files -s >files &&
> >> > +     cut -d" " -f1 files >expect &&
> >> > +     git ls-files --format="%(objectmode)" >actual &&
> >> > +     test_cmp expect actual
> >> > +'
> >>
> >> It only looks at the first column of the "-s" output, and we are
> >> implicitly assuming that the order of output does not change between
> >> the "-s" output and "--format=<format>" output.  I wonder if it is
> >> more useful and less error prone to come up with a format string
> >> that 100% reproduces the "ls-files -s" output and compare the two,
> >> e.g.
> >>
> >>         format="%(objectmode) %(objectname) %(stage)    %(path)" &&
> >>         git ls-files -s >expect &&
> >>         git ls-files --format="$format" >actual &&
> >>         test_cmp expect actual
> >>
> >
> > See test case: 'git ls-files --format imitate --stage' which just do such thing,
>
>
> That was not the point.  By extracting only "%(objectmode)" without
> having any other clues (like "%(path)") on the same line, the test
> is assuming that ls-files will always sort its output in the same
> order regardless of the output format, whether it is "--stage" or
> "--format=<spec>", and that was what the "is this testing the right
> thing?" question was about.
>

Ah, so that we should sort the ls-files output first, and then compare them.

> The other test that makes sure --format=<spec> can recreate --stage
> output is fine.  If some future developer breaks the output order by
> mistake for --format=<spec>, we will catch such a mistake with it.
>
>
> > maybe I should change its name to 'git ls-files --format v.s. -s'?
>
> I do not think you should.  "A v.s. B" does not imply "A and B
> should create identical result".  The original title describes what
> it does much more clearly.

Ok, here I don't need another rerolling to revert it, right?

Thanks for all the reviews!

ZheNing Hu

  parent reply	other threads:[~2022-07-24 11:08 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-15 13:45 [PATCH 0/2] ls-files: introduce "--format" and "--object-only" options ZheNing Hu via GitGitGadget
2022-06-15 13:45 ` [PATCH 1/2] ls-files: introduce "--format" option ZheNing Hu via GitGitGadget
2022-06-15 20:07   ` Ævar Arnfjörð Bjarmason
2022-06-18 10:50     ` ZheNing Hu
2022-06-15 13:45 ` [PATCH 2/2] ls-files: introduce "--object-only" option ZheNing Hu via GitGitGadget
2022-06-15 20:15   ` Ævar Arnfjörð Bjarmason
2022-06-18 10:59     ` ZheNing Hu
2022-06-19  9:13 ` [PATCH v2] ls-files: introduce "--format" option ZheNing Hu via GitGitGadget
2022-06-19 13:50   ` Phillip Wood
2022-06-20 13:32     ` ZheNing Hu
2022-06-21  2:05   ` [PATCH v3] " ZheNing Hu via GitGitGadget
2022-06-23 14:06     ` Phillip Wood
2022-06-23 15:57       ` Junio C Hamano
2022-06-24 10:16         ` Phillip Wood
2022-06-26 13:05           ` ZheNing Hu
2022-06-24 13:20         ` Ævar Arnfjörð Bjarmason
2022-06-24 15:28           ` Junio C Hamano
2022-06-26 13:01       ` ZheNing Hu
2022-06-24 13:25     ` Ævar Arnfjörð Bjarmason
2022-06-24 15:33       ` Junio C Hamano
2022-06-26 13:35         ` ZheNing Hu
2022-06-27  8:22           ` Junio C Hamano
2022-06-27 11:06             ` ZheNing Hu
2022-06-27 15:41               ` Junio C Hamano
2022-07-01 13:30                 ` ZheNing Hu
2022-06-26 13:34       ` ZheNing Hu
2022-06-26 15:29     ` [PATCH v4] " ZheNing Hu via GitGitGadget
2022-06-27  8:32       ` Junio C Hamano
2022-06-27 11:18         ` ZheNing Hu
2022-06-27 18:34       ` Ævar Arnfjörð Bjarmason
2022-07-01 12:42         ` ZheNing Hu
2022-06-28 15:19       ` Phillip Wood
2022-07-01 12:47         ` ZheNing Hu
2022-07-05  6:32       ` [PATCH v5] " ZheNing Hu via GitGitGadget
2022-07-05  8:39         ` Ævar Arnfjörð Bjarmason
2022-07-11 15:14           ` ZheNing Hu
2022-07-05 19:28         ` Torsten Bögershausen
2022-07-11 15:27           ` ZheNing Hu
2022-07-11 16:53         ` [PATCH v6] " ZheNing Hu via GitGitGadget
2022-07-11 22:11           ` Junio C Hamano
2022-07-12 13:53             ` ZheNing Hu
2022-07-12 14:34               ` Junio C Hamano
2022-07-13  6:07           ` [PATCH v7] " ZheNing Hu via GitGitGadget
2022-07-18  8:09             ` Ævar Arnfjörð Bjarmason
2022-07-19 16:19               ` ZheNing Hu
2022-07-19 16:47                 ` Junio C Hamano
2022-07-19 17:21                   ` ZheNing Hu
2022-07-20 16:36             ` [PATCH v8] " ZheNing Hu via GitGitGadget
2022-07-20 17:37               ` Junio C Hamano
2022-07-21 15:54                 ` Ævar Arnfjörð Bjarmason
2022-07-21 17:22                   ` Eric Sunshine
2022-07-21 17:23                   ` Junio C Hamano
2022-07-22  6:44                 ` ZheNing Hu
2022-07-23 18:40                   ` Junio C Hamano
2022-07-23 18:46                     ` Junio C Hamano
2022-07-24 11:08                     ` ZheNing Hu [this message]
2022-07-25  1:03                       ` Junio C Hamano
2022-07-25 11:00                         ` ZheNing Hu
2022-07-23  6:44               ` [PATCH v9] " ZheNing Hu via GitGitGadget
2022-09-08  2:01                 ` Jiang Xin
2022-09-11 11:01                   ` ZheNing Hu

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

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

  git send-email \
    --in-reply-to=CAOLTT8RqMU-k85LmcpY0wATHSDoWDEQLnPtfuZ2OC2nWN9305A@mail.gmail.com \
    --to=adlternative@gmail.com \
    --cc=avarab@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=phillip.wood123@gmail.com \
    --cc=tboegi@web.de \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.