google / git-appraise

Distributed code review system for Git repos
Apache License 2.0
5.14k stars 146 forks source link

git appraise comment does not work on Windows #19

Closed linquize closed 8 years ago

linquize commented 8 years ago

git appraise comment without any options does not work on Windows.

Unable to start editor: exec: "vi": executable file not found in %PATH%

I use Git for Windows 2.6.4 This is because /usr/bin/vi is a shell script, not a native Windows executable. The actual exe is /usr/bin/vim.

barbastan commented 8 years ago

Ooooo... Thanks so much for the error report! Will fix this after the holidays.

Barbara

On Sun, Dec 20, 2015 at 6:13 AM, linquize notifications@github.com wrote:

git appraise comment without any options does not work on Windows.

Unable to start editor: exec: "vi": executable file not found in %PATH%

I use Git for Windows 2.6.4 This is because /usr/bin/vi is a shell script, not a native Windows executable. The actual exe is /usr/bin/vim.

— Reply to this email directly or view it on GitHub https://github.com/google/git-appraise/issues/19.

hazbo commented 8 years ago

Hey @linquize thanks for pointing this out. This is something I was working on the other day. It's using:

git var GIT_EDITOR

to find the default editor, and then starting it (assuming it's in the PATH by default). I think we could maybe check to see if the editor actually exists, and / or set a default editor, as this code is used in one place so far - could be used else where possibly in the future. Here might be a good place for us to do that.

barbastan commented 8 years ago

NICE! Again, sorry for the delay, but I'd like to get our Tech Lead's stamp of approval after his return from the holidays.

THANK YOU!

Barbara

On Sun, Dec 20, 2015 at 9:22 AM, Harry Lawrence notifications@github.com wrote:

Hey @linquize https://github.com/linquize thanks for pointing this out. This is something I was working on the other day. It's using:

git var GIT_EDITOR

to find the default editor, and then starting it (assuming it's in the PATH by default). I think we could maybe check to see if the editor actually exists, and / or set a default editor. As this code is used in one place so far. Here https://github.com/google/git-appraise/blob/master/repository/git.go#L99-L101 might be a good place for us to do that.

— Reply to this email directly or view it on GitHub https://github.com/google/git-appraise/issues/19#issuecomment-166136415.

linquize commented 8 years ago

You may think of git config core.editor

barbastan commented 8 years ago

Ahhhh... Of course! Thanks.

On Sun, Dec 20, 2015 at 3:41 PM, linquize notifications@github.com wrote:

You may think of git config core.editor

— Reply to this email directly or view it on GitHub https://github.com/google/git-appraise/issues/19#issuecomment-166165412.

hazbo commented 8 years ago

@linquize that's what we originally had in there but decided against it. I'm going to have a think about this and see what might work best. Thanks so much for raising this issue, I'd not realised there would be problems on Windows, so this has been very helpful for me.

ojarjur commented 8 years ago

@linquize I don't have a Windows machine to test this on, but I just changed the editor logic to try opening the editor with a shell if the first attempt fails.

Can you update to the latest version and see if that helps?

linquize commented 8 years ago

I tested 39aeaf9dc2b03a53a4e5c0aa5dd82859ad1c80cd, it works with git for windows 2.7.0. vi is launched.

ojarjur commented 8 years ago

Awesome! Thanks for checking, @linquize.

If you find anything else wrong then please don't hesitate to file another bug.