From: Johannes Sixt <j6t@kdbg.org>
Date: Fri, 22 May 2026 08:18:55 GMT
Subject: Re: [PATCH v2 02/11] git-gui: return status from choose_repository::pick
Message-ID: <fdf7aa5c-51ba-4e21-8e4a-5c1fdd8336ab@kdbg.org>
In-Reply-To: <20260520202411.108764-3-mlevedahl@gmail.com>

Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> The repository picker (choose_repository::pick) on success always
> returns with the current directory at the root of the selected worktree,
> and with the global variable _gitdir holding the name of the git
> repository, possibly as a relative path. On failure, _gitdir = {}. If
> the selection was from the "recent" list, no validation has occurred.
> 
> There are too many side effects in this interface. Note that the picker
> only supports worktrees with a .git entry in the worktree root, so git
> repository and worktree discovery will work starting in the current
> directory on return. So, let's change pick to return a 0/1 value, 1
> meaning a worktreee + repo was selected and the current directory is the
> worktree root, and leave validation and setting of _gitdir,
> _gitworktree, and _prefix to the caller.

While the removal of side-effects from the picker is very much desired,
the new return value sounds over-engineered at this point, in particular
due to this note:

> Note: pick actually does not
> return if something was not selected, rather it terminates git-gui.
> But, let's pretend at the call site that pick returns 0/false instead.

If we need the return value later, let's postpone that part of this
commit until then.

> diff --git a/git-gui.sh b/git-gui.sh
> index 4ba25da7b6..4a736190a9 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1151,10 +1151,16 @@ if {[catch {
>  	} err]} {
>  	load_config 1
>  	apply_config
> -	choose_repository::pick
> -	if {![file isdirectory $_gitdir]} {
> +	if {![choose_repository::pick]} {
>  		exit 1
>  	}
> +	if {[catch {
> +		set _gitdir [git rev-parse --git-dir]
> +	} err]} {
> +		catch {wm withdraw .}
> +		error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]

There's something wrong with the quotes here, and an 'exit 1' is missing.

> +	}
> +	set _prefix {}
>  	set picked 1
>  }
-- Hannes