diff mbox series

[v3,01/10] command-list.txt: sort with "LC_ALL=C sort"

Message ID patch-v3-01.10-c385e84c04c-20211105T135058Z-avarab@gmail.com (mailing list archive)
State Accepted
Commit 1ea3d7fcc4deaf1220a752b2bcb7c9c72270e264
Headers show
Series generate-cmdlist.sh: make it (and "make") run faster | expand

Commit Message

Ævar Arnfjörð Bjarmason Nov. 5, 2021, 2:07 p.m. UTC
We should keep these files sorted in the C locale, e.g. in the C
locale the order is:

    git-check-mailmap
    git-check-ref-format
    git-checkout

But under en_US.UTF-8 it's:

    git-check-mailmap
    git-checkout
    git-check-ref-format

In a subsequent commit I'll change generate-cmdlist.sh to use C sort
order, and without this change we'd be led to believe that that change
caused a meaningful change in the output, so let's do this as a
separate step, right now the generate-cmdlist.sh script just uses the
order found in this file.

Note that this refers to the sort order of the lines in
command-list.txt, a subsequent commit will also change how we treat
the sort order of the "category" fields, but that's unrelated to this
change.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 command-list.txt | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Junio C Hamano Nov. 5, 2021, 10:45 p.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> In a subsequent commit I'll change generate-cmdlist.sh to use C sort
> order, and without this change we'd be led to believe that that change
> caused a meaningful change in the output,

What I found misleading in this sentence (which hasn't changed after
I pointed it out in the previous iterations) is that
command-list.txt is an input file, and if the sort order used in the
script that reads this to produce some other file as its output
changes, nobody will be "led to believe" anything.  Not unless/until
which output file to look at and compare between revisions.

Is this talking about the order of entries in command-list.h file?

Also, if the script sorts the input, whether it is in C locale or
other locale, it would not matter how the input originally is
sorted, as the input does not have duplicated entries to make sort
stability matter, no?
Ævar Arnfjörð Bjarmason Nov. 6, 2021, 4:26 a.m. UTC | #2
On Fri, Nov 05 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> In a subsequent commit I'll change generate-cmdlist.sh to use C sort
>> order, and without this change we'd be led to believe that that change
>> caused a meaningful change in the output,
>
> What I found misleading in this sentence (which hasn't changed after
> I pointed it out in the previous iterations) is that[...]

I tried to clarify what you raised in the previous iteration with the
new paragraph after the one you're quoting. I.e.:

    Note that this refers to the sort order of the lines in
    command-list.txt, a subsequent commit will also change how we treat
    the sort order of the "category" fields, but that's unrelated to this
    change.

> command-list.txt is an input file, and if the sort order used in the
> script that reads this to produce some other file as its output
> changes, nobody will be "led to believe" anything.  Not unless/until
> which output file to look at and compare between revisions.

I'd read your comment on the previous iteration as you being unclear on
whether we were changing the sort order of lines in the file, or the
category fields found within those lines.

We don't sort the lines, and never have, just the fields, and within
this series we stop doing that sorting (as it ends up in an OR'd
bitfield, so it doesn't matter).

> Is this talking about the order of entries in command-list.h file?
>
> Also, if the script sorts the input, whether it is in C locale or
> other locale, it would not matter how the input originally is
> sorted, as the input does not have duplicated entries to make sort
> stability matter, no?

This change is just to make this consistent for human editors. I think
we re-sort this wherever we display this in git, whether that's via
help.c or the completion powered by git.c.
Junio C Hamano Nov. 8, 2021, 7:18 p.m. UTC | #3
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Fri, Nov 05 2021, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>>
>>> In a subsequent commit I'll change generate-cmdlist.sh to use C sort
>>> order, and without this change we'd be led to believe that that change
>>> caused a meaningful change in the output,
>>
>> What I found misleading in this sentence (which hasn't changed after
>> I pointed it out in the previous iterations) is that[...]
>
> I tried to clarify what you raised in the previous iteration with the
> new paragraph after the one you're quoting. I.e.:
>
>     Note that this refers to the sort order of the lines in
>     command-list.txt, a subsequent commit will also change how we treat
>     the sort order of the "category" fields, but that's unrelated to this
>     change.

Yeah, but my question was not about the order of category tokens on
each line.  My question was the order of these lines in the file.

The new pargraph is answering a question that wasn't asked.

>> Is this talking about the order of entries in command-list.h file?
>>
>> Also, if the script sorts the input, whether it is in C locale or
>> other locale, it would not matter how the input originally is
>> sorted, as the input does not have duplicated entries to make sort
>> stability matter, no?
>
> This change is just to make this consistent for human editors. I think
> we re-sort this wherever we display this in git, whether that's via
> help.c or the completion powered by git.c.

So

>> order, and without this change we'd be led to believe that that change
>> caused a meaningful change in the output,

is something irrelevant to explain what this change is, no?
diff mbox series

Patch

diff --git a/command-list.txt b/command-list.txt
index eb9cee8dee9..04cde20c3da 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -60,9 +60,9 @@  git-cat-file                            plumbinginterrogators
 git-check-attr                          purehelpers
 git-check-ignore                        purehelpers
 git-check-mailmap                       purehelpers
+git-check-ref-format                    purehelpers
 git-checkout                            mainporcelain
 git-checkout-index                      plumbingmanipulators
-git-check-ref-format                    purehelpers
 git-cherry                              plumbinginterrogators          complete
 git-cherry-pick                         mainporcelain
 git-citool                              mainporcelain
@@ -111,7 +111,6 @@  git-index-pack                          plumbingmanipulators
 git-init                                mainporcelain           init
 git-instaweb                            ancillaryinterrogators          complete
 git-interpret-trailers                  purehelpers
-gitk                                    mainporcelain
 git-log                                 mainporcelain           info
 git-ls-files                            plumbinginterrogators
 git-ls-remote                           plumbinginterrogators
@@ -124,11 +123,11 @@  git-merge-base                          plumbinginterrogators
 git-merge-file                          plumbingmanipulators
 git-merge-index                         plumbingmanipulators
 git-merge-one-file                      purehelpers
-git-mergetool                           ancillarymanipulators           complete
 git-merge-tree                          ancillaryinterrogators
-git-multi-pack-index                    plumbingmanipulators
+git-mergetool                           ancillarymanipulators           complete
 git-mktag                               plumbingmanipulators
 git-mktree                              plumbingmanipulators
+git-multi-pack-index                    plumbingmanipulators
 git-mv                                  mainporcelain           worktree
 git-name-rev                            plumbinginterrogators
 git-notes                               mainporcelain
@@ -154,23 +153,23 @@  git-request-pull                        foreignscminterface             complete
 git-rerere                              ancillaryinterrogators
 git-reset                               mainporcelain           history
 git-restore                             mainporcelain           worktree
-git-revert                              mainporcelain
 git-rev-list                            plumbinginterrogators
 git-rev-parse                           plumbinginterrogators
+git-revert                              mainporcelain
 git-rm                                  mainporcelain           worktree
 git-send-email                          foreignscminterface             complete
 git-send-pack                           synchingrepositories
+git-sh-i18n                             purehelpers
+git-sh-setup                            purehelpers
 git-shell                               synchelpers
 git-shortlog                            mainporcelain
 git-show                                mainporcelain           info
 git-show-branch                         ancillaryinterrogators          complete
 git-show-index                          plumbinginterrogators
 git-show-ref                            plumbinginterrogators
-git-sh-i18n                             purehelpers
-git-sh-setup                            purehelpers
 git-sparse-checkout                     mainporcelain
-git-stash                               mainporcelain
 git-stage                                                               complete
+git-stash                               mainporcelain
 git-status                              mainporcelain           info
 git-stripspace                          purehelpers
 git-submodule                           mainporcelain
@@ -189,7 +188,6 @@  git-var                                 plumbinginterrogators
 git-verify-commit                       ancillaryinterrogators
 git-verify-pack                         plumbinginterrogators
 git-verify-tag                          ancillaryinterrogators
-gitweb                                  ancillaryinterrogators
 git-whatchanged                         ancillaryinterrogators          complete
 git-worktree                            mainporcelain
 git-write-tree                          plumbingmanipulators
@@ -204,6 +202,7 @@  gitfaq                                  guide
 gitglossary                             guide
 githooks                                guide
 gitignore                               guide
+gitk                                    mainporcelain
 gitmailmap                              guide
 gitmodules                              guide
 gitnamespaces                           guide
@@ -211,6 +210,7 @@  gitremote-helpers                       guide
 gitrepository-layout                    guide
 gitrevisions                            guide
 gitsubmodules                           guide
-gittutorial-2                           guide
 gittutorial                             guide
+gittutorial-2                           guide
+gitweb                                  ancillaryinterrogators
 gitworkflows                            guide