code review 5488059: misc/windows: godocserver destroy cmd.exe

misc/windows: godocserver destroy cmd.exe
use 'exit /b 1'.

[mattn, 2011-12-13 12:03:30]

Hello golang-dev@googlecode.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
http://go.googlecode.com/hg/

[bradfitz, 2011-12-13 15:40:28]

LGTM

Do you want a non-zero exit number, though?

exit /b 1

I don't know what Windows error numbers are, conventionally.


On Tue, Dec 13, 2011 at 4:03 AM, <mattn.jp@gmail.com> wrote:

> Reviewers: golang-dev_googlecode.com,
>
> Message:
> Hello golang-dev@googlecode.com (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> http://go.googlecode.com/hg/
>
>
> Description:
> misc/windows: godocserver destroy cmd.exe
> use 'exit /b'.
>
> Please review this at
http://codereview.appspot.com/**5488059/<http://codereview.appspot.com/5488059/>
>
> Affected files:
>  M misc/windows/godocserver.bat
>
>
> Index: misc/windows/godocserver.bat
> ==============================**==============================**=======
> --- a/misc/windows/godocserver.bat
> +++ b/misc/windows/godocserver.bat
> @@ -6,7 +6,7 @@
>  echo Unable to find the godoc executable
>  echo This batch file must run from the root Go folder
>  pause
> -exit
> +exit /b
>
>  :ok
>  start bin\godoc -http=localhost:6060 -goroot="%cwd%"
>
>
>

[Joe Poirier, 2011-12-13 19:36:42]

LGTM

/b is to exit the script but not the cmd shell

[Joe Poirier, 2011-12-13 21:04:07]

the patch doesn't apply cleanly (hg sync and mail)

[mattn, 2011-12-14 01:30:02]

really? I had 'hg sync' and see 'hg diff'. but it include just this line.

I uploaded 'exit /b 1' changes.

On 2011/12/13 21:04:07, Joe Poirier wrote:
> the patch doesn't apply cleanly (hg sync and mail)

[Joe Poirier, 2011-12-14 02:05:53]

On 2011/12/14 01:30:02, mattn wrote:
> really? I had 'hg sync' and see 'hg diff'. but it include just this line.
> 
> I uploaded 'exit /b 1' changes.
> 
> On 2011/12/13 21:04:07, Joe Poirier wrote:
> > the patch doesn't apply cleanly (hg sync and mail)

Hmmm. I just upgraded my MBP to Lion, which may have messed with my folder/file
permissions or something.

[mikio, 2011-12-14 02:20:29]

On Wed, Dec 14, 2011 at 11:05 AM,  <jdpoirier@gmail.com> wrote:

> Hmmm. I just upgraded my MBP to Lion, which may have messed with my
> folder/file permissions or something.

Not sure the reason but me too.

% patch -p1 < issue5488059_4.diff
patch --verbose -p1 < patch
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|Index: misc/windows/godocserver.bat
|===================================================================
|--- a/misc/windows/godocserver.bat
|+++ b/misc/windows/godocserver.bat
--------------------------
Patching file misc/windows/godocserver.bat using Plan A...
Hunk #1 FAILED at 6.
1 out of 1 hunk FAILED -- saving rejects to file
misc/windows/godocserver.bat.rej
Hmm...  Ignoring the trailing garbage.
done

% patch --version
patch 2.5.8

[mattn, 2011-12-14 02:27:22]

Probably, the cause is that godocserver.bat is DOS format?

[Joe Poirier, 2011-12-14 03:09:34]

On 2011/12/14 02:05:53, Joe Poirier wrote:
> On 2011/12/14 01:30:02, mattn wrote:
> > really? I had 'hg sync' and see 'hg diff'. but it include just this line.
> > 
> > I uploaded 'exit /b 1' changes.
> > 
> > On 2011/12/13 21:04:07, Joe Poirier wrote:
> > > the patch doesn't apply cleanly (hg sync and mail)
> 
> Hmmm. I just upgraded my MBP to Lion, which may have messed with my
folder/file
> permissions or something.

Output from the raw patch

$ patch --dry-run -p1 < issue5488059_4_5002.diff 
patching file misc/windows/godocserver.bat
Hunk #1 FAILED at 6.
1 out of 1 hunk FAILED -- saving rejects to file
misc/windows/godocserver.bat.rej

[Joe Poirier, 2011-12-14 03:15:01]

Sorry, didn't see the previous updated posts.

Is it whitespace?

[mattn, 2011-12-14 03:16:59]

On Wednesday, December 14, 2011 12:09:34 PM UTC+9, Joe Poirier wrote:
>
> Output from the raw patch
>
> $ patch --dry-run -p1 < issue5488059_4_5002.diff
> patching file misc/windows/godocserver.bat
> Hunk #1 FAILED at 6.
> 1 out of 1 hunk FAILED -- saving rejects to file
> misc/windows/godocserver.bat.rej
>
> http://codereview.appspot.com/5488059/
>

Yes, codereview.py make post data with using foo.splitlines(). So \r is 
dropped. And I  tried to post \r\n to codereview.appspot.com, But \r seems 
to be dropped.

It seems that need to add workaround for \r into src/pkg/patch.

[mattn, 2011-12-14 04:55:24]

I tried to post original diff.

https://gist.github.com/1475303 

Please see the patch. And posted binary data 'payload.bin'.
You can get payload data that include \r.

https://raw.github.com/gist/1475303/payload.bin 

But codereview server output as text like

http://codereview.appspot.com/download/issue5483068_1003_2002.diff 

It seens need to change src/pkg/patch.