From: Pratyush Yadav <me@yadavpratyush.com>
Date: Mon, 30 Dec 2019 19:41:29 GMT
Subject: Re: [PATCH v3 1/1] git-gui: allow opening currently selected file in default app
Message-ID: <20191230194129.kjmp6r5xuwmq4wum@yadavpratyush.com>
In-Reply-To: <1b2363be726c6d70746aec9fae62edaf857cd665.1577721419.git.gitgitgadget@gmail.com>

Hi Zoli,

On 30/12/19 03:56PM, Zoli Szabó via GitGitGadget wrote:
> From: =?UTF-8?q?Zoli=20Szab=C3=B3?= <zoli.szabo@gmail.com>
> 
> Many times there's the need to quickly open a source file (the one you're
> looking at in Git GUI) in the predefined text editor / IDE. Of course,
> the file can be searched for in your preferred file manager or directly
> in the text editor, but having the option to directly open the current
> file from Git GUI would be just faster. This change enables just that by:
>  - clicking the diff header path (which is now highlighted as a hyperlink)
>  - or diff header path context menu -> Open;

Semi-colon left in by mistake?

> 
> Note: executable files will be run and not opened for editing.
> 
> Signed-off-by: Zoli Szabó <zoli.szabo@gmail.com>
> ---
>  git-gui.sh | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index c1be733e3e..8920e4ddb0 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2259,9 +2258,23 @@ proc do_explore {} {
> +# Open file relative to the working tree by the default associated 
> app.
> +proc do_file_open {file} {
> +	global _gitworktree
> +	set explorer [get_explorer]
> +	set full_file_path [file join $_gitworktree $file]
> +	eval exec $explorer [list [file nativename $full_file_path]] &

IIUC, this line can be simplified to:

  exec $explorer [file nativename $full_file_path] &

It works fine for me including on files with spaces in their names, but 
a test on Windows would be appreciated just to rule out any hidden 
surprises.

No need to send a re-roll just for these two small things. I have 
updated the commit locally before pushing the new version out [0]. The 
rest of the patch looks good. Will merge. Thanks.

> +}
> +
>  set is_quitting 0
>  set ret_code    1
>  

[0] https://github.com/prati0100/git-gui/tree/zs/open-current-file

-- 
Regards,
Pratyush Yadav