Re: [PATCH v13 2/9] refs: standardize output of refs_read_symbolic_ref

From: Patrick Steinhardt <ps@pks.im>
Date: Nov 19, 2024, 06:48 UTC
Message-ID: <Zzw0ooX0HFwSit7v@pks.im>
In-Reply-To: <20241118151755.756265-3-bence@ferdinandy.com>

On Mon, Nov 18, 2024 at 04:09:21PM +0100, Bence Ferdinandy wrote:

Show 14 quoted lines

> diff --git a/refs.h b/refs.h
> index 108dfc93b3..f8b714ca1d 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -83,6 +83,12 @@ int refs_read_ref_full(struct ref_store *refs, const char *refname,
>  
>  int refs_read_ref(struct ref_store *refs, const char *refname, struct object_id *oid);
>  
> +/*
> + * Return 0 if the symbolic reference could be read without error.
> + * Return -1 for generic errors.
> + * Return -2 if the reference was actually non-symbolic.
> + */
> +

Extraneous empty newline.

Also, how about the following:

    /*
     * Read the symbolic ref named "refname" and write its immediate
     * referent into the provided buffer. This does not resolve the
     * symbolic ref recursively in case the target is a symbolic ref, as
     * well.
     *
     * Returns 0 on success, -2 if the "refname" is not a symbolic ref,
     * -1 otherwise.
     */

Show 15 quoted lines

> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 2313c830d8..f0ef354bce 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -673,6 +673,12 @@ struct ref_storage_be {
>  
>  	ref_iterator_begin_fn *iterator_begin;
>  	read_raw_ref_fn *read_raw_ref;
> +
> +	/*
> +	 * Return 0 if the symbolic reference could be read without error.
> +	 * Return -1 for generic errors.
> +	 * Return -2 if the reference was actually non-symbolic.
> +	 */
>  	read_symbolic_ref_fn *read_symbolic_ref;

As proposed in the other thread, this could instead be:

    /*
     * Please refer to `refs_read_symbolic_ref()` for the expected
     * behaviour.
     /

Patrick