Re: [RFC PATCH 2/4] rename all *_sha1 variables and make null_oid hash aware

From: Pratyush Yadav <me@yadavpratyush.com>
Date: Nov 13, 2021, 06:54 UTC
Message-ID: <20211113065406.z2lqhvh24jjaqty6@yadavpratyush.com>
In-Reply-To: <20211011121757.627-3-carenas@gmail.com>

On 11/10/21 05:17AM, Carlo Marcelo Arenas Belón wrote:

Show 20 quoted lines

> Before this change, creating a branch in an SHA-256 repository would
> fail because the null_sha1 used was of the wrong size.
> 
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  git-gui.sh          | 26 +++++++++++++++-----------
>  lib/blame.tcl       | 10 +++++-----
>  lib/checkout_op.tcl |  4 ++--
>  3 files changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index a69b0fe..c0dc8ce 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1820,10 +1820,14 @@ proc short_path {path} {
>  }
>  
>  set next_icon_id 0
> -set null_sha1 [string repeat 0 40]
> +if { [get_config extensions.objectformat] eq "sha256" } {

From the docs I see that this feature is experimental as of now and might change in the future. Can we expect this config option to stay stable over time? If not I think this might be too early to introduce it into git-gui.

Anyway, nitpick: don't add spaces after opening brace and before closing brace.

Show 11 quoted lines

> +	set null_oid [string repeat 0 64]
> +} else {
> +	set null_oid [string repeat 0 40]
> +}
>  
>  proc merge_state {path new_state {head_info {}} {index_info {}}} {
> -	global file_states next_icon_id null_sha1
> +	global file_states next_icon_id null_oid
>  
>  	set s0 [string index $new_state 0]
>  	set s1 [string index $new_state 1]

Rest of the patch looks good to me. Thanks.

-- 
Regards,
Pratyush Yadav