code review 42530043: dashboard/app: properly limit commit description length

dashboard/app: properly limit commit description length
Ensure that we do not split UTF-8 rune in half.
Otherwise appengine python scripts will break badly.

[dvyukov, 2013-12-15 17:08:17]

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

I'd like you to review this change to
https://code.google.com/p/go.tools

[r, 2013-12-15 18:17:57]

https://codereview.appspot.com/42530043/diff/40001/dashboard/app/build/handle...
File dashboard/app/build/handler.go (right):

https://codereview.appspot.com/42530043/diff/40001/dashboard/app/build/handle...
dashboard/app/build/handler.go:476: }
in practice this should be fine but it would be easy to break: just give it no
valid runes and it will index off the beginning. less efficient but cleaner and
safer and simpler is something like

for i, r := range s {
    if i + utf.RuneLen(r) > max {
        max = i
        break
     }
}

[dvyukov, 2013-12-16 07:50:10]

https://codereview.appspot.com/42530043/diff/40001/dashboard/app/build/handle...
File dashboard/app/build/handler.go (right):

https://codereview.appspot.com/42530043/diff/40001/dashboard/app/build/handle...
dashboard/app/build/handler.go:476: }
On 2013/12/15 18:17:57, r wrote:
> in practice this should be fine but it would be easy to break: just give it no
> valid runes and it will index off the beginning. less efficient but cleaner
and
> safer and simpler is something like
> 
> for i, r := range s {
>     if i + utf.RuneLen(r) > max {
>         max = i
>         break
>      }
> }

For an empty string DecodeLastRuneInString must return (RuneError, 0), so my
loop must yield an empty string at worst. What I am missing?

[r, 2013-12-16 16:18:58]

LGTM

https://codereview.appspot.com/42530043/diff/40001/dashboard/app/build/handle...
File dashboard/app/build/handler.go (right):

https://codereview.appspot.com/42530043/diff/40001/dashboard/app/build/handle...
dashboard/app/build/handler.go:476: }
ah yes, i see now. ok, this works. i still prefer mine (it's not as tricky and
uses a normal loop) but if you prefer yours, that's fine.

[adg, 2013-12-16 22:12:37]

LGTM

[dvyukov, 2013-12-17 08:14:41]

*** Submitted as
https://code.google.com/p/go/source/detail?r=139f5d7a1e02&repo=tools ***

dashboard/app: properly limit commit description length
Ensure that we do not split UTF-8 rune in half.
Otherwise appengine python scripts will break badly.

R=adg, r
CC=golang-dev
https://codereview.appspot.com/42530043