public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks•im>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org, Johannes Schindelin <Johannes.Schindelin@gmx•de>
Subject: Re: [PATCH 2/4] request-pull: stop depending on Perl
Date: Wed, 16 Apr 2025 17:07:59 +0200	[thread overview]
Message-ID: <Z__Hz3D6d-4v_z7r@pks.im> (raw)
In-Reply-To: <xmqqtt6pmmd6.fsf@gitster.g>

On Tue, Apr 15, 2025 at 09:16:05AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks•im> writes:
> > diff --git a/git-request-pull.sh b/git-request-pull.sh
> > index 775ba8ea11a..59276fe265b 100755
> > --- a/git-request-pull.sh
> > +++ b/git-request-pull.sh
> > @@ -78,41 +78,47 @@ fi
> >  merge_base=$(git merge-base $baserev $headrev) ||
> >  die "fatal: No commits in common between $base and $head"
> >  
> > -# $head is the refname from the command line.
> > -# Find a ref with the same name as $head that exists at the remote
> > +find_matching_ref () {
> > +	while read sha1 ref
> > +	do
> > +		case "$ref" in
> > +		*"^{}")
> > +			ref="${ref%"^{}"}"
> > +			deref=true
> 
> This got a bit tighter (the original allowed ls-remote output to be
> later extended to throw "^something-else" at the end of line), which
> I do not know is something we need to worry about.  I think retaining
> the original semantics is easy in this case, e.g.,
> 
> 		*"^"?*)
> 			ref="${ref%"^*"}"

This should probably read `ref="${ref%"^"*}"`, with the `*` outside of
the quotes.

> or something, if we wanted to.  As this is meant to be faithful
> rewrite to lose Perl, not a bugfix to correct overly loose pattern
> matching in the original, I think we want to in this case.

Fair enough. It should've been at least mentioned in the commit message,
but let's instead just retain the current behaviour as suggested.

Patrick

  reply	other threads:[~2025-04-16 15:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-15  9:57 [PATCH 0/4] Drop Perl dependency in a couple of subsystems Patrick Steinhardt
2025-04-15  9:57 ` [PATCH 1/4] filter-branch: stop depending on Perl Patrick Steinhardt
2025-04-15 15:50   ` Junio C Hamano
2025-04-15  9:57 ` [PATCH 2/4] request-pull: " Patrick Steinhardt
2025-04-15 16:16   ` Junio C Hamano
2025-04-16 15:07     ` Patrick Steinhardt [this message]
2025-04-15  9:57 ` [PATCH 3/4] Documentation: stop depending on Perl to massage user manual Patrick Steinhardt
2025-04-15  9:57 ` [PATCH 4/4] Documentation: stop depending on Perl to generate command list Patrick Steinhardt
2025-04-15 16:32   ` Junio C Hamano
2025-04-16 12:16 ` [PATCH v2 0/4] Drop Perl dependency in a couple of subsystems Patrick Steinhardt
2025-04-16 12:16   ` [PATCH v2 1/4] filter-branch: stop depending on Perl Patrick Steinhardt
2025-04-16 12:16   ` [PATCH v2 2/4] request-pull: " Patrick Steinhardt
2025-04-16 12:16   ` [PATCH v2 3/4] Documentation: stop depending on Perl to massage user manual Patrick Steinhardt
2025-04-16 12:16   ` [PATCH v2 4/4] Documentation: stop depending on Perl to generate command list Patrick Steinhardt

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=Z__Hz3D6d-4v_z7r@pks.im \
    --to=ps@pks$(echo .)im \
    --cc=Johannes.Schindelin@gmx$(echo .)de \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox