threads / patch / 22779

patchgit-gui: fix open explorer window on Windows 7

Subject: [GIT GUI PATCH] git-gui: fix open explorer window on Windows 7

## tl;dr

13 messages between Feb 23, 2010 and Feb 27, 2010. Diffs are folded; open one to read it.

replies: 12people: 5as markdown or json

Heiko Voigt· Feb 23, 2010, 22:52 UTC · lore

It seems that Windows 7's explorer is not capable to cope with paths that contain forward slashes as path seperator. We thus substitute slash with the platforms native backslash.

Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
---
 git-gui/git-gui.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
Show changes to git-gui/git-gui.sh +3 −1
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 9a9525d..ae45a12 100644
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -2112,15 +2112,17 @@ proc do_git_gui {} {
 proc do_explore {} {
 	global _gitworktree
 	set explorer {}
+	set path $_gitworktree
 	if {[is_Cygwin] || [is_Windows]} {
 		set explorer "explorer.exe"
+		set path [list [string map {/ \\} $path]]
 	} elseif {[is_MacOSX]} {
 		set explorer "open"
 	} else {
 		# freedesktop.org-conforming system is our best shot
 		set explorer "xdg-open"
 	}
-	eval exec $explorer $_gitworktree &
+	eval exec $explorer $path &
 }
 
 set is_quitting 0

-- 
1.7.0.m5.rc2.15.g8ba4c
Markus Heidelberg· Feb 24, 2010, 00:14 UTC · re: Heiko Voigt · lore

Re: [GIT GUI PATCH] git-gui: fix open explorer window on Windows 7

Heiko Voigt, 2010-02-23 23:52:
Show 21 quoted lines
> It seems that Windows 7's explorer is not capable to cope with paths
> that contain forward slashes as path seperator. We thus substitute slash
> with the platforms native backslash.
> 
> Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
> ---
>  git-gui/git-gui.sh |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 9a9525d..ae45a12 100644
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -2112,15 +2112,17 @@ proc do_git_gui {} {
>  proc do_explore {} {
>  	global _gitworktree
>  	set explorer {}
> +	set path $_gitworktree
>  	if {[is_Cygwin] || [is_Windows]} {
>  		set explorer "explorer.exe"
> +		set path [list [string map {/ \\} $path]]

I remember having had the same problem, which I fixed with [file nativename]. See commit 454efb47b (git-gui (Win): make "Explore Working Copy" more robust). But that was on XP, shouldn't be Windows 7 specific. So why does it fail now again, although I had fixed it? ...

Show 8 quoted lines
>  	} elseif {[is_MacOSX]} {
>  		set explorer "open"
>  	} else {
>  		# freedesktop.org-conforming system is our best shot
>  		set explorer "xdg-open"
>  	}
> -	eval exec $explorer $_gitworktree &
> +	eval exec $explorer $path &

Ah, this doesn't look like what it looked like back then. Commit 21985a113 (git-gui: handle non-standard worktree locations) changed it, removing the [file nativename]. I don't know if this was by accident, but there is another place, where it was changed in a different manner:

Compare
-       eval exec $explorer [list [file nativename [file dirname [gitdir]]]] &
+       eval exec $explorer $_gitworktree &
with

-wm title . "[appname] ([reponame]) [file normalize [file dirname [gitdir]]]" +wm title . "[appname] ([reponame]) [file normalize $_gitworktree]"

Maybe _gitworktree should be normalized or nativenamified when it is set and then will work everywhere it is used for every platform?

Markus
Giuseppe Bilotta· Feb 24, 2010, 06:22 UTC · re: Markus Heidelberg · lore

Re: [GIT GUI PATCH] git-gui: fix open explorer window on Windows 7

On Wed, Feb 24, 2010 at 1:14 AM, Markus Heidelberg <markus.heidelberg@web.de> wrote:

Show 5 quoted lines
>
> Ah, this doesn't look like what it looked like back then. Commit
> 21985a113 (git-gui: handle non-standard worktree locations) changed it,
> removing the [file nativename]. I don't know if this was by accident,
> but there is another place, where it was changed in a different manner:
My fault, and yes it was by accident.
Show 12 quoted lines
> Compare
>
> -       eval exec $explorer [list [file nativename [file dirname [gitdir]]]] &
> +       eval exec $explorer $_gitworktree &
>
> with
>
> -wm title . "[appname] ([reponame]) [file normalize [file dirname [gitdir]]]"
> +wm title . "[appname] ([reponame]) [file normalize $_gitworktree]"
>
> Maybe _gitworktree should be normalized or nativenamified when it is set
> and then will work everywhere it is used for every platform?

The only issue with that would be that GIT_DIR and GIT_WORK_TREE are (re)inizialized from _gitdir and _gitworktree, so it would not be safe if git on Windows works better with unix-style rather than native paths. If git on windows handles native paths fine, it should be no problem.

-- 
Giuseppe "Oblomov" Bilotta
Pat Thoyts· Feb 24, 2010, 12:12 UTC · re: Giuseppe Bilotta · lore

Re: [msysGit] Re: [GIT GUI PATCH] git-gui: fix open explorer window on Windows 7

On 24 February 2010 06:22, Giuseppe Bilotta <giuseppe.bilotta@gmail.com> wrote:
>+               set path [list [string map {/ \\} $path]]
No - Tcl provides [file nativename $path] for this purpose.
Show 6 quoted lines
>
> The only issue with that would be that GIT_DIR and GIT_WORK_TREE are
> (re)inizialized
> from _gitdir and _gitworktree, so it would not be safe if git on
> Windows works better with unix-style rather than native paths. If git
> on windows handles native paths fine, it should be no problem.

Tcl internally uses unix type path separators but once you export this path to the operating system you may need to ensure it is a native path. Typically that means when calling [exec]. [open] is a tcl command and will deal with a path variable in either style. The Tcl exec man page has some notes on the compatibility issues.

Pat.
Markus Heidelberg· Feb 25, 2010, 00:14 UTC · re: Pat Thoyts · lore

[PATCH] git-gui: fix "Explore Working Copy" for Windows again

It has already been fixed in commit 454efb47 (git-gui (Win): make "Explore Working Copy" more robust, 2009-04-01), but has been broken in commit 21985a11 (git-gui: handle non-standard worktree locations, 2010-01-23) by accidentally replacing too much with a new variable.

The problem can be reproduced when starting git-gui from within a subdirectory. The solution is to convert the path name, explorer.exe is invoked with, to a platform native name.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
Show 14 quoted lines
> > > Maybe _gitworktree should be normalized or nativenamified when it is set
> > > and then will work everywhere it is used for every platform?
> > 
> > The only issue with that would be that GIT_DIR and GIT_WORK_TREE are
> > (re)inizialized
> > from _gitdir and _gitworktree, so it would not be safe if git on
> > Windows works better with unix-style rather than native paths. If git
> > on windows handles native paths fine, it should be no problem.
> 
> Tcl internally uses unix type path separators but once you export this
> path to the operating system you may need to ensure it is a native
> path. Typically that means when calling [exec]. [open] is a tcl
> command and will deal with a path variable in either style. The Tcl
> exec man page has some notes on the compatibility issues.
So this should be the obvious solution.
 git-gui.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to git-gui.sh +1 −1
diff --git a/git-gui.sh b/git-gui.sh
index 8996d2d..7184afa 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2101,7 +2101,7 @@ proc do_explore {} {
 		# freedesktop.org-conforming system is our best shot
 		set explorer "xdg-open"
 	}
-	eval exec $explorer $_gitworktree &
+	eval exec $explorer [list [file nativename $_gitworktree]] &
 }
 
 set is_quitting 0

-- 
1.7.0.52.gb3f79
Heiko Voigt· Feb 25, 2010, 20:44 UTC · re: Markus Heidelberg · lore

Re: [PATCH] git-gui: fix "Explore Working Copy" for Windows again

On Thu, Feb 25, 2010 at 01:14:22AM +0100, Markus Heidelberg wrote:
Show 11 quoted lines
> It has already been fixed in commit 454efb47 (git-gui (Win): make
> "Explore Working Copy" more robust, 2009-04-01), but has been broken in
> commit 21985a11 (git-gui: handle non-standard worktree locations,
> 2010-01-23) by accidentally replacing too much with a new variable.
> 
> The problem can be reproduced when starting git-gui from within a
> subdirectory. The solution is to convert the path name, explorer.exe is
> invoked with, to a platform native name.
> 
> Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
> ---

I can confirm that this does fix the issue which is actually also apparent on Windows XP. If no objections occur I would apply this to the devel branch for msysgit.

cheers Heiko
Pat Thoyts· Feb 25, 2010, 23:00 UTC · re: Heiko Voigt · lore

Re: [PATCH] git-gui: fix "Explore Working Copy" for Windows again

On 25 February 2010 20:44, Heiko Voigt <hvoigt@hvoigt.net> wrote:
Show 18 quoted lines
> On Thu, Feb 25, 2010 at 01:14:22AM +0100, Markus Heidelberg wrote:
>> It has already been fixed in commit 454efb47 (git-gui (Win): make
>> "Explore Working Copy" more robust, 2009-04-01), but has been broken in
>> commit 21985a11 (git-gui: handle non-standard worktree locations,
>> 2010-01-23) by accidentally replacing too much with a new variable.
>>
>> The problem can be reproduced when starting git-gui from within a
>> subdirectory. The solution is to convert the path name, explorer.exe is
>> invoked with, to a platform native name.
>>
>> Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
>> ---
>
> I can confirm that this does fix the issue which is actually also
> apparent on Windows XP. If no objections occur I would apply this to the
> devel branch for msysgit.
>
> cheers Heiko

Agreed - the loss of the [file nativename] around the path is the fault. Replacement with eval exec $explorer [list [file nativename $_gitworktree]] & will be sufficient for all platforms (the [list] protects any spaces in the path against the eval).

Pat Thoyts
Johannes Schindelin· Feb 26, 2010, 00:46 UTC · re: Heiko Voigt · lore

Re: [PATCH] git-gui: fix "Explore Working Copy" for Windows again

Hi,
On Thu, 25 Feb 2010, Heiko Voigt wrote:
Show 16 quoted lines
> On Thu, Feb 25, 2010 at 01:14:22AM +0100, Markus Heidelberg wrote:
> > It has already been fixed in commit 454efb47 (git-gui (Win): make
> > "Explore Working Copy" more robust, 2009-04-01), but has been broken in
> > commit 21985a11 (git-gui: handle non-standard worktree locations,
> > 2010-01-23) by accidentally replacing too much with a new variable.
> > 
> > The problem can be reproduced when starting git-gui from within a
> > subdirectory. The solution is to convert the path name, explorer.exe is
> > invoked with, to a platform native name.
> > 
> > Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
> > ---
> 
> I can confirm that this does fix the issue which is actually also 
> apparent on Windows XP. If no objections occur I would apply this to the 
> devel branch for msysgit.
No objections from my side!

Thanks, Dscho

Heiko Voigt· Feb 27, 2010, 21:14 UTC · re: Johannes Schindelin · lore

Re: Re: [PATCH] git-gui: fix "Explore Working Copy" for Windows again

On Fri, Feb 26, 2010 at 01:46:05AM +0100, Johannes Schindelin wrote:
Show 20 quoted lines
> On Thu, 25 Feb 2010, Heiko Voigt wrote:
> 
> > On Thu, Feb 25, 2010 at 01:14:22AM +0100, Markus Heidelberg wrote:
> > > It has already been fixed in commit 454efb47 (git-gui (Win): make
> > > "Explore Working Copy" more robust, 2009-04-01), but has been broken in
> > > commit 21985a11 (git-gui: handle non-standard worktree locations,
> > > 2010-01-23) by accidentally replacing too much with a new variable.
> > > 
> > > The problem can be reproduced when starting git-gui from within a
> > > subdirectory. The solution is to convert the path name, explorer.exe is
> > > invoked with, to a platform native name.
> > > 
> > > Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
> > > ---
> > 
> > I can confirm that this does fix the issue which is actually also 
> > apparent on Windows XP. If no objections occur I would apply this to the 
> > devel branch for msysgit.
> 
> No objections from my side!
Done.
cheers Heiko
Pat Thoyts· Feb 24, 2010, 12:15 UTC · re: Heiko Voigt · lore

Re: [msysGit] [GIT GUI PATCH] git-gui: fix open explorer window on Windows 7

On 23 February 2010 22:52, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> It seems that Windows 7's explorer is not capable to cope with paths
> that contain forward slashes as path seperator. We thus substitute slash
> with the platforms native backslash.
>

What bug are you actually addressing here? How can I reproduce it? My experience on Windows 7 is that there is no problem but I assume I'm doing a different operation. If I use the git-gui "Explore working copy" it opens the Windows explorer for me just fine.

Heiko Voigt· Feb 25, 2010, 20:30 UTC · re: Pat Thoyts · lore

Re: Re: [GIT GUI PATCH] git-gui: fix open explorer window on Windows 7

On Wed, Feb 24, 2010 at 12:15:03PM +0000, Pat Thoyts wrote:
Show 10 quoted lines
> On 23 February 2010 22:52, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > It seems that Windows 7's explorer is not capable to cope with paths
> > that contain forward slashes as path seperator. We thus substitute slash
> > with the platforms native backslash.
> >
> 
> What bug are you actually addressing here? How can I reproduce it? My
> experience on Windows 7 is that there is no problem but I assume I'm
> doing a different operation. If I use the git-gui  "Explore working
> copy" it opens the Windows explorer for me just fine.

Very strange, again another inconsistency between Windowses? I can reproduce this even on Windows XP. I am using the current devel branches of msysgit (msysgit and git). It happens when using the "Repository->Explore working copy" menu item. The Windows 7 I tested this on is 32-bit Professional. Which Windows 7 are you using?

cheers Heiko
Markus Heidelberg· Feb 25, 2010, 22:23 UTC · re: Heiko Voigt · lore

Re: [msysGit] [GIT GUI PATCH] git-gui: fix open explorer window on Windows 7

Heiko Voigt, 2010-02-25 21:30:
Show 17 quoted lines
> On Wed, Feb 24, 2010 at 12:15:03PM +0000, Pat Thoyts wrote:
> > On 23 February 2010 22:52, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > > It seems that Windows 7's explorer is not capable to cope with paths
> > > that contain forward slashes as path seperator. We thus substitute slash
> > > with the platforms native backslash.
> > >
> > 
> > What bug are you actually addressing here? How can I reproduce it? My
> > experience on Windows 7 is that there is no problem but I assume I'm
> > doing a different operation. If I use the git-gui  "Explore working
> > copy" it opens the Windows explorer for me just fine.
> 
> Very strange, again another inconsistency between Windowses? I can
> reproduce this even on Windows XP. I am using the current devel branches
> of msysgit (msysgit and git). It happens when using the
> "Repository->Explore working copy" menu item. The Windows 7 I tested
> this on is 32-bit Professional. Which Windows 7 are you using?

You should tell, how exactly you start git-gui, because this makes the difference in this bug.

Markus
Pat Thoyts· Feb 25, 2010, 22:49 UTC · re: Markus Heidelberg · lore

Re: [msysGit] [GIT GUI PATCH] git-gui: fix open explorer window on Windows 7

On 25 February 2010 22:23, Markus Heidelberg <markus.heidelberg@web.de> wrote:
Show 21 quoted lines
> Heiko Voigt, 2010-02-25 21:30:
>> On Wed, Feb 24, 2010 at 12:15:03PM +0000, Pat Thoyts wrote:
>> > On 23 February 2010 22:52, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>> > > It seems that Windows 7's explorer is not capable to cope with paths
>> > > that contain forward slashes as path seperator. We thus substitute slash
>> > > with the platforms native backslash.
>> > >
>> >
>> > What bug are you actually addressing here? How can I reproduce it? My
>> > experience on Windows 7 is that there is no problem but I assume I'm
>> > doing a different operation. If I use the git-gui  "Explore working
>> > copy" it opens the Windows explorer for me just fine.
>>
>> Very strange, again another inconsistency between Windowses? I can
>> reproduce this even on Windows XP. I am using the current devel branches
>> of msysgit (msysgit and git). It happens when using the
>> "Repository->Explore working copy" menu item. The Windows 7 I tested
>> this on is 32-bit Professional. Which Windows 7 are you using?
>
> You should tell, how exactly you start git-gui, because this makes the
> difference in this bug.

I have Windows 7 64 bit Ultimate - but I doubt the version of Win7 is going to matter here.

In my case, using the current release of msysGit (1.6.5.1.1367.gcd48) can launch the Windows explorer and so can the current devel branch of msysgit. I launch these from the windows cmd shell using the scripts in msysgit\cmd\ for the devel version

But lets be real certain we are runing the script in the msysgit\git\git-gui directory:

C:\src\msysgit\git\git-gui>set PATH=c:\src\msysgit\bin;c:\src\msysgit\mingw\bin;%PATH% C:\src\msysgit\git\git-gui>tclkit86 git-gui.sh

This is now guaranteed running the script mentioned and it uses the 'lib' subfolder from this directory too. Help About says git-gui version @@GITGUI_VERSION@@ and git version 1.7.0.rc2.1441.g8e037 as I built and installed this a few days ago.

However, I notice that it is not actually browsing the working folder when it launches the browser - it has actually opened "My Documents". I presume this is the bug you intend to fix -- and applying your patch confirms that it solves this issue.

Can you please give enough information in the commit message to let us know what issue is being solved by the changes provided.

To avoid any confusion I confirm the following patch ensures that exploring the working folder on Windows correctly opens the working folder of the current repository.

Tested-by: Pat Thoyts <patthoyts@users.sourceforge.net>
Show changes to git-gui/git-gui.sh +3 −1
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index cd8da37..82c352b 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -2099,15 +2099,17 @@ proc do_git_gui {} {
 proc do_explore {} {
        global _gitworktree
        set explorer {}
+       set path $_gitworktree
        if {[is_Cygwin] || [is_Windows]} {
                set explorer "explorer.exe"
+               set path [list [file nativename $path]]
        } elseif {[is_MacOSX]} {
                set explorer "open"
        } else {
                # freedesktop.org-conforming system is our best shot
                set explorer "xdg-open"
        }
-       eval exec $explorer $_gitworktree &
+       eval exec $explorer $path &
 }

 set is_quitting 0

← back to recent threads