[PATCH v2 00/10] cat-file: better usage UX & error messages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This series of patches to cat-file significantly improves the UX of
the -h output, see 08/10. For the v1 see[1]

This update addresses comments Eric Sunshine had on test patterns &
typos. I've taken (or attemted to address) all his comments
here. There was a leftover unused $switch variable from development,
and some other minor test & typo nits.

Junio: John Cai expressed (off-list) interest in building on top of
this for the cat-file "new stdin mode"+fflush() batch command. You
seemed to think the direction of my WIP patch in that direction was
good, whic his much easier to implement, test & be sure about the
correctness off after this series.

So it would be great to have this tightening of the options behavior &
UX improvement picked up.

1. https://lore.kernel.org/git/cover-00.10-00000000000-20211106T214259Z-avarab@xxxxxxxxx/
2. https://lore.kernel.org/git/xmqqk0hitnkc.fsf@gitster.g/
3. https://lore.kernel.org/git/xmqqk0hitnkc.fsf@gitster.g/

Ævar Arnfjörð Bjarmason (10):
  cat-file tests: test bad usage
  cat-file tests: test messaging on bad objects/paths
  parse-options API: add a usage_msg_optf()
  cat-file docs: fix SYNOPSIS and "-h" output
  cat-file: move "usage" variable to cmd_cat_file()
  cat-file: make --batch-all-objects a CMDMODE
  cat-file: fix remaining usage bugs
  cat-file: correct and improve usage information
  object-name.c: don't have GET_OID_ONLY_TO_DIE imply *_QUIETLY
  cat-file: improve --(textconv|filters) disambiguation

 Documentation/git-cat-file.txt |  10 +-
 builtin/cat-file.c             | 181 ++++++++++++++++++++-------------
 builtin/stash.c                |   4 +-
 cache.h                        |   1 +
 object-name.c                  |  11 +-
 parse-options.c                |  13 +++
 parse-options.h                |  10 ++
 t/t1006-cat-file.sh            |  92 +++++++++++++++++
 t/t8007-cat-file-textconv.sh   |  26 +++++
 9 files changed, 267 insertions(+), 81 deletions(-)

Range-diff against v1:
 1:  c8040da8e55 !  1:  3a0d2923cfa cat-file tests: test bad usage
    @@ Commit message
         when --batch-all-objects is combined with --textconv or --filters, but
         we don't.
     
    +    The cases of needing to assign to opt=2 in the "opt" loop are because
    +    on those we do the right thing already, in subsequent commits the
    +    "test_expect_failure" cases will be fixed, and the for-loops unified.
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
      ## t/t1006-cat-file.sh ##
    @@ t/t1006-cat-file.sh: test_description='git cat-file'
      
      . ./test-lib.sh
      
    -+test_cmdmode_usage() {
    ++test_cmdmode_usage () {
     +	test_expect_code 129 "$@" 2>err &&
     +	grep "^error:.*is incompatible with" err
     +}
     +
    -+test_expect_success 'usage: cmdmode' '
    -+	test_cmdmode_usage git cat-file -e -p &&
    -+	test_cmdmode_usage git cat-file -p -t &&
    -+	test_cmdmode_usage git cat-file -t -s &&
    -+	test_cmdmode_usage git cat-file -s --textconv &&
    -+	test_cmdmode_usage git cat-file --textconv --filters
    -+'
    ++for switches in \
    ++    '-e -p' \
    ++    '-p -t' \
    ++    '-t -s' \
    ++    '-s --textconv' \
    ++    '--textconv --filters'
    ++do
    ++	test_expect_success "usage: cmdmode $switches" '
    ++		test_cmdmode_usage git cat-file $switches
    ++	'
    ++done
     +
    -+test_incompatible_usage() {
    ++test_incompatible_usage () {
     +	test_expect_code 129 "$@" 2>err &&
    -+	grep -E "^error:.*$switch.*needs" err
    ++	grep -E "^error:.**needs" err
     +}
     +
     +for opt in --batch --batch-check
 2:  a473185eb97 =  2:  fc8d5e60682 cat-file tests: test messaging on bad objects/paths
 3:  5d87897f49c =  3:  0e2e5ab9d2d parse-options API: add a usage_msg_optf()
 4:  29b67330a48 =  4:  b9c935b95b7 cat-file docs: fix SYNOPSIS and "-h" output
 5:  1974136d483 =  5:  664c5db634e cat-file: move "usage" variable to cmd_cat_file()
 6:  ee49e586483 !  6:  d945fc94774 cat-file: make --batch-all-objects a CMDMODE
    @@ Commit message
         instead. It doesn't combine with --filters, --textconv etc.
     
         By giving parse_options() information about what options are mutually
    -    exclusive with one another we can get the die9) message being removed
    +    exclusive with one another we can get the die() message being removed
         here for free, we didn't even use that removed message in some cases,
         e.g. for both of:
     
    @@ builtin/cat-file.c: int cmd_cat_file(int argc, const char **argv, const char *pr
      		usage_with_options(usage, options);
     
      ## t/t1006-cat-file.sh ##
    -@@ t/t1006-cat-file.sh: test_expect_success 'usage: cmdmode' '
    - 	test_cmdmode_usage git cat-file -p -t &&
    - 	test_cmdmode_usage git cat-file -t -s &&
    - 	test_cmdmode_usage git cat-file -s --textconv &&
    --	test_cmdmode_usage git cat-file --textconv --filters
    -+	test_cmdmode_usage git cat-file --textconv --filters &&
    -+	test_cmdmode_usage git cat-file --batch-all-objects -e
    - '
    - 
    - test_incompatible_usage() {
    +@@ t/t1006-cat-file.sh: for switches in \
    +     '-p -t' \
    +     '-t -s' \
    +     '-s --textconv' \
    +-    '--textconv --filters'
    ++    '--textconv --filters' \
    ++    '--batch-all-objects -e'
    + do
    + 	test_expect_success "usage: cmdmode $switches" '
    + 		test_cmdmode_usage git cat-file $switches
     @@ t/t1006-cat-file.sh: do
      	test_expect_success "usage: $opt requires another option" '
      		test_expect_code 129 git cat-file $opt
 7:  9e1dcd6b824 !  7:  22f55e1fb6b cat-file: fix remaining usage bugs
    @@ builtin/cat-file.c: int cmd_cat_file(int argc, const char **argv, const char *pr
      		die("git cat-file --allow-unknown-type: use with -s or -t");
     
      ## t/t1006-cat-file.sh ##
    -@@ t/t1006-cat-file.sh: test_expect_success 'usage: cmdmode' '
    +@@ t/t1006-cat-file.sh: done
      
    - test_incompatible_usage() {
    + test_incompatible_usage () {
      	test_expect_code 129 "$@" 2>err &&
    --	grep -E "^error:.*$switch.*needs" err
    -+	grep -E "^(fatal|error):.*$switch.*(requires|incompatible with|needs)" err
    +-	grep -E "^error:.**needs" err
    ++	grep -E "^(fatal|error):.*(requires|incompatible with|needs)" err
      }
      
      for opt in --batch --batch-check
 8:  951fea02b83 =  8:  0842df64695 cat-file: correct and improve usage information
 9:  49f9e30792b !  9:  6642b57c6fe object-name.c: don't have GET_OID_ONLY_TO_DIE imply *_QUIETLY
    @@ Commit message
         call get_oid_with_context_1() again via this
         maybe_die_on_misspelt_object_name() function.
     
    -    In the subsequent commit we'll add a new caller that excepts to call
    +    In the subsequent commit we'll add a new caller that expects to call
         this only once, but who would still like to have all the error
         messaging that GET_OID_ONLY_TO_DIE gives it, in addition to any
         regular errors.
10:  3d61399aa78 = 10:  177f16ba856 cat-file: improve --(textconv|filters) disambiguation
-- 
2.34.0.rc2.795.g926201d1cc8




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux