All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Alison Winters <alisonatwork@outlook.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Alison Winters via GitGitGadget <gitgitgadget@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Sibi Siddharthan <sibisiv.siddharthan@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 0/2] add case insensitivity option to bash completion
Date: Tue, 29 Nov 2022 18:40:40 +0100	[thread overview]
Message-ID: <221129.86y1rtirhl.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <PRAP192MB1506B74F9BB25061419FA228DE129@PRAP192MB1506.EURP192.PROD.OUTLOOK.COM>


On Tue, Nov 29 2022, Alison Winters wrote:

> On 2022-11-29 10:38, Junio C Hamano wrote:
>> I did not try to figure out the reason but the topic with its tests
>> seem to break in 'seen' the linux-cmake-ctest CI job.
>>
>> https://github.com/git/git/actions/runs/3570230611/jobs/6001013549
>>
>> but the same test does not break under usual "make test".
>
> I cannot speak for how the changes of the ab/cmake-nix-and-ci impact
> the aw/complete-case-insensitive branch, but the failure seems to be
> pointing to a test that I have since changed in the v2 patch, on the
> suggestion of Szeder Gabor. The variable is no longer exported and
> the script is no longer sourced a second time. I don't know if those
> v2 changes would change the results of this test, but they might be a
> starting point for the other people CC:ed here to consider.

The breakage is due to a semantic conflict between the two, which is
solved thusly:
	
	diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
	index d45d820da28..2aa128075ca 100755
	--- a/t/t9902-completion.sh
	+++ b/t/t9902-completion.sh
	@@ -2261,7 +2261,7 @@ test_expect_success 'checkout does not match ref names of a different case' '
	 
	 test_expect_success 'checkout matches case insensitively with GIT_COMPLETION_IGNORE_CASE' '
	 	(
	-		. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
	+		. "$GIT_SOURCE_DIR/contrib/completion/git-completion.bash" &&
	 		GIT_COMPLETION_IGNORE_CASE=1 && export GIT_COMPLETION_IGNORE_CASE &&
	 		test_completion "git checkout M" <<-\EOF
	 		main Z
	@@ -2279,7 +2279,7 @@ test_expect_success 'checkout completes pseudo refs' '
	 
	 test_expect_success 'checkout completes pseudo refs case insensitively with GIT_COMPLETION_IGNORE_CASE' '
	 	(
	-		. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
	+		. "$GIT_SOURCE_DIR/contrib/completion/git-completion.bash" &&
	 		GIT_COMPLETION_IGNORE_CASE=1 && export GIT_COMPLETION_IGNORE_CASE &&
	 		test_completion "git checkout h" <<-\EOF
	 		HEAD Z

Junio: I don't think this warrants kicking out the cmake topic. This
sort of problem is just going to occur while something like that is
in-flight, or Alison would have otherwise presumably seen it as a CI
failure if based off "master".

With hindsight I could have made e74e51a9154 (cmake & test-lib.sh: add a
$GIT_SOURCE_DIR variable, 2022-11-03) do the migration in two phases,
but I didn't expect another topic to rely on the relatively obscure bits
that were being copied to the $GIT_BUILD_DIR, we weren't exactly
drowning in git-completion.bash patches...

  reply	other threads:[~2022-11-29 17:46 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-05 17:28 [PATCH 0/2] add case insensitivity option to bash completion Alison Winters via GitGitGadget
2022-11-05 17:28 ` [PATCH 1/2] completion: add optional ignore-case when matching refs Alison Winters via GitGitGadget
2022-11-20 20:24   ` SZEDER Gábor
2022-11-05 17:28 ` [PATCH 2/2] completion: add case-insensitive match of pseudorefs Alison Winters via GitGitGadget
2022-11-20 20:42   ` SZEDER Gábor
2022-11-20 20:46     ` SZEDER Gábor
2022-11-08  3:00 ` [PATCH 0/2] add case insensitivity option to bash completion Taylor Blau
2022-11-21  0:26 ` [PATCH v2 " Alison Winters via GitGitGadget
2022-11-21  0:26   ` [PATCH v2 1/2] completion: add optional ignore-case when matching refs Alison Winters via GitGitGadget
2022-11-21  0:26   ` [PATCH v2 2/2] completion: add case-insensitive match of pseudorefs Alison Winters via GitGitGadget
2022-11-29  2:38 ` [PATCH 0/2] add case insensitivity option to bash completion Junio C Hamano
2022-11-29 15:56   ` Alison Winters
2022-11-29 17:40     ` Ævar Arnfjörð Bjarmason [this message]
2022-11-30  0:37       ` Alison Winters
2022-11-30  3:08         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=221129.86y1rtirhl.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=alisonatwork@outlook.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=sibisiv.siddharthan@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.