Re: [PATCH 1/2] gitk: Add a basic support of SHA256 repositories into Gitk

From: Johannes Sixt <j6t@kdbg.org>
Date: May 8, 2025, 06:20 UTC
Message-ID: <9bb1699a-ec99-40b3-bd6a-00e809d77d0d@kdbg.org>
In-Reply-To: <20250320154136.23262-2-tiwai@suse.de>

Am 20.03.25 um 16:41 schrieb Takashi Iwai:

Show 22 quoted lines

> From: Rostislav Krasny <rosti.bsd@gmail.com>
> 
> This PR makes Gitk working on both SHA256 and SHA1 repositories without
> errors/crashes. I made it by changing and testing the gitk script of Git
> for Windows [https://gitforwindows.org/] version 2.32.0.windows.1 that
> is a little bit different than the mainstream 2.32.0 version.
> 
> Still not fixed functionality: [1] There is the "Auto-select SHA1
> (length)" configuration preference that affects "Copy commit reference"
> on both SHA1 and SHA256 repositories.
> 
> A new "Auto-select SHA256 (length)" configuration preference should be
> added and used on SHA256 repositories instead of the old one. Since I'm
> not familiar with Tcl/Tk and this issue isn't critical I didn't
> implement it.
> 
> [ Changes from the original patch:
>   * Discard the changes for generic words (e.g. "Commit ID"), so that
>     translations can be still applied after this patch
>   * Simplify the regexp check in gotocommit as suggested in the
>     previous review
>   -- tiwai ]

The message should be updated to not mention the evolution of the change and what is not relevant anymore or not relevant in this patch.

Show 5 quoted lines

> 
> Signed-off-by: Rostislav Krasny <rosti.bsd@gmail.com>
> Link: https://patchwork.kernel.org/project/git/patch/pull.979.git.1623687519832.gitgitgadget@gmail.com
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---

Show 6 quoted lines

> @@ -8920,11 +8932,11 @@ proc gotocommit {} {
>          set id $headids($sha1string)
>      } else {
>          set id [string tolower $sha1string]
> -        if {[regexp {^[0-9a-f]{4,39}$} $id]} {
> +	if {[regexp {^[0-9a-f]{4,63}$} $id]} {

This doesn't use $hashlength. Should it?

Also watch out space vs. TAB.

Show 11 quoted lines

> @@ -12524,6 +12539,18 @@ if {$tclencoding == {}} {
>      puts stderr "Warning: encoding $gitencoding is not supported by Tcl/Tk"
>  }
>  
> +set objformat [exec git rev-parse --show-object-format]
> +if {$objformat eq "sha1"} {
> +    set hashlength 40
> +} elseif {$objformat eq "sha256"} {
> +    set hashlength 64
> +} else {
> +    error_popup "[mc "Not supported hash algorithm:"] {$objformat}"

This looks strange. Where is the $objformat substituted?

> +    exit 1
> +}
> +set hashalgorithm [string toupper $objformat]
> +unset objformat

Why not set hashalgorithm right away, without using a temporary objformat? Why set it at all here? It's unused.

> +
>  set gui_encoding [encoding system]
>  catch {
>      set enc [exec git config --get gui.encoding]