Issue 14023043: code review 14023043: go.tools/blog: replace "\\" with "/" in path string to ...
Descriptiongo.tools/blog: replace "\\" with "/" in path string to be compatible with windows platform
Fixes Issue 6539
The problem happens on my win7,
for example, the path is "/content\\foo.article".
It leads to the wrong link in generated html page.
So I think we should replace all"\\" with "/" in path string at first.
Patch Set 1 #Patch Set 2 : diff -r a7a5efbad067 http://code.google.com/p/go.tools #
Total comments: 1
Patch Set 3 : diff -r 46a5b05299b4 http://code.google.com/p/go.tools #MessagesTotal messages: 14
https://codereview.appspot.com/14023043/diff/1002/blog/blog.go File blog/blog.go (right): https://codereview.appspot.com/14023043/diff/1002/blog/blog.go#newcode194 blog/blog.go:194: p = strings.Replace(p, "\\", "/", -1) you probably want to use filepath.ToSlash here instead.
Sign in to reply to this message.
Hello golang-dev@googlegroups.com, dsymonds@golang.org, mirtchovski@gmail.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to http://code.google.com/p/go.tools
Sign in to reply to this message.
On 2013/10/06 01:52:57, Tw wrote: > Hello mailto:golang-dev@googlegroups.com, mailto:dsymonds@golang.org, mailto:mirtchovski@gmail.com > (cc: mailto:golang-dev@googlegroups.com), > > I'd like you to review this change to > http://code.google.com/p/go.tools I defer to the go team members here. Personally I'd like to see a better explanation of the issue.
Sign in to reply to this message.
On 2013/10/06 01:56:46, aam wrote: > On 2013/10/06 01:52:57, Tw wrote: > > Hello mailto:golang-dev@googlegroups.com, mailto:dsymonds@golang.org, > mailto:mirtchovski@gmail.com > > (cc: mailto:golang-dev@googlegroups.com), > > > > I'd like you to review this change to > > http://code.google.com/p/go.tools > > I defer to the go team members here. Personally I'd like to see a better > explanation of the issue. Sorry, this is my fault. The problem happens on my win7, for example, the path is "/content\\foo.article". It leads to the wrong link in generated html page. So I think we should replace all "\\" with "/" in path string at first.
Sign in to reply to this message.
On Sun, Oct 6, 2013 at 1:07 PM, <tw19881113@gmail.com> wrote: > On 2013/10/06 01:56:46, aam wrote: >> >> On 2013/10/06 01:52:57, Tw wrote: >> > Hello mailto:golang-dev@googlegroups.com, > > mailto:dsymonds@golang.org, >> >> mailto:mirtchovski@gmail.com >> > (cc: mailto:golang-dev@googlegroups.com), >> > >> > I'd like you to review this change to >> > http://code.google.com/p/go.tools > > >> I defer to the go team members here. Personally I'd like to see a > > better >> >> explanation of the issue. > > > Sorry, this is my fault. > The problem happens on my win7, for example, the path is > "/content\\foo.article". > It leads to the wrong link in generated html page. So I think we should > replace all > "\\" with "/" in path string at first. > > > https://codereview.appspot.com/14023043/ > > -- > > ---You received this message because you are subscribed to the Google Groups > "golang-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-dev+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/groups/opt_out.
Sign in to reply to this message.
tw, we're deep in the bug fix stage of go 1.2. Could you please start by raising a bug for the issue you have with the blog tool, then, when the problem is understood we'll be in a better position to digest your proposed fix. On Mon, Oct 7, 2013 at 11:33 AM, Dave Cheney <dave@cheney.net> wrote: > On Sun, Oct 6, 2013 at 1:07 PM, <tw19881113@gmail.com> wrote: >> On 2013/10/06 01:56:46, aam wrote: >>> >>> On 2013/10/06 01:52:57, Tw wrote: >>> > Hello mailto:golang-dev@googlegroups.com, >> >> mailto:dsymonds@golang.org, >>> >>> mailto:mirtchovski@gmail.com >>> > (cc: mailto:golang-dev@googlegroups.com), >>> > >>> > I'd like you to review this change to >>> > http://code.google.com/p/go.tools >> >> >>> I defer to the go team members here. Personally I'd like to see a >> >> better >>> >>> explanation of the issue. >> >> >> Sorry, this is my fault. >> The problem happens on my win7, for example, the path is >> "/content\\foo.article". >> It leads to the wrong link in generated html page. So I think we should >> replace all >> "\\" with "/" in path string at first. >> >> >> https://codereview.appspot.com/14023043/ >> >> -- >> >> ---You received this message because you are subscribed to the Google Groups >> "golang-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to golang-dev+unsubscribe@googlegroups.com. >> For more options, visit https://groups.google.com/groups/opt_out.
Sign in to reply to this message.
On 2013/10/07 00:34:46, dfc wrote: > tw, we're deep in the bug fix stage of go 1.2. Could you please start > by raising a bug for the issue you have with the blog tool, then, when > the problem is understood we'll be in a better position to digest your > proposed fix. > > On Mon, Oct 7, 2013 at 11:33 AM, Dave Cheney <mailto:dave@cheney.net> wrote: > > On Sun, Oct 6, 2013 at 1:07 PM, <mailto:tw19881113@gmail.com> wrote: > >> On 2013/10/06 01:56:46, aam wrote: > >>> > >>> On 2013/10/06 01:52:57, Tw wrote: > >>> > Hello mailto:golang-dev@googlegroups.com, > >> > >> mailto:dsymonds@golang.org, > >>> > >>> mailto:mirtchovski@gmail.com > >>> > (cc: mailto:golang-dev@googlegroups.com), > >>> > > >>> > I'd like you to review this change to > >>> > http://code.google.com/p/go.tools > >> > >> > >>> I defer to the go team members here. Personally I'd like to see a > >> > >> better > >>> > >>> explanation of the issue. > >> > >> > >> Sorry, this is my fault. > >> The problem happens on my win7, for example, the path is > >> "/content\\foo.article". > >> It leads to the wrong link in generated html page. So I think we should > >> replace all > >> "\\" with "/" in path string at first. > >> > >> > >> https://codereview.appspot.com/14023043/ > >> > >> -- > >> > >> ---You received this message because you are subscribed to the Google Groups > >> "golang-dev" group. > >> To unsubscribe from this group and stop receiving emails from it, send an > >> email to mailto:golang-dev+unsubscribe@googlegroups.com. > >> For more options, visit https://groups.google.com/groups/opt_out. http://code.google.com/p/go/issues/detail?id=6539 done.
Sign in to reply to this message.
LGTM Have you signed the CLA? http://golang.org/doc/contribute.html#Copyright
Sign in to reply to this message.
LGTM Please add "Fixes Issue 6539" to your CL description, as explained here http://golang.org/doc/contribute.html. Thank you. Alex
Sign in to reply to this message.
On 2013/10/08 03:05:59, brainman wrote: > LGTM > > Please add "Fixes Issue 6539" to your CL description, as explained here > http://golang.org/doc/contribute.html. > > Thank you. > > Alex done!
Sign in to reply to this message.
On 2013/10/08 00:14:18, adg wrote: > LGTM > > Have you signed the CLA? http://golang.org/doc/contribute.html#Copyright No. I have submitted CLA right now.
Sign in to reply to this message.
*** Submitted as https://code.google.com/p/go/source/detail?r=86e5a2953df6&repo=tools *** go.tools/blog: replace "\\" with "/" in path string to be compatible with windows platform Fixes Issue 6539 The problem happens on my win7, for example, the path is "/content\\foo.article". It leads to the wrong link in generated html page. So I think we should replace all"\\" with "/" in path string at first. R=golang-dev, dsymonds, mirtchovski, dave, adg, alex.brainman CC=golang-dev https://codereview.appspot.com/14023043 Committer: Andrew Gerrand <adg@golang.org>
Sign in to reply to this message.
|
