Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(7)

Issue 10539043: code review 10539043: go.talks/pkg/present: include line numbers in output HTML (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 7 months ago by adg
Modified:
7 years, 7 months ago
Reviewers:
r
CC:
golang-dev, dvyukov, r
Visibility:
Public.

Description

go.talks/pkg/present: include line numbers in output HTML Also refactor HTML code generation to be line and template based.

Patch Set 1 #

Patch Set 2 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Patch Set 3 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Patch Set 4 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Patch Set 5 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Patch Set 6 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Patch Set 7 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Patch Set 8 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Total comments: 4

Patch Set 9 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Patch Set 10 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Patch Set 11 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Patch Set 12 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Total comments: 2

Patch Set 13 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Total comments: 2

Patch Set 14 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Patch Set 15 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Patch Set 16 : diff -r 27c84def09a6 https://code.google.com/p/go.talks #

Unified diffs Side-by-side diffs Delta from patch set Stats (+79 lines, -60 lines) Patch
M pkg/present/code.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 4 chunks +78 lines, -59 lines 0 comments Download
M present/js/play.js View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 10
adg
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go.talks
7 years, 7 months ago (2013-06-25 10:48:16 UTC) #1
dvyukov
https://codereview.appspot.com/10539043/diff/19001/pkg/present/code.go File pkg/present/code.go (right): https://codereview.appspot.com/10539043/diff/19001/pkg/present/code.go#newcode173 pkg/present/code.go:173: startLine++ have you tested it for off-by-one bugs? https://codereview.appspot.com/10539043/diff/19001/pkg/present/code.go#newcode181 ...
7 years, 7 months ago (2013-06-25 11:02:16 UTC) #2
adg
PTAL https://codereview.appspot.com/10539043/diff/19001/pkg/present/code.go File pkg/present/code.go (right): https://codereview.appspot.com/10539043/diff/19001/pkg/present/code.go#newcode173 pkg/present/code.go:173: startLine++ On 2013/06/25 11:02:16, dvyukov wrote: > have ...
7 years, 7 months ago (2013-06-25 11:11:22 UTC) #3
dvyukov
looks good, but please wait for somebody else
7 years, 7 months ago (2013-06-25 11:21:11 UTC) #4
r
https://codereview.appspot.com/10539043/diff/28001/pkg/present/code.go File pkg/present/code.go (right): https://codereview.appspot.com/10539043/diff/28001/pkg/present/code.go#newcode150 pkg/present/code.go:150: `)) this really doesn't have to be one declaration. ...
7 years, 7 months ago (2013-06-25 16:12:40 UTC) #5
adg
PTAL https://codereview.appspot.com/10539043/diff/28001/pkg/present/code.go File pkg/present/code.go (right): https://codereview.appspot.com/10539043/diff/28001/pkg/present/code.go#newcode150 pkg/present/code.go:150: `)) On 2013/06/25 16:12:40, r wrote: > this ...
7 years, 7 months ago (2013-06-25 23:05:57 UTC) #6
r
https://codereview.appspot.com/10539043/diff/33001/pkg/present/code.go File pkg/present/code.go (right): https://codereview.appspot.com/10539043/diff/33001/pkg/present/code.go#newcode139 pkg/present/code.go:139: return "" leadingSpaceRE = regexp.MustCompile("^[ \t]*") return leadingSpaceRE.FindString(s) in ...
7 years, 7 months ago (2013-06-25 23:37:33 UTC) #7
adg
PTAL https://codereview.appspot.com/10539043/diff/33001/pkg/present/code.go File pkg/present/code.go (right): https://codereview.appspot.com/10539043/diff/33001/pkg/present/code.go#newcode139 pkg/present/code.go:139: return "" On 2013/06/25 23:37:33, r wrote: > ...
7 years, 7 months ago (2013-06-25 23:44:17 UTC) #8
r
LGTM much easier to read
7 years, 7 months ago (2013-06-26 00:01:54 UTC) #9
adg
7 years, 7 months ago (2013-06-26 00:03:35 UTC) #10
*** Submitted as
https://code.google.com/p/go/source/detail?r=8068452134af&repo=talks ***

go.talks/pkg/present: include line numbers in output HTML

Also refactor HTML code generation to be line and template based.

R=golang-dev, dvyukov, r
CC=golang-dev
https://codereview.appspot.com/10539043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b