* Re: [PATCH] i18n: factorize even more 'incompatible options' messages
From: Eric Sunshine @ 2023-11-27 17:48 UTC (permalink / raw)
To: René Scharfe
Cc: Jean-Noël Avila, Patrick Steinhardt, Junio C Hamano,
Git List
In-Reply-To: <d1f28272-635d-4638-b0f4-76d64013b0d5@web.de>
On Mon, Nov 27, 2023 at 12:39 PM René Scharfe <l.s.r@web.de> wrote:
> Am 26.11.23 um 18:49 schrieb Eric Sunshine:
> > A couple additional instances recently slipped into `show-ref.c` which
> > were caught during review[1,2] but nevertheless made it to
> > "master"[3,4]. This patch, of course, doesn't need to address those,
> > but if rerolling for some other reason, perhaps they can be included,
> > as well(?).
> >
> > [1]: https://lore.kernel.org/git/CAPig+cSrp7vZuy7D_ENHKZKZzF4OSmCtfYNHPGMtS1Hj6gArDw@mail.gmail.com/
> > [2]: https://lore.kernel.org/git/CAPig+cRTOMie0rUf=Mhbo9e2EXf-_2kQyMeqpB9OCRB1MZZ1rw@mail.gmail.com/
> > [3]: 199970e72f (builtin/show-ref: ensure mutual exclusiveness of
> > subcommands, 2023-10-31)
> > [4]: 9080a7f178 (builtin/show-ref: add new mode to check for reference
> > existence, 2023-10-31)
>
> [4] changes the message added by [3], so that's one instance, right?
Yes, correct. Sorry for misleadingly saying there were two new
messages; it's been a month since I looked at the patches closely, and
forgot that it was just the single message which had been modified by
the subsequent patch.
Thanks for taking care of this so quickly!
> --- >8 ---
> Subject: [PATCH] show-ref: use die_for_incompatible_opt3()
>
> Use the standard message for reporting the use of multiple mutually
> exclusive options by calling die_for_incompatible_opt3() instead of
> rolling our own. This has the benefits of showing only the actually
> given options, reducing the number of strings to translate and making
> the UI slightly more consistent.
>
> Adjust the test to no longer insist on a specific order of the
> reported options, as this implementation detail does not affect the
> usefulness of the error message.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
Apparently, I haven't been following along closely enough, thus wasn't
aware that we had the die_for_incompatible_opt*() family of functions.
Neat.
^ permalink raw reply
* Re: [PATCH] i18n: factorize even more 'incompatible options' messages
From: René Scharfe @ 2023-11-27 17:39 UTC (permalink / raw)
To: Eric Sunshine
Cc: Jean-Noël Avila, Patrick Steinhardt, Junio C Hamano,
Git List
In-Reply-To: <CAPig+cR5PKkyC24LkOU4+yzng1xeBOBbADTBHXH61xkAR7kymw@mail.gmail.com>
Am 26.11.23 um 18:49 schrieb Eric Sunshine:
> On Sun, Nov 26, 2023 at 6:57 AM René Scharfe <l.s.r@web.de> wrote:
>> Continue the work of 12909b6b8a (i18n: turn "options are incompatible"
>> into "cannot be used together", 2022-01-05) and a699367bb8 (i18n:
>> factorize more 'incompatible options' messages, 2022-01-31) to use the
>> same parameterized error message for reporting incompatible command line
>> options. This reduces the number of strings to translate and makes the
>> UI slightly more consistent.
>
> Thanks for tackling this.
>
> A couple additional instances recently slipped into `show-ref.c` which
> were caught during review[1,2] but nevertheless made it to
> "master"[3,4]. This patch, of course, doesn't need to address those,
> but if rerolling for some other reason, perhaps they can be included,
> as well(?).
>
> [1]: https://lore.kernel.org/git/CAPig+cSrp7vZuy7D_ENHKZKZzF4OSmCtfYNHPGMtS1Hj6gArDw@mail.gmail.com/
> [2]: https://lore.kernel.org/git/CAPig+cRTOMie0rUf=Mhbo9e2EXf-_2kQyMeqpB9OCRB1MZZ1rw@mail.gmail.com/
> [3]: 199970e72f (builtin/show-ref: ensure mutual exclusiveness of
> subcommands, 2023-10-31)
> [4]: 9080a7f178 (builtin/show-ref: add new mode to check for reference
> existence, 2023-10-31)
[4] changes the message added by [3], so that's one instance, right?
--- >8 ---
Subject: [PATCH] show-ref: use die_for_incompatible_opt3()
Use the standard message for reporting the use of multiple mutually
exclusive options by calling die_for_incompatible_opt3() instead of
rolling our own. This has the benefits of showing only the actually
given options, reducing the number of strings to translate and making
the UI slightly more consistent.
Adjust the test to no longer insist on a specific order of the
reported options, as this implementation detail does not affect the
usefulness of the error message.
Reported-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
builtin/show-ref.c | 6 +++---
t/t1403-show-ref.sh | 16 +++++++++-------
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 7aac525a87..59d2291cbf 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -315,9 +315,9 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, show_ref_options,
show_ref_usage, 0);
- if ((!!exclude_existing_opts.enabled + !!verify + !!exists) > 1)
- die(_("only one of '%s', '%s' or '%s' can be given"),
- "--exclude-existing", "--verify", "--exists");
+ die_for_incompatible_opt3(exclude_existing_opts.enabled, "--exclude-existing",
+ verify, "--verify",
+ exists, "--exists");
if (exclude_existing_opts.enabled)
return cmd_show_ref__exclude_existing(&exclude_existing_opts);
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index b50ae6fcf1..d477689e33 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -197,18 +197,20 @@ test_expect_success 'show-ref --verify with dangling ref' '
'
test_expect_success 'show-ref sub-modes are mutually exclusive' '
- cat >expect <<-EOF &&
- fatal: only one of ${SQ}--exclude-existing${SQ}, ${SQ}--verify${SQ} or ${SQ}--exists${SQ} can be given
- EOF
-
test_must_fail git show-ref --verify --exclude-existing 2>err &&
- test_cmp expect err &&
+ grep "verify" err &&
+ grep "exclude-existing" err &&
+ grep "cannot be used together" err &&
test_must_fail git show-ref --verify --exists 2>err &&
- test_cmp expect err &&
+ grep "verify" err &&
+ grep "exists" err &&
+ grep "cannot be used together" err &&
test_must_fail git show-ref --exclude-existing --exists 2>err &&
- test_cmp expect err
+ grep "exclude-existing" err &&
+ grep "exists" err &&
+ grep "cannot be used together" err
'
test_expect_success '--exists with existing reference' '
--
2.43.0
^ permalink raw reply related
* What's cooking in git.git (Nov 2023, #09; Mon, 27)
From: Junio C Hamano @ 2023-11-27 14:24 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
As I mentioned earlier, I'll be offline for a week or two, during
which time there will little movement in my repositories. The next
cycle should start soon after that.
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[New Topics]
* ad/merge-file-diff-algo (2023-11-22) 1 commit
- merge-file: add --diff-algorithm option
"git merge-file" learned to take the "--diff-algorithm" option to
use algorithm different from the default "myers" diff.
Will merge to 'next'?
source: <pull.1606.v2.git.git.1700507932937.gitgitgadget@gmail.com>
* ak/p4-initial-empty-commits (2023-11-23) 1 commit
- git-p4: fix fast import when empty commit is first
Expecting a reroll.
source: <pull.1609.git.git.1700639764041.gitgitgadget@gmail.com>
* jc/checkout-B-branch-in-use (2023-11-23) 2 commits
- checkout: forbid "-B <branch>" from touching a branch used elsewhere
- checkout: refactor die_if_checked_out() caller
"git checkout -B <branch> [<start-point>]" allowed a branch that is
in use in another worktree to be updated and checked out, which
might be a bit unexpected. The rule has been tightened, which is a
breaking change. "--ignore-other-worktrees" option is required to
unbreak you, if you are used to the current behaviour that "-B"
overrides the safety.
Needs review and documentation updates.
source: <xmqqjzq9cl70.fsf@gitster.g>
* jh/trace2-redact-auth (2023-11-23) 4 commits
- t0212: test URL redacting in EVENT format
- t0211: test URL redacting in PERF format
- trace2: redact passwords from https:// URLs by default
- trace2: fix signature of trace2_def_param() macro
trace2 streams used to record the URLs that potentially embed
authentication material, which has been corrected.
Will merge to 'next'.
source: <pull.1616.git.1700680717.gitgitgadget@gmail.com>
* ps/commit-graph-less-paranoid (2023-11-26) 1 commit
- commit-graph: disable GIT_COMMIT_GRAPH_PARANOIA by default
Earlier we stopped relying on commit-graph that (still) records
information about commits that are lost from the object store,
which has negative performance implications. The default has been
flipped to disable this pessimization.
Will merge to 'next'.
source: <17e08289cd59d20de0de9b4e18f5e6bf77987351.1700823746.git.ps@pks.im>
* ps/reftable-fixes (2023-11-22) 8 commits
- reftable/stack: fix stale lock when dying
- reftable/merged: reuse buffer to compute record keys
- reftable/stack: reuse buffers when reloading stack
- reftable/stack: perform auto-compaction with transactional interface
- reftable/stack: verify that `reftable_stack_add()` uses auto-compaction
- reftable: handle interrupted writes
- reftable: handle interrupted reads
- reftable: wrap EXPECT macros in do/while
Bunch of small fix-ups to the reftable code.
Needs review.
source: <cover.1700549493.git.ps@pks.im>
* en/complete-sparse-checkout (2023-11-27) 4 commits
- completion: avoid user confusion in non-cone mode
- completion: avoid misleading completions in cone mode
- completion: fix logic for determining whether cone mode is active
- completion: squelch stray errors in sparse-checkout completion
source: <pull.1349.v2.git.1700985086.gitgitgadget@gmail.com>
* jb/reflog-expire-delete-dry-run-options (2023-11-26) 1 commit
- builtin/reflog.c: fix dry-run option short name
source: <20231126000514.85509-1-josh@brob.st>
* jc/orphan-unborn (2023-11-24) 2 commits
- orphan/unborn: fix use of 'orphan' in end-user facing messages
- orphan/unborn: add to the glossary and use them consistently
source: <xmqq4jhb977x.fsf@gitster.g>
* rs/column-leakfix (2023-11-27) 1 commit
- column: release strbuf and string_list after use
source: <f087137d-a5aa-487e-a1cb-0ad7117b38ed@web.de>
* rs/i18n-cannot-be-used-together (2023-11-27) 1 commit
- i18n: factorize even more 'incompatible options' messages
source: <e6eb12e4-bb63-473c-9c2f-965a4d5981ad@web.de>
--------------------------------------------------
[Stalled]
* pw/rebase-sigint (2023-09-07) 1 commit
- rebase -i: ignore signals when forking subprocesses
If the commit log editor or other external programs (spawned via
"exec" insn in the todo list) receive internactive signal during
"git rebase -i", it caused not just the spawned program but the
"Git" process that spawned them, which is often not what the end
user intended. "git" learned to ignore SIGINT and SIGQUIT while
waiting for these subprocesses.
Expecting a reroll.
cf. <12c956ea-330d-4441-937f-7885ab519e26@gmail.com>
source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>
* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
- cherry-pick: refuse cherry-pick sequence if index is dirty
"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.
Expecting a reroll.
cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>
* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
- diff-lib: fix check_removed() when fsmonitor is active
- Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
- Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
(this branch uses jc/fake-lstat.)
The optimization based on fsmonitor in the "diff --cached"
codepath is resurrected with the "fake-lstat" introduced earlier.
It is unknown if the optimization is worth resurrecting, but in case...
source: <xmqqr0n0h0tw.fsf@gitster.g>
--------------------------------------------------
[Cooking]
* ac/fuzz-show-date (2023-11-20) 1 commit
- fuzz: add new oss-fuzz fuzzer for date.c / date.h
Subject approxidate() and show_date() macchinery to OSS-Fuzz.
Will merge to 'next'?
source: <pull.1612.v4.git.1700243267653.gitgitgadget@gmail.com>
* js/packfile-h-typofix (2023-11-20) 1 commit
- packfile.c: fix a typo in `each_file_in_pack_dir_fn()`'s declaration
Typofix.
Will merge to 'next'.
source: <pull.1614.git.1700226915859.gitgitgadget@gmail.com>
* jw/builtin-objectmode-attr (2023-11-16) 2 commits
- SQUASH???
- attr: add builtin objectmode values support
The builtin_objectmode attribute is populated for each path
without adding anything in .gitattributes files, which would be
useful in magic pathspec, e.g., ":(attr:builtin_objectmode=100755)"
to limit to executables.
source: <20231116054437.2343549-1-jojwang@google.com>
* ps/ref-deletion-updates (2023-11-17) 4 commits
- refs: remove `delete_refs` callback from backends
- refs: deduplicate code to delete references
- refs/files: use transactions to delete references
- t5510: ensure that the packed-refs file needs locking
Simplify API implementation to delete references by eliminating
duplication.
Will merge to 'next'.
source: <cover.1699951815.git.ps@pks.im>
* tz/send-email-negatable-options (2023-11-17) 2 commits
(merged to 'next' on 2023-11-17 at f09e533e43)
+ send-email: avoid duplicate specification warnings
+ perl: bump the required Perl version to 5.8.1 from 5.8.0
Newer versions of Getopt::Long started giving warnings against our
(ab)use of it in "git send-email". Bump the minimum version
requirement for Perl to 5.8.1 (from September 2002) to allow
simplifying our implementation.
Will merge to 'master'.
source: <20231116193014.470420-1-tmz@pobox.com>
* js/ci-discard-prove-state (2023-11-14) 1 commit
(merged to 'next' on 2023-11-14 at fade3ba143)
+ ci: avoid running the test suite _twice_
(this branch uses ps/ci-gitlab.)
The way CI testing used "prove" could lead to running the test
suite twice needlessly, which has been corrected.
Will merge to 'master'.
source: <pull.1613.git.1699894837844.gitgitgadget@gmail.com>
* jk/chunk-bounds-more (2023-11-09) 9 commits
(merged to 'next' on 2023-11-13 at 3df4b18bea)
+ commit-graph: mark chunk error messages for translation
+ commit-graph: drop verify_commit_graph_lite()
+ commit-graph: check order while reading fanout chunk
+ commit-graph: use fanout value for graph size
+ commit-graph: abort as soon as we see a bogus chunk
+ commit-graph: clarify missing-chunk error messages
+ commit-graph: drop redundant call to "lite" verification
+ midx: check consistency of fanout table
+ commit-graph: handle overflow in chunk_size checks
(this branch is used by tb/pair-chunk-expect.)
Code clean-up for jk/chunk-bounds topic.
Will merge to 'master'.
source: <20231109070310.GA2697602@coredump.intra.peff.net>
* ps/httpd-tests-on-nixos (2023-11-11) 3 commits
(merged to 'next' on 2023-11-13 at 81bd6f5334)
+ t9164: fix inability to find basename(1) in Subversion hooks
+ t/lib-httpd: stop using legacy crypt(3) for authentication
+ t/lib-httpd: dynamically detect httpd and modules path
Portability tweak.
Will merge to 'master'.
source: <cover.1699596457.git.ps@pks.im>
* ss/format-patch-use-encode-headers-for-cover-letter (2023-11-10) 1 commit
(merged to 'next' on 2023-11-14 at 1a4bd59e15)
+ format-patch: fix ignored encode_email_headers for cover letter
"git format-patch --encode-email-headers" ignored the option when
preparing the cover letter, which has been corrected.
Will merge to 'master'.
source: <20231109111950.387219-1-contact@emersion.fr>
* ps/ban-a-or-o-operator-with-test (2023-11-11) 4 commits
(merged to 'next' on 2023-11-14 at d84471baab)
+ Makefile: stop using `test -o` when unlinking duplicate executables
+ contrib/subtree: convert subtree type check to use case statement
+ contrib/subtree: stop using `-o` to test for number of args
+ global: convert trivial usages of `test <expr> -a/-o <expr>`
Test and shell scripts clean-up.
Will merge to 'master'.
source: <cover.1699609940.git.ps@pks.im>
* ak/rebase-autosquash (2023-11-16) 3 commits
(merged to 'next' on 2023-11-17 at 3ed6e79445)
+ rebase: rewrite --(no-)autosquash documentation
+ rebase: support --autosquash without -i
+ rebase: fully ignore rebase.autoSquash without -i
"git rebase --autosquash" is now enabled for non-interactive rebase,
but it is still incompatible with the apply backend.
Will merge to 'master'.
source: <20231114214339.10925-1-andy.koppe@gmail.com>
* vd/for-each-ref-unsorted-optimization (2023-11-16) 10 commits
(merged to 'next' on 2023-11-17 at ff99420bf6)
+ t/perf: add perf tests for for-each-ref
+ ref-filter.c: use peeled tag for '*' format fields
+ for-each-ref: clean up documentation of --format
+ ref-filter.c: filter & format refs in the same callback
+ ref-filter.c: refactor to create common helper functions
+ ref-filter.c: rename 'ref_filter_handler()' to 'filter_one()'
+ ref-filter.h: add functions for filter/format & format-only
+ ref-filter.h: move contains caches into filter
+ ref-filter.h: add max_count and omit_empty to ref_format
+ ref-filter.c: really don't sort when using --no-sort
"git for-each-ref --no-sort" still sorted the refs alphabetically
which paid non-trivial cost. It has been redefined to show output
in an unspecified order, to allow certain optimizations to take
advantage of.
Will merge to 'master'.
source: <pull.1609.v2.git.1699991638.gitgitgadget@gmail.com>
* jw/git-add-attr-pathspec (2023-11-04) 1 commit
(merged to 'next' on 2023-11-13 at b61be94e4d)
+ attr: enable attr pathspec magic for git-add and git-stash
"git add" and "git stash" learned to support the ":(attr:...)"
magic pathspec.
Will merge to 'master'.
source: <20231103163449.1578841-1-jojwang@google.com>
* ps/ci-gitlab (2023-11-09) 8 commits
(merged to 'next' on 2023-11-10 at ea7ed67945)
+ ci: add support for GitLab CI
+ ci: install test dependencies for linux-musl
+ ci: squelch warnings when testing with unusable Git repo
+ ci: unify setup of some environment variables
+ ci: split out logic to set up failed test artifacts
+ ci: group installation of Docker dependencies
+ ci: make grouping setup more generic
+ ci: reorder definitions for grouping functions
(this branch is used by js/ci-discard-prove-state.)
Add support for GitLab CI.
Will merge to 'master'.
source: <cover.1699514143.git.ps@pks.im>
* ps/ref-tests-update (2023-11-03) 10 commits
(merged to 'next' on 2023-11-13 at dc26e55d6f)
+ t: mark several tests that assume the files backend with REFFILES
+ t7900: assert the absence of refs via git-for-each-ref(1)
+ t7300: assert exact states of repo
+ t4207: delete replace references via git-update-ref(1)
+ t1450: convert tests to remove worktrees via git-worktree(1)
+ t: convert tests to not access reflog via the filesystem
+ t: convert tests to not access symrefs via the filesystem
+ t: convert tests to not write references via the filesystem
+ t: allow skipping expected object ID in `ref-store update-ref`
+ Merge branch 'ps/show-ref' into ps/ref-tests-update
Update ref-related tests.
Will merge to 'master'.
source: <cover.1698914571.git.ps@pks.im>
* jx/fetch-atomic-error-message-fix (2023-10-19) 2 commits
- fetch: no redundant error message for atomic fetch
- t5574: test porcelain output of atomic fetch
"git fetch --atomic" issued an unnecessary empty error message,
which has been corrected.
Expecting an update.
cf. <ZTjQIrCgSANAT8wR@tanuki>
source: <ced46baeb1c18b416b4b4cc947f498bea2910b1b.1697725898.git.zhiyou.jx@alibaba-inc.com>
* js/bugreport-in-the-same-minute (2023-10-16) 1 commit
- bugreport: include +i in outfile suffix as needed
Instead of auto-generating a filename that is already in use for
output and fail the command, `git bugreport` learned to fuzz the
filename to avoid collisions with existing files.
Expecting a reroll.
cf. <ZTtZ5CbIGETy1ucV.jacob@initialcommit.io>
source: <20231016214045.146862-2-jacob@initialcommit.io>
* kh/t7900-cleanup (2023-10-17) 9 commits
- t7900: fix register dependency
- t7900: factor out packfile dependency
- t7900: fix `print-args` dependency
- t7900: fix `pfx` dependency
- t7900: factor out common schedule setup
- t7900: factor out inheritance test dependency
- t7900: create commit so that branch is born
- t7900: setup and tear down clones
- t7900: remove register dependency
Test clean-up.
Perhaps discard?
cf. <655ca147-c214-41be-919d-023c1b27b311@app.fastmail.com>
source: <cover.1697319294.git.code@khaugsbakk.name>
* tb/merge-tree-write-pack (2023-10-23) 5 commits
- builtin/merge-tree.c: implement support for `--write-pack`
- bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
- bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
- bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
- bulk-checkin: extract abstract `bulk_checkin_source`
"git merge-tree" learned "--write-pack" to record its result
without creating loose objects.
Broken when an object created during a merge is needed to continue merge
cf. <CABPp-BEfy9VOvimP9==ry_rZXu=metOQ8s=_-XiG_Pdx9c06Ww@mail.gmail.com>
source: <cover.1698101088.git.me@ttaylorr.com>
* tb/pair-chunk-expect (2023-11-10) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDL` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
- Merge branch 'jk/chunk-bounds-more' into HEAD
(this branch uses jk/chunk-bounds-more.)
Further code clean-up.
Needs review.
source: <cover.1699569246.git.me@ttaylorr.com>
* tb/path-filter-fix (2023-10-18) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Needs (hopefully final and quick) review.
source: <cover.1697653929.git.me@ttaylorr.com>
* cc/git-replay (2023-11-26) 14 commits
- replay: stop assuming replayed branches do not diverge
- replay: add --contained to rebase contained branches
- replay: add --advance or 'cherry-pick' mode
- replay: use standard revision ranges
- replay: make it a minimal server side command
- replay: remove HEAD related sanity check
- replay: remove progress and info output
- replay: add an important FIXME comment about gpg signing
- replay: change rev walking options
- replay: introduce pick_regular_commit()
- replay: die() instead of failing assert()
- replay: start using parse_options API
- replay: introduce new builtin
- t6429: remove switching aspects of fast-rebase
Introduce "git replay", a tool meant on the server side without
working tree to recreate a history.
Will merge to 'next'.
cf. <6bfe1541-54dd-ca6b-e930-94d3038060f1@gmx.de>
source: <20231124111044.3426007-1-christian.couder@gmail.com>
* ak/color-decorate-symbols (2023-10-23) 7 commits
- log: add color.decorate.pseudoref config variable
- refs: exempt pseudorefs from pattern prefixing
- refs: add pseudorefs array and iteration functions
- log: add color.decorate.ref config variable
- log: add color.decorate.symbol config variable
- log: use designated inits for decoration_colors
- config: restructure color.decorate documentation
A new config for coloring.
Needs review.
source: <20231023221143.72489-1-andy.koppe@gmail.com>
* js/update-urls-in-doc-and-comment (2023-11-26) 4 commits
- doc: refer to internet archive
- doc: update links for andre-simon.de
- doc: switch links to https
- doc: update links to current pages
Stale URLs have been updated to their current counterparts (or
archive.org) and HTTP links are replaced with working HTTPS links.
Will merge to 'next'.
source: <pull.1589.v3.git.1700796916.gitgitgadget@gmail.com>
* la/trailer-cleanups (2023-10-20) 3 commits
- trailer: use offsets for trailer_start/trailer_end
- trailer: find the end of the log message
- commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
Comments?
source: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2023-10-04) 4 commits
- archive: support remote archive from stateless transport
- transport-helper: call do_take_over() in connect_helper
- transport-helper: call do_take_over() in process_connect
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Needs review.
source: <cover.1696432593.git.zhiyou.jx@alibaba-inc.com>
* jx/sideband-chomp-newline-fix (2023-10-04) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Needs review.
source: <cover.1696425168.git.zhiyou.jx@alibaba-inc.com>
* js/config-parse (2023-09-21) 5 commits
- config-parse: split library out of config.[c|h]
- config.c: accept config_parse_options in git_config_from_stdin
- config: report config parse errors using cb
- config: split do_event() into start and flush operations
- config: split out config_parse_options
The parsing routines for the configuration files have been split
into a separate file.
Needs review.
source: <cover.1695330852.git.steadmon@google.com>
* jc/fake-lstat (2023-09-15) 1 commit
- cache: add fake_lstat()
(this branch is used by jc/diff-cached-fsmonitor-fix.)
A new helper to let us pretend that we called lstat() when we know
our cache_entry is up-to-date via fsmonitor.
Needs review.
source: <xmqqcyykig1l.fsf@gitster.g>
* js/doc-unit-tests (2023-11-10) 3 commits
(merged to 'next' on 2023-11-10 at 7d00ffd06b)
+ ci: run unit tests in CI
+ unit tests: add TAP unit test framework
+ unit tests: add a project plan document
(this branch is used by js/doc-unit-tests-with-cmake.)
Process to add some form of low-level unit tests has started.
Will merge to 'master'.
source: <cover.1699555664.git.steadmon@google.com>
* js/doc-unit-tests-with-cmake (2023-11-10) 7 commits
(merged to 'next' on 2023-11-10 at b4503c9c8c)
+ cmake: handle also unit tests
+ cmake: use test names instead of full paths
+ cmake: fix typo in variable name
+ artifacts-tar: when including `.dll` files, don't forget the unit-tests
+ unit-tests: do show relative file paths
+ unit-tests: do not mistake `.pdb` files for being executable
+ cmake: also build unit tests
(this branch uses js/doc-unit-tests.)
Update the base topic to work with CMake builds.
Will merge to 'master'.
source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-10-16) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Needs review.
source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>
--------------------------------------------------
[Discarded]
* jc/strbuf-comment-line-char (2023-11-01) 4 commits
. strbuf: move env-using functions to environment.c
. strbuf: make add_lines() public
. strbuf_add_commented_lines(): drop the comment_line_char parameter
. strbuf_commented_addf(): drop the comment_line_char parameter
Code simplification that goes directly against a past libification
topic. It is hard to judge because the "libification" is done
piecewise without seemingly clear design principle.
Will discard.
source: <cover.1698791220.git.jonathantanmy@google.com>
^ permalink raw reply
* Re: [PATCH v2 0/5] Avoid hang if curl needs eof twice + minor related improvements
From: Jiří Hruška @ 2023-11-27 13:39 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <20231115232824.2507648-1-jonathantanmy@google.com>
> I've already reviewed 1/5 so I'll summarize what I think of the rest.
Thank you for your time looking at everything, Jonathan.
I'm sorry for the delay in my reply, I was away for a while and also trying
to get clarity from the cURL side, summarized in [1].
[1] https://lore.kernel.org/git/CAGE_+C5u9H8m5faK1vXKk6QTyjcHgKHqxOZy5ptzsYbF_0yrCQ@mail.gmail.com/
Most importantly, the bug has been apparently in libcurl only for ~1 year and
it can no longer manifest for a few months. Therefore, the main point of my
changes is a bit moot and we might reconsider what is useful and what is not.
> I'm a bit reluctant to include 2/5 and 4/5.
> I think 3/5 [...], part of 4/5 [...] and 5/5 have significant benefit and
> should be included, though.
Fair enough. I will update everything based on your and other's comments and
my new learnings, then the final decision can be made about each part.
^ permalink raw reply
* Re: [PATCH v2 5/5] http: reset CURLOPT_POSTFIELDSIZE_LARGE between requests
From: Jiří Hruška @ 2023-11-27 13:21 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jeff King, Jonathan Tan, Junio C Hamano
In-Reply-To: <ZVRovA9OSfY5odhy@tanuki>
> It feels quite easy for this list to grow stale whenever we start to set
> a new option somewhere else. Is there a specific reason why we can't
> instead use `curl_easy_reset()` here?
Good point, makes sense. Thanks, Patrick, I will use that instead.
^ permalink raw reply
* Re: [PATCH] remote-curl: avoid hang if curl asks for more data after eof
From: Jiří Hruška @ 2023-11-27 13:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Tan, git, Jeff King
In-Reply-To: <CAGE_+C7h6HOh+ptdhwJ3MNn5HWFoc1WF-foLRrew6FJEo_yupg@mail.gmail.com>
I have discussed this with Daniel Stenberg himself in this thread:
https://curl.se/mail/lib-2023-11/0017.html
The summary is:
- It was never intended/expected that CURLOPT_READFUNCTION would be called
more after already returning EOF once. libcurl has been modified to never
do this anymore (in master, to be released in what comes after 8.4.0).
- The problem has been tracked down to a particular case that could reproduce
only if using HTTP/2 and only in cURL versions 7.84.0 – 8.1.2 (inclusive,
roughly between June 2022 and July 2023). The same misbehavior is still
technically possible in newer versions, but not happening in practice.
Therefore, I believe nothing strongly _needs_ to be done on the Git side.
^ permalink raw reply
* Re: [PATCH] i18n: factorize even more 'incompatible options' messages
From: Junio C Hamano @ 2023-11-27 13:12 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Jean-Noël Avila
In-Reply-To: <e6eb12e4-bb63-473c-9c2f-965a4d5981ad@web.de>
René Scharfe <l.s.r@web.de> writes:
> Continue the work of 12909b6b8a (i18n: turn "options are incompatible"
> into "cannot be used together", 2022-01-05) and a699367bb8 (i18n:
> factorize more 'incompatible options' messages, 2022-01-31) to use the
> same parameterized error message for reporting incompatible command line
> options. This reduces the number of strings to translate and makes the
> UI slightly more consistent.
Will queue. Thanks.
^ permalink raw reply
* Re: [PATCH] i18n: factorize even more 'incompatible options' messages
From: Junio C Hamano @ 2023-11-27 13:11 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Jean-Noël Avila
In-Reply-To: <e6eb12e4-bb63-473c-9c2f-965a4d5981ad@web.de>
René Scharfe <l.s.r@web.de> writes:
> Continue the work of 12909b6b8a (i18n: turn "options are incompatible"
> into "cannot be used together", 2022-01-05) and a699367bb8 (i18n:
> factorize more 'incompatible options' messages, 2022-01-31) to use the
> same parameterized error message for reporting incompatible command line
> options. This reduces the number of strings to translate and makes the
> UI slightly more consistent.
Will queue. Thanks.
^ permalink raw reply
* Re: [PATCH] column: release strbuf and string_list after use
From: Junio C Hamano @ 2023-11-27 12:04 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List
In-Reply-To: <f087137d-a5aa-487e-a1cb-0ad7117b38ed@web.de>
René Scharfe <l.s.r@web.de> writes:
> Releasing strbuf and string_list just before exiting is not strictly
> necessary, but it gets rid of false positives reported by leak checkers,
> which can then be more easily used to show that the column-printing
> machinery behind print_columns() are free of leaks.
Thanks for being careful. Will queue.
^ permalink raw reply
* --end-of-options inconsistently available?!
From: Sven Strickroth @ 2023-11-27 11:22 UTC (permalink / raw)
To: git
Hi,
gitcli(7) states:
> Because -- disambiguates revisions and paths in some commands, it cannot be used for those commands to separate options and revisions. You can use --end-of-options for this (it also works for commands that do not distinguish between revisions in paths, in which case it is simply an alias for --).
However, when I use this for certain commands it fails:
$ git reset --end-of-options HEAD --
fatal: option '--end-of-options' must come before non-option arguments
$ git rev-parse --symbolic-full-name --end-of-options master
--end-of-options
refs/heads/master
Here, the output also contains "--end-of-options" as if it is a
reference (same for "--")
$ git checkout -f --end-of-options HEAD~1 -- afile.txt
fatal: only one reference expected, 2 given.
Best,
Sven
^ permalink raw reply
* Re: Fix git-send-email.perl w.r.t. recent Getopt::Long update
From: H.Merijn Brand @ 2023-11-27 8:38 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <xmqqzfz03tbn.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 2548 bytes --]
On Mon, 27 Nov 2023 09:58:52 +0900, Junio C Hamano <gitster@pobox.com> wrote:
> "H.Merijn Brand" <linux@tux.freedom.nl> writes:
>
> > From the Getopt::Long changes:
> > ```
> > Changes in version 2.55
> > -----------------------
> > * Fix long standing bug that duplicate options were not detected when
> > the options differ in case while ignore_case is in effect.
> > This will now yield a warning and become a fatal error in a future
> > release.
> > ```
> >
> > Current version is 2.57
>
> This patch looks like duplicate of
>
> https://lore.kernel.org/git/20231116193014.470420-1-tmz@pobox.com/
>
> perhaps independently discovered and worked on. Thanks for caring.
>
> One downside of unconditional upgrade of the call is, of course,
> that it would no longer work for those with older Getopt::Long that
> did not support the "!" suffix. Fortunately, Getopt::Long 2.33
> started shipping with Perl 5.8.1 that is more than 20 years old, so
> with the series we accepted, we also have a change to bump the
> required version of Perl from 5.8.0 to 5.8.1 to make it clear that
> it is deliberate that we drop the support for anything older at the
> same time.
The is a no-issue ...
Just the 'use Getopt::Long' is enough to guarantee a working version:
The '!' was already implemented in version 2.10 (April 1997):
--8<---
=item !
Option does not take an argument and may be negated, i.e. prefixed by
"no". E.g. "foo!" will allow B<--foo> (with value 1) and B<-nofoo>
(with value 0).
The option variable will be set to 1, or 0 if negated.
-->8---
Looking at the ChangeLog, a reliable behavior of '!' was available
since version 2.22 (march 2000):
--8<---
Changes in version 2.22
-----------------------
* Fixes a bug in the combination of aliases and negation.
Old: "foo|bar!" allowed negation on foo, but not on bar.
New: "foo|bar!" allows negation on foo and bar.
Caveat: "foo|f!", with bundling, issues the warning that negation on
a short option is ignored. To obtain the desired behaviour, use
"foo!" => \$opt_foo, "f" => \$opt_foo
or
"foo|f" => \$opt_foo, "nofoo" => sub { $opt_foo = 0 }
Remember that this is _only_ required when bundling is in effect.
-->8---
--
H.Merijn Brand https://tux.nl Perl Monger http://amsterdam.pm.org/
using perl5.00307 .. 5.37 porting perl5 on HP-UX, AIX, and Linux
https://tux.nl/email.html http://qa.perl.org https://www.test-smoke.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] checkout: forbid "-B <branch>" from touching a branch used elsewhere
From: Junio C Hamano @ 2023-11-27 1:51 UTC (permalink / raw)
To: Phillip Wood; +Cc: Willem Verstraeten, git
In-Reply-To: <bf848477-b4dd-49d3-8e4b-de0fc3948570@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
>> diff --git a/builtin/checkout.c b/builtin/checkout.c
>> index b4ab972c5a..8a8ad23e98 100644
>> --- a/builtin/checkout.c
>> +++ b/builtin/checkout.c
>> @@ -1600,6 +1600,13 @@ static int checkout_branch(struct checkout_opts *opts,
>> if (new_branch_info->path && !opts->force_detach && !opts->new_branch)
>> die_if_switching_to_a_branch_in_use(opts, new_branch_info->path);
>> + /* "git checkout -B <branch>" */
>> + if (opts->new_branch_force) {
>> + char *full_ref = xstrfmt("refs/heads/%s", opts->new_branch);
>> + die_if_switching_to_a_branch_in_use(opts, full_ref);
>> + free(full_ref);
>
> At the moment this is academic as neither of the test scripts changed
> by this patch are leak free and so I don't think we need to worry
> about it but it raises an interesting question about how we should
> handle memory leaks when dying. Leaving the leak when dying means that
> a test script that tests an expected failure will never be leak free
> but using UNLEAK() would mean we miss a leak being introduced in the
> successful case should the call to "free()" ever be removed.
Is there a leak here? The piece of memory is pointed at by an on-stack
variable full_ref when leak sanitizer starts scanning the heap and
the stack just before the process exits due to die, so I do not see
a reason to worry about this particular variable over all the other
on stack variables we accumulated before the control reached this
point of the code.
Are you worried about optimizing compilers that behave more cleverly
than their own good to somehow lose the on-stack reference to
full_ref while calling die_if_switching_to_a_branch_in_use()? We
might need to squelch them with UNLEAK() but that does not mean we
have to remove the free() we see above, and I suspect a more
productive use of our time to solve that issue is ensure that our
leak-sanitizing build will not triger such an unwanted optimization
anyway.
Thanks.
^ permalink raw reply
* Re: [PATCH 4/4] completion: avoid user confusion in non-cone mode
From: Junio C Hamano @ 2023-11-27 1:39 UTC (permalink / raw)
To: Elijah Newren; +Cc: Elijah Newren via GitGitGadget, git
In-Reply-To: <CABPp-BE73cDq-DBKzsw3R0awiKn5J5LCfdXRULHyPbrS9GEn4Q@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
>> > if [[ "$using_cone" == "true" ]]; then
>> > __gitcomp_directories
>>
>> Hmph, doesn't "Providing the files and directories currently present
>> is thus always wrong." apply equally to cone mode?
>
> Absolutely, it definitely applies to cone mode. We (mostly) fixed
> that a long time ago, making it not complete on the files &
> directories currently present. In particular, the
> __gitcomp_directories() function highlighted here completes on the
> output of `git ls-tree -z -d --name-only HEAD`.
Thanks; what I missed was exactly what __gitcomp_directories does
not do you explained above.
>> > + # 4. Provides no completions when run from a
>> > + # subdirectory of the repository root. (If we
>> > + # did provide file/directory completions, the
>> > + # user would just get a "please run from the
>> > + # toplevel directory" error message when they
>> > + # ran it. *Further*, if the user did rerun
>> > + # the command from the toplevel, the
>> > + # completions we previously provided would
>> > + # likely be wrong as they'd be relative to the
>> > + # subdirectory rather than the repository
>> > + # root. That could lead to users getting a
>> > + # nasty surprise based on trying to use a
>> > + # command we helped them create.)
>>
>> Hmph, would an obvious alternative to (1) check against the HEAD (or
>> the index) to see if the prefix string matches an entity at the
>> current directory level, and then (2) to prefix the result of the
>> previous step with "/$(git rev-parse --show-prefix)" work? That is
>> something like this:
>>
>> $ cd t
>> $ git sparse-checkout add help<TAB>
>> ->
>> $ git sparse-checkout add /t/helper/
>
> I thought bash-completion was only for completions, not for startings
> as well. Was I mistaken?
To my mind, the completion is what I as an end user get when I type
<TAB> to help me formulate input that is acceptable by the command.
As I said, I consider it a bug (or UI mistake) in the a command if
it pretends to work inside a subdirecctory but complains when it is
given a path relative to the current directory, so I'd rather prefer
the approach to "fix" the underlying command, but if that is too
much work or cannot be done for whatever reason, the second best
would be to turn whatever we can do to help the end-user input into
a form that is accepted by the command without changing what the
input means. If it takes more than "appending at the end", that is
fine, at least by me as an end user.
If you are saying "completion code can only append at the end
because we can only return strings to be appended, not the entire
strings, to the readline machinery, so mucking with the start of the
string is not doable", then sorry---I accept that what we cannot do
cannot be done, and in that case you are "not mistaken".
But from the existing use of COMPREPLY[], it didn't look that way
(it seems __gitcomp is equipped to take fixed prefix to all
candidates by passing it in $2 and used to complete names of
configuration variables in a section, but it seems to me that it can
be repurposed when prefixing "$(git rev-parse --show-prefix)" to a
given pathname relative to the $cwd). And if that can be done, then
you are "not mistaken", but merely being dogmatic and limiting what
your code can do yourself.
>> Another more fundamental approach to avoid "confusion" this bullet
>> item tries to side step might be to *fix* the command that gets
>> completed. As "git sparse-checkout --help" is marked as
>> EXPERIMENTAL in capital letters, we should be able to say "what was
>> traditionally known as 'add' is from now on called 'add-pattern' and
>> command line completion would not get in the way; the 'add'
>> subcommand now takes only literal paths, not patterns, that are
>> relative to the current directory" if we wanted to.
>
> That's interesting...but it opens up a new can of worms:
> * Would we also need both `set-patterns` and `set`, in addition to
> `add-patterns` and `add`?
If "set" has a similar UI issue that confuses end-users, then sure,
I do not see a reason why we want to leave it confusing---the
experimental labelling is to allow us to fix these warts more
easily, no?
> * In cone mode, the paths passed are literal directories (and only
> directories; no individual files), but the thing added is a
> telescoping "cone" of leading directories as well. Does this make it
> potentially confusing to users to say that `add` only takes literal
> paths?
I do not know.
> * In cone mode (the default), should `add-patterns` just be an
> error, since no pattern specification is allowed?
I do not really care. "add-patterns" is a potential tool you can
use to reduce friction while fixing the UI warts in an experimental
command.
> * In the git-sparse-checkout manual, for performance reasons, we
> recommend users _not_ specify individual paths in non-cone mode.
> Would our recommendation then be to just not use `add` or `set` and
> only use `add-patterns` and `set-patterns`?
Very likely. If the desired behaviour from the command can only be
had by castrating features, then such a recommendation would not
mean much to end-users anyway, though.
> If so, what have we
> accomplished by adding the new names?
It is valuable for those who do need to go against recommendation
(because the recommendation robs usability from them way too much),
will have much less confusing and working completion when they use
'add' or 'set', no?
> Maybe I'm missing something about your suggestion, but this seems much
> more complex than the simple solution we implemented in bb8b5e9a90d
> ("sparse-checkout: pay attention to prefix for {set, add}",
> 2022-02-19) for the !core_sparse_checkout_cone case.
Oh, if we do honor the $(git rev-parse --show-prefix), then that
changes the equation somewhat. I got an impression from your log
message or cover letter that it wasn't the case, and that was where
the "if the command is so broken, then completion can add it for the
user" and "if the command is so broken, then fix it to take relative
paths" came from.
^ permalink raw reply
* Re: Fix git-send-email.perl w.r.t. recent Getopt::Long update
From: Junio C Hamano @ 2023-11-27 0:58 UTC (permalink / raw)
To: H.Merijn Brand; +Cc: git
In-Reply-To: <20231124103932.31ca7688@pc09>
"H.Merijn Brand" <linux@tux.freedom.nl> writes:
> From the Getopt::Long changes:
> ```
> Changes in version 2.55
> -----------------------
> * Fix long standing bug that duplicate options were not detected when
> the options differ in case while ignore_case is in effect.
> This will now yield a warning and become a fatal error in a future
> release.
> ```
>
> Current version is 2.57
This patch looks like duplicate of
https://lore.kernel.org/git/20231116193014.470420-1-tmz@pobox.com/
perhaps independently discovered and worked on. Thanks for caring.
One downside of unconditional upgrade of the call is, of course,
that it would no longer work for those with older Getopt::Long that
did not support the "!" suffix. Fortunately, Getopt::Long 2.33
started shipping with Perl 5.8.1 that is more than 20 years old, so
with the series we accepted, we also have a change to bump the
required version of Perl from 5.8.0 to 5.8.1 to make it clear that
it is deliberate that we drop the support for anything older at the
same time.
^ permalink raw reply
* Re: [PATCH v3 0/4] Switch links to https
From: Junio C Hamano @ 2023-11-27 0:49 UTC (permalink / raw)
To: Josh Soref; +Cc: Elijah Newren, Josh Soref via GitGitGadget, git, Eric Sunshine
In-Reply-To: <CACZqfqAq0ijtYO9-1q6h2KQMyxHgLpFOg--d4c9F4xT0nMkvUg@mail.gmail.com>
Josh Soref <jsoref@gmail.com> writes:
> Elijah Newren wrote:
>> As stated elsewhere, I'd be fine with using the archived link if the
>> justification presented in the series for using archived links was
>> consistent and mentioned both reasons for changes. But, I think this
>> series is fine to merge down as-is if you don't want to go through the
>> trouble. Especially given how long you've waited.
>
> I'm clearly still contributing, so I can come back later and cross
> that bridge...
>
>> Anyway, I checked through every link in this series; it all looks good to me.
>
> Let's take this as-is. Thanks for taking the time to re-check every
> link, I know exactly how tedious that is :).
Thanks, both. Will queue.
^ permalink raw reply
* Re: [PATCH] i18n: factorize even more 'incompatible options' messages
From: Eric Sunshine @ 2023-11-26 17:49 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Jean-Noël Avila
In-Reply-To: <e6eb12e4-bb63-473c-9c2f-965a4d5981ad@web.de>
On Sun, Nov 26, 2023 at 6:57 AM René Scharfe <l.s.r@web.de> wrote:
> Continue the work of 12909b6b8a (i18n: turn "options are incompatible"
> into "cannot be used together", 2022-01-05) and a699367bb8 (i18n:
> factorize more 'incompatible options' messages, 2022-01-31) to use the
> same parameterized error message for reporting incompatible command line
> options. This reduces the number of strings to translate and makes the
> UI slightly more consistent.
Thanks for tackling this.
A couple additional instances recently slipped into `show-ref.c` which
were caught during review[1,2] but nevertheless made it to
"master"[3,4]. This patch, of course, doesn't need to address those,
but if rerolling for some other reason, perhaps they can be included,
as well(?).
[1]: https://lore.kernel.org/git/CAPig+cSrp7vZuy7D_ENHKZKZzF4OSmCtfYNHPGMtS1Hj6gArDw@mail.gmail.com/
[2]: https://lore.kernel.org/git/CAPig+cRTOMie0rUf=Mhbo9e2EXf-_2kQyMeqpB9OCRB1MZZ1rw@mail.gmail.com/
[3]: 199970e72f (builtin/show-ref: ensure mutual exclusiveness of
subcommands, 2023-10-31)
[4]: 9080a7f178 (builtin/show-ref: add new mode to check for reference
existence, 2023-10-31)
^ permalink raw reply
* No automatic continue of rebasing with rerere
From: Vovodroid @ 2023-11-26 12:08 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <511253473.8084883.1701000481697.ref@mail.yahoo.com>
Hi,
when running non-interactive rebasing (i.e. no -i option) with rerere turned on, already resolved conflicts and recorded conflicts causing Git to return exit code 1, as if error occurred.
Also "git.exe rebase --continue" still opens log editor unless GIT_EDITOR environment variable is set to true.
Such behavior causes to user to perform a lot of unnecessary Git runs, or clicking if some GUI shell, like in GitExtension, is used.
See https://github.com/gitextensions/gitextensions/issues/11382 for discussion and screenshots.
Regards.
^ permalink raw reply
* [PATCH] i18n: factorize even more 'incompatible options' messages
From: René Scharfe @ 2023-11-26 11:57 UTC (permalink / raw)
To: Git List; +Cc: Jean-Noël Avila
Continue the work of 12909b6b8a (i18n: turn "options are incompatible"
into "cannot be used together", 2022-01-05) and a699367bb8 (i18n:
factorize more 'incompatible options' messages, 2022-01-31) to use the
same parameterized error message for reporting incompatible command line
options. This reduces the number of strings to translate and makes the
UI slightly more consistent.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
builtin/clone.c | 4 +++-
builtin/merge-tree.c | 3 ++-
parse-options.c | 3 ++-
revision.c | 4 ++--
t/t0040-parse-options.sh | 8 ++++----
t/t1006-cat-file.sh | 2 +-
t/t4301-merge-tree-write-tree.sh | 2 +-
t/t5606-clone-options.sh | 2 +-
8 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index c6357af949..45a5070268 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -965,7 +965,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
}
if (bundle_uri && deepen)
- die(_("--bundle-uri is incompatible with --depth, --shallow-since, and --shallow-exclude"));
+ die(_("options '%s' and '%s' cannot be used together"),
+ "--bundle-uri",
+ "--depth/--shallow-since/--shallow-exclude");
repo_name = argv[0];
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index a35e0452d6..a4aa6013c5 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -577,7 +577,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
if (o.mode == MODE_TRIVIAL)
die(_("--trivial-merge is incompatible with all other options"));
if (merge_base)
- die(_("--merge-base is incompatible with --stdin"));
+ die(_("options '%s' and '%s' cannot be used together"),
+ "--merge-base", "--stdin");
line_termination = '\0';
while (strbuf_getline_lf(&buf, stdin) != EOF) {
struct strbuf **split;
diff --git a/parse-options.c b/parse-options.c
index e0c94b0546..c3955847f4 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -279,7 +279,8 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
opt_name = optnamearg(opt, arg, flags);
other_opt_name = optnamearg(elem->opt, elem->arg, elem->flags);
- error(_("%s is incompatible with %s"), opt_name, other_opt_name);
+ error(_("options '%s' and '%s' cannot be used together"),
+ opt_name, other_opt_name);
free(opt_name);
free(other_opt_name);
return -1;
diff --git a/revision.c b/revision.c
index 00d5c29bfc..b2861474b1 100644
--- a/revision.c
+++ b/revision.c
@@ -2384,8 +2384,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->left_right = 1;
} else if (!strcmp(arg, "--left-only")) {
if (revs->right_only)
- die("--left-only is incompatible with --right-only"
- " or --cherry");
+ die(_("options '%s' and '%s' cannot be used together"),
+ "--left-only", "--right-only/--cherry");
revs->left_only = 1;
} else if (!strcmp(arg, "--right-only")) {
if (revs->left_only)
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 8fdef88b65..ec974867e4 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -376,7 +376,7 @@ test_expect_success 'OPT_CMDMODE() detects incompatibility (1)' '
test_must_be_empty output &&
test_grep "mode1" output.err &&
test_grep "mode2" output.err &&
- test_grep "is incompatible with" output.err
+ test_grep "cannot be used together" output.err
'
test_expect_success 'OPT_CMDMODE() detects incompatibility (2)' '
@@ -384,7 +384,7 @@ test_expect_success 'OPT_CMDMODE() detects incompatibility (2)' '
test_must_be_empty output &&
test_grep "mode2" output.err &&
test_grep "set23" output.err &&
- test_grep "is incompatible with" output.err
+ test_grep "cannot be used together" output.err
'
test_expect_success 'OPT_CMDMODE() detects incompatibility (3)' '
@@ -392,7 +392,7 @@ test_expect_success 'OPT_CMDMODE() detects incompatibility (3)' '
test_must_be_empty output &&
test_grep "mode2" output.err &&
test_grep "set23" output.err &&
- test_grep "is incompatible with" output.err
+ test_grep "cannot be used together" output.err
'
test_expect_success 'OPT_CMDMODE() detects incompatibility (4)' '
@@ -401,7 +401,7 @@ test_expect_success 'OPT_CMDMODE() detects incompatibility (4)' '
test_must_be_empty output &&
test_grep "mode2" output.err &&
test_grep "mode34.3" output.err &&
- test_grep "is incompatible with" output.err
+ test_grep "cannot be used together" output.err
'
test_expect_success 'OPT_COUNTUP() with PARSE_OPT_NODASH works' '
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index d73a0be1b9..271c5e4fd3 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -6,7 +6,7 @@ test_description='git cat-file'
test_cmdmode_usage () {
test_expect_code 129 "$@" 2>err &&
- grep "^error:.*is incompatible with" err
+ grep "^error: .* cannot be used together" err
}
for switches in \
diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index b2c8a43fce..12ac436873 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -887,7 +887,7 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '
test_expect_success '--merge-base is incompatible with --stdin' '
test_must_fail git merge-tree --merge-base=side1 --stdin 2>expect &&
- grep "^fatal: --merge-base is incompatible with --stdin" expect
+ grep "^fatal: .*merge-base.*stdin.* cannot be used together" expect
'
# specify merge-base as parent of branch2
diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh
index fc4bbd9daf..a400bcca62 100755
--- a/t/t5606-clone-options.sh
+++ b/t/t5606-clone-options.sh
@@ -64,7 +64,7 @@ test_expect_success 'disallows --bundle-uri with shallow options' '
for option in --depth=1 --shallow-since=01-01-2000 --shallow-exclude=HEAD
do
test_must_fail git clone --bundle-uri=bundle $option from to 2>err &&
- grep "bundle-uri is incompatible" err || return 1
+ grep "bundle-uri.* cannot be used together" err || return 1
done
'
--
2.43.0
^ permalink raw reply related
* [PATCH] column: release strbuf and string_list after use
From: René Scharfe @ 2023-11-26 11:57 UTC (permalink / raw)
To: Git List
Releasing strbuf and string_list just before exiting is not strictly
necessary, but it gets rid of false positives reported by leak checkers,
which can then be more easily used to show that the column-printing
machinery behind print_columns() are free of leaks.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
builtin/column.c | 2 ++
t/t9002-column.sh | 1 +
2 files changed, 3 insertions(+)
diff --git a/builtin/column.c b/builtin/column.c
index a83be8bc99..e80218f81f 100644
--- a/builtin/column.c
+++ b/builtin/column.c
@@ -56,5 +56,7 @@ int cmd_column(int argc, const char **argv, const char *prefix)
string_list_append(&list, sb.buf);
print_columns(&list, colopts, &copts);
+ strbuf_release(&sb);
+ string_list_clear(&list, 0);
return 0;
}
diff --git a/t/t9002-column.sh b/t/t9002-column.sh
index 6d3dbde3fe..348cc40658 100755
--- a/t/t9002-column.sh
+++ b/t/t9002-column.sh
@@ -1,6 +1,7 @@
#!/bin/sh
test_description='git column'
+TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success 'setup' '
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] builtin/reflog.c: fix dry-run option short name
From: Junio C Hamano @ 2023-11-26 8:44 UTC (permalink / raw)
To: Josh Brobst; +Cc: git
In-Reply-To: <20231126000514.85509-1-josh@brob.st>
Josh Brobst <josh@brob.st> writes:
> The documentation for reflog states that the --dry-run option of the
> expire and delete subcommands has a corresponding short name, -n.
> However, 33d7bdd645 (builtin/reflog.c: use parse-options api for expire,
> delete subcommands, 2022-01-06) did not include this short name in the
> new options parsing.
>
> Re-add the short name in the new dry-run option definitions.
Perfect. Thanks for spotting and doing necessary archaeology.
^ permalink raw reply
* [PATCH v2 4/4] completion: avoid user confusion in non-cone mode
From: Elijah Newren via GitGitGadget @ 2023-11-26 7:51 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1349.v2.git.1700985086.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
It is tempting to think of "files and directories" of the current
directory as valid inputs to the add and set subcommands of git
sparse-checkout. However, in non-cone mode, they often aren't and using
them as potential completions leads to *many* forms of confusion:
Issue #1. It provides the *wrong* files and directories.
For
git sparse-checkout add
we always want to add files and directories not currently in our sparse
checkout, which means we want file and directories not currently present
in the current working tree. Providing the files and directories
currently present is thus always wrong.
For
git sparse-checkout set
we have a similar problem except in the subset of cases where we are
trying to narrow our checkout to a strict subset of what we already
have. That is not a very common scenario, especially since it often
does not even happen to be true for the first use of the command; for
years we required users to create a sparse-checkout via
git sparse-checkout init
git sparse-checkout set <args...>
(or use a clone option that did the init step for you at clone time).
The init command creates a minimal sparse-checkout with just the
top-level directory present, meaning the set command has to be used to
expand the checkout. Thus, only in a special and perhaps unusual cases
would any of the suggestions from normal file and directory completion
be appropriate.
Issue #2: Suggesting patterns that lead to warnings is unfriendly.
If the user specifies any regular file and omits the leading '/', then
the sparse-checkout command will warn the user that their command is
problematic and suggest they use a leading slash instead.
Issue #3: Completion gets confused by leading '/', and provides wrong paths.
Users often want to anchor their patterns to the toplevel of the
repository, especially when listing individual files. There are a
number of reasons for this, but notably even sparse-checkout encourages
them to do so (as noted above). However, if users do so (via adding a
leading '/' to their pattern), then bash completion will interpret the
leading slash not as a request for a path at the toplevel of the
repository, but as a request for a path at the root of the filesytem.
That means at best that completion cannot help with such paths, and if
it does find any completions, they are almost guaranteed to be wrong.
Issue #4: Suggesting invalid patterns from subdirectories is unfriendly.
There is no per-directory equivalent to .gitignore with
sparse-checkouts. There is only a single worktree-global
$GIT_DIR/info/sparse-checkout file. As such, paths to files must be
specified relative to the toplevel of a repository. Providing
suggestions of paths that are relative to the current working directory,
as bash completion defaults to, is wrong when the current working
directory is not the worktree toplevel directory.
Issue #5: Paths with special characters will be interpreted incorrectly
The entries in the sparse-checkout file are patterns, not paths. While
most paths also qualify as patterns (though even in such cases it would
be better for users to not use them directly but prefix them with a
leading '/'), there are a variety of special characters that would need
special escaping beyond the normal shell escaping: '*', '?', '\', '[',
']', and any leading '#' or '!'. If completion suggests any such paths,
users will likely expect them to be treated as an exact path rather than
as a pattern that might match some number of files other than 1.
Because of the combination of the above issues, turn completion off for
the `set` and `add` subcommands of `sparse-checkout` when in non-cone
mode, but leave a NEEDSWORK comment specifying what could theoretically
be done if someone wanted to provide completion rules that were more
helpful than harmful.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
contrib/completion/git-completion.bash | 62 ++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c614e5d4f07..1e07dc8b73e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3138,6 +3138,68 @@ _git_sparse_checkout ()
fi
if [[ "$using_cone" == "true" ]]; then
__gitcomp_directories
+ else
+ # NEEDSWORK: It might be useful to provide a
+ # completion function which:
+ #
+ # 1. Provides completions based on
+ # files/directories that exist in HEAD (or in
+ # the index since sparse-index isn't possible
+ # in non-cone mode), not just those currently
+ # present in the working tree. Bash's
+ # default file and directory completion is
+ # totally useless for "git sparse-checkout
+ # add" because of this. It is likewise
+ # problematic for "git sparse-checkout set"
+ # except in those subset of cases when trying
+ # to narrow scope to a strict subset of what
+ # you already have checked out.
+ #
+ # 2. Always provides file/directory completions
+ # with a prepended leading '/', so that
+ # files/directories are only searched at the
+ # relevant level rather than throughout all
+ # trees in the hierarchy. Doing this also
+ # avoids suggesting the user run a
+ # sparse-checkout command that will result in
+ # a warning be thrown at the user.
+ #
+ # 3. Does not accidentally search the root of
+ # the filesystem when a path with a leading
+ # slash is specified. ("git sparse-checkout
+ # add /ho<TAB>" should not complete to
+ # "/home" but to e.g. "/hooks" if there is a
+ # "hooks" in the top of the repository.)
+ #
+ # 4. Provides no completions when run from a
+ # subdirectory of the repository root. (If we
+ # did provide file/directory completions, the
+ # user would just get a "please run from the
+ # toplevel directory" error message when they
+ # ran it. *Further*, if the user did rerun
+ # the command from the toplevel, the
+ # completions we previously provided would
+ # likely be wrong as they'd be relative to the
+ # subdirectory rather than the repository
+ # root. That could lead to users getting a
+ # nasty surprise based on trying to use a
+ # command we helped them create.)
+ #
+ # 5. Provides escaped completions for any paths
+ # containing a '*', '?', '\', '[', ']', or
+ # leading '#' or '!'. (These characters might
+ # already be escaped to protect from the
+ # shell, but they need an *extra* layer of
+ # escaping to prevent the pattern parsing in
+ # Git from seeing them as special characters.)
+ #
+ # Of course, this would be a lot of work, so for now,
+ # just avoid the many forms of user confusion that
+ # could be caused by providing bad completions by
+ # providing a fake completion to avoid falling back to
+ # bash's normal file and directory completion.
+
+ COMPREPLY=( "" )
fi
esac
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 3/4] completion: avoid misleading completions in cone mode
From: Elijah Newren via GitGitGadget @ 2023-11-26 7:51 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1349.v2.git.1700985086.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
The "set" and "add" subcommands of "sparse-checkout", when in cone mode,
should only complete on directories. For bash_completion in general,
when no completions are returned for any subcommands, it will often fall
back to standard completion of files and directories as a substitute.
That is not helpful here. Since we have already looked for all valid
completions, if none are found then falling back to standard bash file
and directory completion is at best actively misleading. In fact, there
are three different ways it can be actively misleading. Add a long
comment in the code about how that fallback behavior can deceive, and
disable the fallback by returning a fake result as the sole completion.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
contrib/completion/git-completion.bash | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7aa66c19ede..c614e5d4f07 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3090,6 +3090,26 @@ __gitcomp_directories ()
# No possible further completions any deeper, so assume we're at
# a leaf directory and just consider it complete
__gitcomp_direct_append "$cur "
+ elif [[ $_found == 0 ]]; then
+ # No possible completions found. Avoid falling back to
+ # bash's default file and directory completion, because all
+ # valid completions have already been searched and the
+ # fallbacks can do nothing but mislead. In fact, they can
+ # mislead in three different ways:
+ # 1) Fallback file completion makes no sense when asking
+ # for directory completions, as this function does.
+ # 2) Fallback directory completion is bad because
+ # e.g. "/pro" is invalid and should NOT complete to
+ # "/proc".
+ # 3) Fallback file/directory completion only completes
+ # on paths that exist in the current working tree,
+ # i.e. which are *already* part of their
+ # sparse-checkout. Thus, normal file and directory
+ # completion is always useless for "git
+ # sparse-checkout add" and is also probelmatic for
+ # "git sparse-checkout set" unless using it to
+ # strictly narrow the checkout.
+ COMPREPLY=( "" )
fi
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/4] completion: fix logic for determining whether cone mode is active
From: Elijah Newren via GitGitGadget @ 2023-11-26 7:51 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1349.v2.git.1700985086.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
_git_sparse_checkout() was checking whether we were in cone mode by
checking whether either:
A) core.sparseCheckoutCone was "true"
B) "--cone" was specified on the command line
This code has 2 bugs I didn't catch in my review at the time
1) core.sparseCheckout must be "true" for core.sparseCheckoutCone to
be relevant (which matters since "git sparse-checkout disable"
only unsets core.sparseCheckout, not core.sparseCheckoutCone)
2) The presence of "--no-cone" should override any config setting
Further, I forgot to update this logic as part of 2d95707a02
("sparse-checkout: make --cone the default", 2022-04-22) for the new
default.
Update the code for the new default and make it be more careful in
determining whether to complete based on cone mode or non-cone mode.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
contrib/completion/git-completion.bash | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b8661701718..7aa66c19ede 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3097,6 +3097,7 @@ _git_sparse_checkout ()
{
local subcommands="list init set disable add reapply"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
+ local using_cone=true
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
return
@@ -3107,8 +3108,15 @@ _git_sparse_checkout ()
__gitcomp_builtin sparse-checkout_$subcommand "" "--"
;;
set,*|add,*)
- if [ "$(__git config core.sparseCheckoutCone)" == "true" ] ||
- [ -n "$(__git_find_on_cmdline --cone)" ]; then
+ if [[ "$(__git config core.sparseCheckout)" == "true" &&
+ "$(__git config core.sparseCheckoutCone)" == "false" &&
+ -z "$(__git_find_on_cmdline --cone)" ]]; then
+ using_cone=false
+ fi
+ if [[ -n "$(__git_find_on_cmdline --no-cone)" ]]; then
+ using_cone=false
+ fi
+ if [[ "$using_cone" == "true" ]]; then
__gitcomp_directories
fi
esac
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 0/4] Sparse checkout completion fixes
From: Elijah Newren via GitGitGadget @ 2023-11-26 7:51 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren
In-Reply-To: <pull.1349.git.1700761448.gitgitgadget@gmail.com>
This fixes a few issues with tab completion for the sparse-checkout command,
specifically with the "add" and "set" subcommands.
As noted in v1, the 4th patch implements a somewhat suboptimal solution that
at least improves the situation, while also documenting with a code comment
a much more involved alternative solution that we could consider in the
future.
Changes since v1:
* Use __git wrapper function to squelch errors, as suggested by SZEDER
Gábor
* note that we could use the index or HEAD for the more involved solution
in patch 4.
[1] https://lore.kernel.org/git/xmqqv8yjz5us.fsf@gitster.g/
Elijah Newren (4):
completion: squelch stray errors in sparse-checkout completion
completion: fix logic for determining whether cone mode is active
completion: avoid misleading completions in cone mode
completion: avoid user confusion in non-cone mode
contrib/completion/git-completion.bash | 96 +++++++++++++++++++++++++-
1 file changed, 93 insertions(+), 3 deletions(-)
base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1349%2Fnewren%2Fsparse-checkout-completion-fixes-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1349/newren/sparse-checkout-completion-fixes-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1349
Range-diff vs v1:
1: 591c7b8d73b ! 1: 97e20e3b99d completion: squelch stray errors in sparse-checkout completion
@@ Commit message
fatal: ../: '../' is outside repository at '/home/newren/floss/git'
is written to stderr, which munges the users view of their own command.
- Squelch such messages.
+ Squelch such messages by using the __git() wrapper, designed for this
+ purpose; see commit e15098a314 (completion: consolidate silencing errors
+ from git commands, 2017-02-03) for more on the wrapper.
Signed-off-by: Elijah Newren <newren@gmail.com>
@@ contrib/completion/git-completion.bash: __gitcomp_directories ()
_found=1
fi
- done < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)
-+ done < <(git ls-tree -z -d --name-only HEAD $_tmp_dir 2>/dev/null)
++ done < <(__git ls-tree -z -d --name-only HEAD $_tmp_dir)
if [[ $_found == 0 ]] && [[ "$cur" =~ /$ ]]; then
# No possible further completions any deeper, so assume we're at
2: a15fb054579 = 2: 212ba35ed46 completion: fix logic for determining whether cone mode is active
3: e8cc5c54e60 = 3: 1cbbcd9097c completion: avoid misleading completions in cone mode
4: fe8669a3f4f ! 4: 604f21dc827 completion: avoid user confusion in non-cone mode
@@ contrib/completion/git-completion.bash: _git_sparse_checkout ()
+ # completion function which:
+ #
+ # 1. Provides completions based on
-+ # files/directories that exist in HEAD, not
-+ # just those currently present in the working
-+ # tree. Bash's default file and directory
-+ # completion is totally useless for "git
-+ # sparse-checkout add" because of this. It is
-+ # likewise problematic for "git
-+ # sparse-checkout set" except in those subset
-+ # of cases when trying to narrow scope to a
-+ # strict subset of what you already have
-+ # checked out.
++ # files/directories that exist in HEAD (or in
++ # the index since sparse-index isn't possible
++ # in non-cone mode), not just those currently
++ # present in the working tree. Bash's
++ # default file and directory completion is
++ # totally useless for "git sparse-checkout
++ # add" because of this. It is likewise
++ # problematic for "git sparse-checkout set"
++ # except in those subset of cases when trying
++ # to narrow scope to a strict subset of what
++ # you already have checked out.
+ #
+ # 2. Always provides file/directory completions
+ # with a prepended leading '/', so that
--
gitgitgadget
^ permalink raw reply
* [PATCH v2 1/4] completion: squelch stray errors in sparse-checkout completion
From: Elijah Newren via GitGitGadget @ 2023-11-26 7:51 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1349.v2.git.1700985086.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
If, in the root of a project, one types
git sparse-checkout set --cone ../<TAB>
then an error message of the form
fatal: ../: '../' is outside repository at '/home/newren/floss/git'
is written to stderr, which munges the users view of their own command.
Squelch such messages by using the __git() wrapper, designed for this
purpose; see commit e15098a314 (completion: consolidate silencing errors
from git commands, 2017-02-03) for more on the wrapper.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 13a39ebd2e7..b8661701718 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3084,7 +3084,7 @@ __gitcomp_directories ()
COMPREPLY+=("$c/")
_found=1
fi
- done < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)
+ done < <(__git ls-tree -z -d --name-only HEAD $_tmp_dir)
if [[ $_found == 0 ]] && [[ "$cur" =~ /$ ]]; then
# No possible further completions any deeper, so assume we're at
--
gitgitgadget
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox