code review 7096045: exp/locale/search: API proposal for locale-sensitive st...

exp/locale/search: API proposal for locale-sensitive string search.

[mpvl, 2013-01-11 20:40:38]

Hello rsc@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg

[r, 2013-02-21 00:17:09]

looks reasonable. why not write an illustrative example now, even if it doesn't run yet? https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... File src/pkg/exp/locale/search/search.go (right): https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:19: IgnoreDiacritics // Ignore diacritics marks. ("o" == "ö"). s/ marks/ also i'd swap the order of the two o's: "ö" == "o" https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:27: // Search provides locale-sensitve search functionality. s/sensitve/sensitive/ it's a Searcher, not a Search. i slightly prefer you rename the struct Search, since it's not an interface. https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:37: // NewFromTable returns a Searcher given a Weigher. if it's from a weigher, why is it "FromTable"? https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:52: // Match checks whether a an b are equivalent. s/an/and/ https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:57: // MatchString checks whether a an b are equivalent. ditto https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:131: // pattern, it will be more efficient to use Pattern, rather than the direct s/, than/ https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:144: func (s *Searcher) CompileString(string) *Pattern { s/string/b &/ https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:148: // Match checks whether b is equivalent to p. is 'equivalent' a term of art here? why not 'match'?

[mpvl, 2013-02-21 17:17:30]

Hello rsc@golang.org, r@golang.org (cc: golang-dev@googlegroups.com), Please take another look.

[mpvl, 2013-02-21 17:18:16]

https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... File src/pkg/exp/locale/search/search.go (right): https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:19: IgnoreDiacritics // Ignore diacritics marks. ("o" == "ö"). On 2013/02/21 00:17:09, r wrote: > s/ marks/ > also i'd swap the order of the two o's: "ö" == "o" Done. https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:27: // Search provides locale-sensitve search functionality. On 2013/02/21 00:17:09, r wrote: > s/sensitve/sensitive/ > > it's a Searcher, not a Search. i slightly prefer you rename the struct Search, > since it's not an interface. Done. https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:37: // NewFromTable returns a Searcher given a Weigher. On 2013/02/21 00:17:09, r wrote: > if it's from a weigher, why is it "FromTable"? Done. https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:52: // Match checks whether a an b are equivalent. On 2013/02/21 00:17:09, r wrote: > s/an/and/ Done. https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:57: // MatchString checks whether a an b are equivalent. On 2013/02/21 00:17:09, r wrote: > ditto Done. https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:144: func (s *Searcher) CompileString(string) *Pattern { On 2013/02/21 00:17:09, r wrote: > s/string/b &/ Done. https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:148: // Match checks whether b is equivalent to p. On 2013/02/21 00:17:09, r wrote: > is 'equivalent' a term of art here? why not 'match'? Done. https://codereview.appspot.com/7096045/diff/5001/src/pkg/exp/locale/search/se... src/pkg/exp/locale/search/search.go:148: // Match checks whether b is equivalent to p. On 2013/02/21 00:17:09, r wrote: > is 'equivalent' a term of art here? why not 'match'? Done.

[r, 2013-02-22 23:58:28]

LGTM for now but i think there may be another approach worth trying. this is exp, so check it in for now and we'll revisit.

[rsc, 2013-12-18 05:33:49]

R=close