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

[Johannes Sixt <j6t@kdbg.org>]

Am 20.03.25 um 16:41 schrieb Takashi Iwai:
> 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.

> 
> 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>
> ---

> @@ -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.

> @@ -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]