All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: Thomas via GitGitGadget <gitgitgadget@gmail.com>,
	 git@vger.kernel.org, Thomas <thomasqueirozb@gmail.com>
Subject: Re: [PATCH] completion: fix zsh parsing $GIT_PS1_SHOWUPSTREAM
Date: Thu, 25 Apr 2024 22:39:10 -0700	[thread overview]
Message-ID: <xmqqr0esbs3l.fsf@gitster.g> (raw)
In-Reply-To: <Zir-eeK0CZxVLhcR@tapette.crustytoothpaste.net> (brian m. carlson's message of "Fri, 26 Apr 2024 01:08:09 +0000")

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> I wonder if it might actually be better to adjust the shell options when
> we call into __git_ps1.  We could write this like so:
>
> 	[ -z "${ZSH_VERSION-}" ] || setopt localoptions shwordsplit
>
> That will turn on shell word splitting for just that function (and the
> functions it calls), so the existing code will work fine and we won't
> tamper with the user's preferred shell options.

Nice.  I did

    $ git grep -e 'for [a-z0-9_]* in ' contrib/completion/

and wondered why other hits were OK.  The completion one seems to
have "emulate" all over the place to hide zsh-ness from functions it
borrows from git-completion.bash, but git-prompt side seems to lack
necessary "compatibility" stuff.

> My concern is that changing the way we write the code here might result
> in someone unintentionally changing it back because it's less intuitive.
> By specifically asking zsh to use shell word splitting, we get
> consistent behaviour between bash and zsh, which is really what we want
> anyway.

Very well said.

> I use the above syntax (minus the shell check) in my zsh prompt and can
> confirm it works as expected.

Thanks.

By the way, I notice that the title of the patch talks about
"completion", but this is about a prompt.  It needs to be updated in
a future iteration.


      reply	other threads:[~2024-04-26  5:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-25 18:59 [PATCH] completion: fix zsh parsing $GIT_PS1_SHOWUPSTREAM Thomas via GitGitGadget
2024-04-26  1:08 ` brian m. carlson
2024-04-26  5:39   ` Junio C Hamano [this message]

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=xmqqr0esbs3l.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=sandals@crustytoothpaste.net \
    --cc=thomasqueirozb@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.