code review 6811104: go.talks/present: allow initial punctuation as well as ...

go.talks/present: allow initial punctuation as well as trailing near marks
Previously,
        _text_.
worked as expected but
        (_text_)
did not. This change fixes the behavior by treating leading
punctuation just like trailing.

[r, 2012-11-12 19:17:39]

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

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

[francesc, 2012-11-12 19:37:32]

I'd add a bit more testing, other than that LGTM

https://codereview.appspot.com/6811104/diff/1/present/style_test.go
File present/style_test.go (right):

https://codereview.appspot.com/6811104/diff/1/present/style_test.go#newcode48
present/style_test.go:48: {"(_a)", "(_a)"},
Add a test with more than one char of punctuation

[r, 2012-11-12 19:46:41]

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

Please take another look.

[francesc, 2012-11-12 19:58:30]

On 2012/11/12 19:46:41, r wrote:
> Hello mailto:adg@golang.org, mailto:campoy@golang.org (cc:
mailto:golang-dev@googlegroups.com),
> 
> Please take another look.

LGTM

[r, 2012-11-12 20:48:27]

*** Submitted as
http://code.google.com/p/go/source/detail?r=b8eea95e35a3&repo=talks ***

go.talks/present: allow initial punctuation as well as trailing near marks
Previously,
        _text_.
worked as expected but
        (_text_)
did not. This change fixes the behavior by treating leading
punctuation just like trailing.

R=adg, campoy
CC=golang-dev
http://codereview.appspot.com/6811104

[adg, 2012-11-13 09:11:46]

LGTM

Thanks for doing this. I had it on my list to do the same.


On 12 November 2012 20:17, <r@golang.org> wrote:

> Reviewers: adg,
>
> Message:
> Hello adg (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go.**talks <https://code.google.com/p/go.talks>
>
>
> Description:
> go.talks/present: allow initial punctuation as well as trailing near
> marks
> Previously,
>         _text_.
> worked as expected but
>         (_text_)
> did not. This change fixes the behavior by treating leading
> punctuation just like trailing.
>
> Please review this at
http://codereview.appspot.com/**6811104/<http://codereview.appspot.com/6811104/>
>
> Affected files:
>   M present/style.go
>   M present/style_test.go
>
>
> Index: present/style.go
> ==============================**==============================**=======
> --- a/present/style.go
> +++ b/present/style.go
> @@ -42,20 +42,33 @@
>                 if len(word) < 2 {
>                         continue Word
>                 }
> +               const punctuation = `.,;:()!?—–'"`
> +               const marker = "_*`"
> +               // Initial punctuation is OK but must be peeled off.
> +               first := strings.IndexAny(word, marker)
> +               if first == -1 {
> +                       continue Word
> +               }
> +               // Is the marker prefixed only by punctuation?
> +               for _, r := range word[:first] {
> +                       if !strings.ContainsRune(**punctuation, r) {
> +                               continue Word
> +                       }
> +               }
> +               open, word := word[:first], word[first:]
>                 char := word[0] // ASCII is OK.
> -               open := ""
>                 close := ""
>                 switch char {
>                 default:
>                         continue Word
>                 case '_':
> -                       open = "<i>"
> +                       open += "<i>"
>                         close = "</i>"
>                 case '*':
> -                       open = "<b>"
> +                       open += "<b>"
>                         close = "</b>"
>                 case '`':
> -                       open = "<code>"
> +                       open += "<code>"
>                         close = "</code>"
>                 }
>                 // Terminal punctuation is OK but must be peeled off.
> @@ -65,7 +78,7 @@
>                 }
>                 head, tail := word[:last+1], word[last+1:]
>                 for _, r := range tail {
> -                       if !strings.ContainsRune(`.,;:()!**?—–'"`, r) {
> +                       if !strings.ContainsRune(**punctuation, r) {
>                                 continue Word
>                         }
>                 }
> Index: present/style_test.go
> ==============================**==============================**=======
> --- a/present/style_test.go
> +++ b/present/style_test.go
> @@ -44,6 +44,8 @@
>                 {"_a___b_", "<i>a_ b</i>"},
>                 {"*a**b*?", "<b>a*b</b>?"},
>                 {"_a_<>_b_.", "<i>a <> b</i>."},
> +               {"(_a_)", "(<i>a</i>)"},
> +               {"(_a)", "(_a)"},
>                 {"_Why_use_scoped__ptr_? Use plain ***ptr* instead.",
> "<i>Why use scoped_ptr</i>? Use plain <b>*ptr</b> instead."},
>         }
>         for _, test := range tests {
> @@ -70,6 +72,8 @@
>                 {"_a___b_", "<i>a_ b</i>"},
>                 {"*a**b*?", "<b>a*b</b>?"},
>                 {"_a_<>_b_.", "<i>a &lt;&gt; b</i>."},
> +               {"(_a_<>_b_)", "(<i>a &lt;&gt; b</i>)"},
> +               {"(_a)", "(_a)"},
>         }
>         for _, test := range tests {
>                 out := string(style(test.in))
>
>
>