From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 05 Dec 2025 12:16:46 GMT
Subject: Re: [PATCH v5 2/7] builtin/credential-store: move is_rfc3986_unreserved to url.[ch]
Message-ID: <aTLNLnF_ZLpMnso-@pks.im>
In-Reply-To: <20251119211030.2008441-3-adrian.ratiu@collabora.com>

On Wed, Nov 19, 2025 at 11:10:25PM +0200, Adrian Ratiu wrote:
> is_rfc3986_unreserved() was moved to credential-store.c and was made
> static by f89854362c (credential-store: move related functions to
> credential-store file, 2023-06-06) under a correct assumption, at the
> time, that it was the only place using it.
> 
> However now we need it to apply URL-encoding to submodule names when
> constructing gitdir paths, to avoid conflicts, so bring it back as a
> public function exposed via url.h, instead of the old helper path
> (strbuf), which has nothing to do with 3986 encoding/decoding anymore.
> 
> This function will be used by submodule.c in the next commit.

Nit: this statement isn't true anymore :)

> diff --git a/url.c b/url.c
> index 282b12495a..0fb1859b28 100644
> --- a/url.c
> +++ b/url.c
> @@ -3,6 +3,17 @@
>  #include "strbuf.h"
>  #include "url.h"
>  
> +/*
> + * The set of unreserved characters as per STD66 (RFC3986) is
> + * '[A-Za-z0-9-._~]'. These characters are safe to appear in URI
> + * components without percent-encoding.
> + */
> +int is_rfc3986_unreserved(char ch)
> +{
> +	return isalnum(ch) ||
> +		ch == '-' || ch == '_' || ch == '.' || ch == '~';
> +}
> +
>  int is_urlschemechar(int first_flag, int ch)
>  {
>  	/*

Nit: the function documentation should rather live in the header file.

Patrick