Skip to contentSkip to conversation

git-gui: add possibility to open currently selected file

PATCH In review Zoli Szabó via GitGitGadget 4 participants 14 messages Dec 26, 2019 – Dec 30, 2019

lore
Share & tools
lore
#0 lore raw

cover git-gui: add possibility to open currently selected file

...in the default associated app (e.g. in a text editor / IDE).

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:

* Diff header path context menu -> Open;
* or double-clicking the diff header path.

One "downside" of the approach is that executable files will be run and not opened for editing.

Zoli Szabó (1):
git-gui: add possibility to open currently selected file

 git-gui.sh | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

base-commit: 23cbe427c44645a3ab0449919e55bade5eb264bc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-499%2Fzoliszabo%2Fgit-gui%2Fopen-current-file-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-499/zoliszabo/git-gui/open-current-file-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/499

Signature · 1 line
gitgitgadget
#1 lore raw

PATCH 1/1 git-gui: add possibility to open currently selected file

From: =?UTF-8?q?Zoli=20Szab=C3=B3?= <[email protected]>

...in the default associated app (e.g. in a text editor / IDE).

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:
- Diff header path context menu -> Open;
- or double-clicking the diff header path.

One "downside" of the approach is that executable files will be run and not opened for editing.

---
 git-gui.sh | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)
git-gui.sh +24 -2
index c1be733e3e..705b97f7ab 100755
@@ -2248,8 +2248,8 @@ proc do_git_gui {} {
 	}
 }
 
-proc do_explore {} {
-	global _gitworktree
+# Get the system-specific explorer app/command.
+proc get_explorer {} {
 	set explorer {}
 	if {[is_Cygwin] || [is_Windows]} {
 		set explorer "explorer.exe"
@@ -2259,9 +2259,25 @@ proc do_explore {} {
 		# freedesktop.org-conforming system is our best shot
 		set explorer "xdg-open"
 	}
+	return $explorer
+}
+
+proc do_explore {} {
+	global _gitworktree
+	set explorer [get_explorer]
 	eval exec $explorer [list [file nativename $_gitworktree]] &
 }
 
+# Trigger opening a file (relative to the working tree) by the default
+# associated app of the OS (e.g. a text editor or IDE).
+# FIXME: What about executables (will be run, not opened for editing)?
+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]] &
+}
+
 set is_quitting 0
 set ret_code    1
 
@@ -3530,8 +3546,14 @@ $ctxm add command \
 			-type STRING \
 			-- $current_diff_path
 	}
+$ctxm add command \
+	-label [mc Open] \
+	-command {
+		do_file_open $current_diff_path
+	}
 lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
 bind_button3 .vpane.lower.diff.header.path "tk_popup $ctxm %X %Y"
+bind .vpane.lower.diff.header.path <Double-1> {do_file_open $current_diff_path}
 
 # -- Diff Body
 #
Signature · 1 line
gitgitgadget
#2 lore raw

Hi Zoli,

Thanks for the patch.

Quoted 10 lines · Zoli Szabó via GitGitGadget

From: =?UTF-8?q?Zoli=20Szab=C3=B3?= <[email protected]>

...in the default associated app (e.g. in a text editor / IDE).

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:
- Diff header path context menu -> Open;

Would it be a better idea to have this option in the diff body context menu (.vpane.lower.diff.body.ctxm) instead? The problem I see with the way its currently done is visibility/discovery. It is not very likely for a user to try and click the file name which doesn't give any indication that it is clickable. So how will someone who hasn't read this commit message know that they can use this neat feature. The diff body context menu is much more "visible" IMO.

Quoted 1 line

- or double-clicking the diff header path.

An alternative to the above suggestion would be to make this path underlined and blue in color (like a hyperlink in a web browser). This will give the indication that this is not just plain text.

I like the latter idea more, but I don't mind either.

Quoted 2 lines

One "downside" of the approach is that executable files will be run and not opened for editing.

FWIW, I do not see it as a downside at all. The menu option is called "open" not "edit". So if you click it, you should expect the file to open. In case its a binary file, executing it is the correct outcome. In case its a text file, opening it in the editor is the correct outcome.

Quoted 10 lines
---
 git-gui.sh | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)
git-gui.sh +0 -0
index c1be733e3e..705b97f7ab 100755

git-gui.sh proc do_git_gui {} { · line 2253

Quoted 8 lines
git-gui.sh +2 -2
@@ -2248,8 +2248,8 @@ proc do_git_gui {} {
 	}
 }
 
-proc do_explore {} {
-	global _gitworktree
+# Get the system-specific explorer app/command.
+proc get_explorer {} {
 	set explorer {}

Not exactly related to this patch, but this line is redundant. No point in clearing 'explorer' only to set it in the very next if-else chain.

Quoted 7 lines
git-gui.sh +1 -0
 	if {[is_Cygwin] || [is_Windows]} {
 		set explorer "explorer.exe"
@@ -2259,9 +2259,25 @@ proc do_explore {} {
 		# freedesktop.org-conforming system is our best shot
 		set explorer "xdg-open"
 	}
+	return $explorer

git-gui.sh proc do_explore {} { · line 2272

Quoted 10 lines
git-gui.sh +7 -0
+}
+
+proc do_explore {} {
+	global _gitworktree
+	set explorer [get_explorer]
 	eval exec $explorer [list [file nativename $_gitworktree]] &
 }
 
+# Trigger opening a file (relative to the working tree) by the default
+# associated app of the OS (e.g. a text editor or IDE).

Nitpick: This comment doesn't really add a whole lot of information. The procedure name already make it pretty clear that it will open a file. And it is pretty easy to understand from reading the body of the procedure that it will be relative to the working tree. So, I think it can be removed altogether.

But even if you think it should stay, please at least make it more concise. Maybe something like:

Open file relative to the working tree by the default associated app.

git-gui.sh proc do_explore {} { · line 2273

Quoted 1 line
git-gui.sh +1 -0
+# FIXME: What about executables (will be run, not opened for editing)?

Again, I think running the executables is the correct behaviour. So I don't think this needs any fixing.

git-gui.sh proc do_explore {} { · line 2278

Quoted 5 lines
git-gui.sh +5 -0
+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]] &

This executes $explorer, which is 'explorer.exe' on Windows. I'm not a heavy Windows user but AFAIK it is a file manager. This makes it quite different from 'xdg-open' which is used to open _any_ file/URL in the user's default application. So it also happens to open _directories_ in the default file explorer which was the original intention of this procedure.

Have you tested it on Windows? Does 'explorer.exe' do the correct thing?

Looking at MacOS's 'open' man page, I think it should also work like xdg-open and shouldn't be a problem.

Quoted 6 lines
git-gui.sh +2 -0
+}
+
 set is_quitting 0
 set ret_code    1
 

git-gui.sh $ctxm add command \ · line 3553

Quoted 8 lines
git-gui.sh +5 -0
@@ -3530,8 +3546,14 @@ $ctxm add command \
 			-type STRING \
 			-- $current_diff_path
 	}
+$ctxm add command \
+	-label [mc Open] \
+	-command {
+		do_file_open $current_diff_path
+	}

Nitpick: no need to make the command multi-line. So, change it to something like:

-command {do_file_open $current_diff_path}

git-gui.sh $ctxm add command \ · line 3559

Quoted 6 lines
git-gui.sh +1 -0
 lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
 bind_button3 .vpane.lower.diff.header.path "tk_popup $ctxm %X %Y"
+bind .vpane.lower.diff.header.path <Double-1> {do_file_open $current_diff_path}
 
 # -- Diff Body
 #

Tested on Linux. Works fine. Looking forward to the re-roll.

Signature · 2 lines
Regards,
Pratyush Yadav
#3 lore raw
Quoted 8 lines · Pratyush Yadav

Have you tested it on Windows? Does 'explorer.exe' do the correct thing?

Looking at MacOS's 'open' man page, I think it should also work like xdg-open and shouldn't be a problem.
...
Tested on Linux. Works fine. Looking forward to the re-roll.

Subject: Re: [PATCH 1/1] git-gui: add possibility to open currently selected file

The phrasing on the title is a bit awkward. "add possibility to do X" is better spelled "allow doing X", no?

#7 lore raw
In reply to Pratyush Yadav

Hi Pratyush,

Thanks for your thorough review.

Quoted 11 lines · Pratyush Yadav
Quoted 2 lines

...This change enables just that by:
- Diff header path context menu -> Open;

Would it be a better idea to have this option in the diff body context menu (.vpane.lower.diff.body.ctxm) instead? The problem I see with the way its currently done is visibility/discovery. It is not very likely for a user to try and click the file name which doesn't give any indication that it is clickable. So how will someone who hasn't read this commit message know that they can use this neat feature. The diff body context menu is much more "visible" IMO.

Quoted 7 lines
Quoted 1 line

- or double-clicking the diff header path.

An alternative to the above suggestion would be to make this path underlined and blue in color (like a hyperlink in a web browser). This will give the indication that this is not just plain text.

I like the latter idea more, but I don't mind either.

For me, the body context menu holds the diff-related options, I am not sure the "Open" fits in there. But I do like your suggestion of making the filename web browser-link-like. I'll try to implement that.

Quoted 7 lines
Quoted 2 lines

One "downside" of the approach is that executable files will be run and not opened for editing.

FWIW, I do not see it as a downside at all. The menu option is called "open" not "edit". So if you click it, you should expect the file to open. In case its a binary file, executing it is the correct outcome. In case its a text file, opening it in the editor is the correct outcome.

Alright then.

Quoted 13 lines

git-gui.sh proc do_explore {} { · line 2278

Quoted 5 lines
git-gui.sh +5 -0
+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]] &

This executes $explorer, which is 'explorer.exe' on Windows. I'm not a heavy Windows user but AFAIK it is a file manager. This makes it quite different from 'xdg-open' which is used to open _any_ file/URL in the user's default application. So it also happens to open _directories_ in the default file explorer which was the original intention of this procedure.

Quoted 4 lines

Have you tested it on Windows? Does 'explorer.exe' do the correct thing?

Looking at MacOS's 'open' man page, I think it should also work like xdg-open and shouldn't be a problem.

I am in fact working on this patch on Windows. explorer.exe works also for files and even brings up the "Open with..." dialog for not-recognized file types.

Quoted 1 line

Tested on Linux. Works fine. Looking forward to the re-roll.

Thanks for testing it on Linux. Since I am working on Windows, I just read about xdg-open and open and assumed everything should work OK.

All other points I agree with you and will push a new version of the patch.

Thanks,
Zoli

#9 lore raw

PATCH v2 1/1 git-gui: allow opening currently selected file

In reply to Zoli Szabó via GitGitGadget

From: =?UTF-8?q?Zoli=20Szab=C3=B3?= <[email protected]>

...in the default associated app (e.g. in a text editor / IDE).

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;

Note: executable files will be run and not opened for editing.

---
 git-gui.sh | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)
git-gui.sh +24 -5
index c1be733e3e..8920e4ddb0 100755
@@ -2248,9 +2248,8 @@ proc do_git_gui {} {
 	}
 }
 
-proc do_explore {} {
-	global _gitworktree
-	set explorer {}
+# Get the system-specific explorer app/command.
+proc get_explorer {} {
 	if {[is_Cygwin] || [is_Windows]} {
 		set explorer "explorer.exe"
 	} elseif {[is_MacOSX]} {
@@ -2259,9 +2258,23 @@ proc do_explore {} {
 		# freedesktop.org-conforming system is our best shot
 		set explorer "xdg-open"
 	}
+	return $explorer
+}
+
+proc do_explore {} {
+	global _gitworktree
+	set explorer [get_explorer]
 	eval exec $explorer [list [file nativename $_gitworktree]] &
 }
 
+# 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]] &
+}
+
 set is_quitting 0
 set ret_code    1
 
@@ -3513,9 +3526,11 @@ tlabel .vpane.lower.diff.header.file \
 	-justify left
 tlabel .vpane.lower.diff.header.path \
 	-background gold \
-	-foreground black \
+	-foreground blue \
 	-anchor w \
-	-justify left
+	-justify left \
+	-font [eval font create [font configure font_ui] -underline 1] \
+	-cursor hand2
 pack .vpane.lower.diff.header.status -side left
 pack .vpane.lower.diff.header.file -side left
 pack .vpane.lower.diff.header.path -fill x
@@ -3530,8 +3545,12 @@ $ctxm add command \
 			-type STRING \
 			-- $current_diff_path
 	}
+$ctxm add command \
+	-label [mc Open] \
+	-command {do_file_open $current_diff_path}
 lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
 bind_button3 .vpane.lower.diff.header.path "tk_popup $ctxm %X %Y"
+bind .vpane.lower.diff.header.path <Button-1> {do_file_open $current_diff_path}
 
 # -- Diff Body
 #
Signature · 1 line
gitgitgadget
#8 lore raw

v2 cover git-gui: allow opening currently selected file

In reply to Zoli Szabó via GitGitGadget

...in the default associated app (e.g. in a text editor / IDE).

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;

Note: executable files will be run and not opened for editing.

Zoli Szabó (1):
git-gui: allow opening currently selected file

 git-gui.sh | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

base-commit: 23cbe427c44645a3ab0449919e55bade5eb264bc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-499%2Fzoliszabo%2Fgit-gui%2Fopen-current-file-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-499/zoliszabo/git-gui/open-current-file-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/499

Range-diff vs v1:

1: fce80f1b95 ! 1: a6fde256f8 git-gui: add possibility to open currently selected file
@@ -1,6 +1,6 @@
Author: Zoli Szabó <[email protected]>

- git-gui: add possibility to open currently selected file
+ git-gui: allow opening currently selected file

...in the default associated app (e.g. in a text editor / IDE).

@@ -9,11 +9,10 @@
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:
- - Diff header path context menu -> Open;
- - or double-clicking the diff header path.
+ - clicking the diff header path (which is now highlighted as a hyperlink)
+ - or diff header path context menu -> Open;

- One "downside" of the approach is that executable files will be run
- and not opened for editing.
+ Note: executable files will be run and not opened for editing.

@@ -26,11 +25,12 @@

-proc do_explore {} {
- global _gitworktree
+- set explorer {}
+# Get the system-specific explorer app/command. +proc get_explorer {} {
- set explorer {}
if {[is_Cygwin] || [is_Windows]} {
set explorer "explorer.exe"
+ } elseif {[is_MacOSX]} {
@@
# freedesktop.org-conforming system is our best shot set explorer "xdg-open"
@@ -44,9 +44,7 @@
eval exec $explorer [list [file nativename $_gitworktree]] &
}

-+# Trigger opening a file (relative to the working tree) by the default -+# associated app of the OS (e.g. a text editor or IDE). -+# FIXME: What about executables (will be run, not opened for editing)? ++# Open file relative to the working tree by the default associated app.
+proc do_file_open {file} {
+ global _gitworktree
+ set explorer [get_explorer]
@@ -57,18 +55,30 @@
set is_quitting 0
set ret_code 1

+@@
+ -justify left
+ tlabel .vpane.lower.diff.header.path \
+ -background gold \
+- -foreground black \
++ -foreground blue \
+ -anchor w \
+- -justify left
++ -justify left \
++ -font [eval font create [font configure font_ui] -underline 1] \
++ -cursor hand2
+ pack .vpane.lower.diff.header.status -side left
+ pack .vpane.lower.diff.header.file -side left
+ pack .vpane.lower.diff.header.path -fill x
@@
-type STRING \
-- $current_diff_path
}
+$ctxm add command \
+ -label [mc Open] \
-+ -command {
-+ do_file_open $current_diff_path
-+ }
++ -command {do_file_open $current_diff_path}
lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state] bind_button3 .vpane.lower.diff.header.path "tk_popup $ctxm %X %Y"
-+bind .vpane.lower.diff.header.path <Double-1> {do_file_open $current_diff_path} ++bind .vpane.lower.diff.header.path <Button-1> {do_file_open $current_diff_path}

# -- Diff Body
#

Signature · 1 line
gitgitgadget
#4 lore raw
In reply to Junio C Hamano
Quoted 2 lines · Junio C Hamano

The phrasing on the title is a bit awkward. "add possibility to do X" is better spelled "allow doing X", no?

Thank you, Junio, for the hint. Updated the commit message accordingly (PATCH v2).

#5 lore raw
Quoted 7 lines · Zoli Szabó
Quoted 2 lines · Junio C Hamano

The phrasing on the title is a bit awkward. "add possibility to do X" is better spelled "allow doing X", no?

Thank you, Junio, for the hint. Updated the commit message accordingly (PATCH v2).

Also, do not start the body of the proposed log message with half sentence. The title is supposed to be a full sentence.

#10 lore raw

v3 cover git-gui: allow opening currently selected file

In reply to Zoli Szabó via GitGitGadget

...in the default associated app (e.g. in a text editor / IDE).

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;

Note: executable files will be run and not opened for editing.

Zoli Szabó (1):
git-gui: allow opening currently selected file in default app

 git-gui.sh | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

base-commit: 23cbe427c44645a3ab0449919e55bade5eb264bc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-499%2Fzoliszabo%2Fgit-gui%2Fopen-current-file-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-499/zoliszabo/git-gui/open-current-file-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/499

Range-diff vs v2:

1: a6fde256f8 ! 1: 1b2363be72 git-gui: allow opening currently selected file
@@ -1,8 +1,6 @@
Author: Zoli Szabó <[email protected]>

- git-gui: allow opening currently selected file
-
- ...in the default associated app (e.g. in a text editor / IDE).
+ git-gui: allow opening currently selected file in default app

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,

Signature · 1 line
gitgitgadget
#11 lore raw

PATCH v3 1/1 git-gui: allow opening currently selected file in default app

From: =?UTF-8?q?Zoli=20Szab=C3=B3?= <[email protected]>

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;

Note: executable files will be run and not opened for editing.

---
 git-gui.sh | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)
git-gui.sh +24 -5
index c1be733e3e..8920e4ddb0 100755
@@ -2248,9 +2248,8 @@ proc do_git_gui {} {
 	}
 }
 
-proc do_explore {} {
-	global _gitworktree
-	set explorer {}
+# Get the system-specific explorer app/command.
+proc get_explorer {} {
 	if {[is_Cygwin] || [is_Windows]} {
 		set explorer "explorer.exe"
 	} elseif {[is_MacOSX]} {
@@ -2259,9 +2258,23 @@ proc do_explore {} {
 		# freedesktop.org-conforming system is our best shot
 		set explorer "xdg-open"
 	}
+	return $explorer
+}
+
+proc do_explore {} {
+	global _gitworktree
+	set explorer [get_explorer]
 	eval exec $explorer [list [file nativename $_gitworktree]] &
 }
 
+# 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]] &
+}
+
 set is_quitting 0
 set ret_code    1
 
@@ -3513,9 +3526,11 @@ tlabel .vpane.lower.diff.header.file \
 	-justify left
 tlabel .vpane.lower.diff.header.path \
 	-background gold \
-	-foreground black \
+	-foreground blue \
 	-anchor w \
-	-justify left
+	-justify left \
+	-font [eval font create [font configure font_ui] -underline 1] \
+	-cursor hand2
 pack .vpane.lower.diff.header.status -side left
 pack .vpane.lower.diff.header.file -side left
 pack .vpane.lower.diff.header.path -fill x
@@ -3530,8 +3545,12 @@ $ctxm add command \
 			-type STRING \
 			-- $current_diff_path
 	}
+$ctxm add command \
+	-label [mc Open] \
+	-command {do_file_open $current_diff_path}
 lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
 bind_button3 .vpane.lower.diff.header.path "tk_popup $ctxm %X %Y"
+bind .vpane.lower.diff.header.path <Button-1> {do_file_open $current_diff_path}
 
 # -- Diff Body
 #
Signature · 1 line
gitgitgadget
#6 lore raw
In reply to Junio C Hamano

Adapted the commit message once again (PATCH v3).

Quoted 15 lines · Junio C Hamano
Quoted 7 lines · Zoli Szabó
Quoted 2 lines · Junio C Hamano

The phrasing on the title is a bit awkward. "add possibility to do X" is better spelled "allow doing X", no?

Thank you, Junio, for the hint. Updated the commit message accordingly (PATCH v2).

Also, do not start the body of the proposed log message with half sentence. The title is supposed to be a full sentence.

#12 lore raw

Re: [PATCH v3 1/1] git-gui: allow opening currently selected file in default app

In reply to Zoli Szabó via GitGitGadget

Hi Zoli,

Quoted 9 lines · Zoli Szabó via GitGitGadget

From: =?UTF-8?q?Zoli=20Szab=C3=B3?= <[email protected]>

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?

Quoted 15 lines

Note: executable files will be run and not opened for editing.

---
 git-gui.sh | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)
git-gui.sh +1 -0
index c1be733e3e..8920e4ddb0 100755
@@ -2259,9 +2258,23 @@ proc do_explore {} {
+# Open file relative to the working tree by the default associated 

app.

git-gui.sh proc do_explore {} { · line 2275

Quoted 5 lines
git-gui.sh +5 -0
+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.

git-gui.sh proc do_explore {} { · line 2279

Quoted 5 lines
git-gui.sh +2 -0
+}
+
 set is_quitting 0
 set ret_code    1

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

Signature · 2 lines
Regards,
Pratyush Yadav
#13 lore raw

Re: Re: [PATCH v3 1/1] git-gui: allow opening currently selected file in default app

Quoted 13 lines · Pratyush Yadav

Hi Zoli,

Quoted 9 lines · Zoli Szabó via GitGitGadget

From: =?UTF-8?q?Zoli=20Szab=C3=B3?= <[email protected]>

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;

Quoted 2 lines

Semi-colon left in by mistake?

Yes, that should have been a simple period (.), signaling the end of the sentence.

Quoted 9 lines

git-gui.sh proc do_explore {} { · line 2275

Quoted 1 line
git-gui.sh +1 -0
+	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.

You're right. It can be simplified like you have proposed. Tested it and works correctly also on Windows. (Since I have copied that line from the existing do_explore proc, the same simplification can be done there too. (I did not realize this myself, since I am just learning some TCL in order to have this feature in Git GUI...))

Quoted 7 lines

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.

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

Thank you very much for your support!

All the best,
Zoli