Keyboard Shortcuts

File
u :up to issue
m :publish + mail comments
M :edit review message
j / k :jump to file after / before current file
J / K :jump to next file with a comment after / before current file
Side-by-side diff
i :toggle intra-line diffs
e :expand all comments
c :collapse all comments
s :toggle showing all comments
n / p :next / previous diff chunk or comment
N / P :next / previous comment
<Up> / <Down> :next / previous line
<Enter> :respond to / edit current comment
d :mark current comment as done
Issue
u :up to list of issues
m :publish + mail comments
j / k :jump to patch after / before current patch
o / <Enter> :open current patch in side-by-side view
i :open current patch in unified diff view
Issue List
j / k :jump to issue after / before current issue
o / <Enter> :open current issue
# : close issue
Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :save comment
<Esc> :cancel edit
Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(133)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 1 month ago by mattn
Modified:
14 years, 1 month ago
Reviewers:
Joe Poirier , golang-dev, mikio, bradfitz
CC:
golang-dev
Visibility:
Public.
misc/windows: godocserver destroy cmd.exe use 'exit /b 1'.

Patch Set 1 #

Patch Set 2 : diff -r f03312eab4ce http://go.googlecode.com/hg/ #

Patch Set 3 : diff -r f03312eab4ce http://go.googlecode.com/hg/ #

Patch Set 4 : diff -r cc4e4e6675a3 http://go.googlecode.com/hg/ #

Created: 14 years, 1 month ago
Download [raw] [tar.bz2]
Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M misc/windows/godocserver.bat View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
Total messages: 12
|
mattn
Hello golang-dev@googlecode.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to http://go.googlecode.com/hg/
14 years, 1 month ago (2011年12月13日 12:03:30 UTC) #1
Hello golang-dev@googlecode.com (cc: golang-dev@googlegroups.com),
I'd like you to review this change to
http://go.googlecode.com/hg/ 
Sign in to reply to this message.
bradfitz
LGTM Do you want a non-zero exit number, though? exit /b 1 I don't know ...
14 years, 1 month ago (2011年12月13日 15:40:28 UTC) #2
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%"
>
>
>
Sign in to reply to this message.
Joe Poirier
LGTM /b is to exit the script but not the cmd shell
14 years, 1 month ago (2011年12月13日 19:36:42 UTC) #3
LGTM
/b is to exit the script but not the cmd shell
Sign in to reply to this message.
Joe Poirier
the patch doesn't apply cleanly (hg sync and mail)
14 years, 1 month ago (2011年12月13日 21:04:07 UTC) #4
the patch doesn't apply cleanly (hg sync and mail)
Sign in to reply to this message.
mattn
really? I had 'hg sync' and see 'hg diff'. but it include just this line. ...
14 years, 1 month ago (2011年12月14日 01:30:02 UTC) #5
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)
Sign in to reply to this message.
Joe Poirier
On 2011年12月14日 01:30:02, mattn wrote: > really? I had 'hg sync' and see 'hg diff'. ...
14 years, 1 month ago (2011年12月14日 02:05:53 UTC) #6
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.
Sign in to reply to this message.
mikio
On Wed, Dec 14, 2011 at 11:05 AM, <jdpoirier@gmail.com> wrote: > Hmmm. I just upgraded ...
14 years, 1 month ago (2011年12月14日 02:20:29 UTC) #7
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
Sign in to reply to this message.
mattn
Probably, the cause is that godocserver.bat is DOS format?
14 years, 1 month ago (2011年12月14日 02:27:22 UTC) #8
Probably, the cause is that godocserver.bat is DOS format?
Sign in to reply to this message.
Joe Poirier
On 2011年12月14日 02:05:53, Joe Poirier wrote: > On 2011年12月14日 01:30:02, mattn wrote: > > really? ...
14 years, 1 month ago (2011年12月14日 03:09:34 UTC) #9
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
Sign in to reply to this message.
Joe Poirier
Sorry, didn't see the previous updated posts. Is it whitespace?
14 years, 1 month ago (2011年12月14日 03:15:01 UTC) #10
Sorry, didn't see the previous updated posts.
Is it whitespace?
Sign in to reply to this message.
mattn
On Wednesday, December 14, 2011 12:09:34 PM UTC+9, Joe Poirier wrote: > > Output from ...
14 years, 1 month ago (2011年12月14日 03:16:59 UTC) #11
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.
Sign in to reply to this message.
mattn
I tried to post original diff. https://gist.github.com/1475303 Please see the patch. And posted binary data ...
14 years, 1 month ago (2011年12月14日 04:55:24 UTC) #12
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.
Sign in to reply to this message.
|
This is Rietveld f62528b

AltStyle によって変換されたページ (->オリジナル) /