highlight suppression in FullSearch/PageList

highlight suppression in FullSearch/PageList

[ThomasJWaldmann, 2012-05-20 17:12:49]

http://codereview.appspot.com/6209085/diff/1/MoinMoin/config/multiconfig.py
File MoinMoin/config/multiconfig.py (right):

http://codereview.appspot.com/6209085/diff/1/MoinMoin/config/multiconfig.py#n...
MoinMoin/config/multiconfig.py:1335: "in search results generated by macro."),
highlight_titles?

http://codereview.appspot.com/6209085/diff/1/MoinMoin/config/multiconfig.py#n...
MoinMoin/config/multiconfig.py:1337: "results generated by search macros by
default."),
highlight_pages?

http://codereview.appspot.com/6209085/diff/1/MoinMoin/macro/FullSearch.py
File MoinMoin/macro/FullSearch.py (right):

http://codereview.appspot.com/6209085/diff/1/MoinMoin/macro/FullSearch.py#new...
MoinMoin/macro/FullSearch.py:96: # It is needed because othervise macro
instances like
otherwise

http://codereview.appspot.com/6209085/diff/1/MoinMoin/macro/PageList.py
File MoinMoin/macro/PageList.py (right):

http://codereview.appspot.com/6209085/diff/1/MoinMoin/macro/PageList.py#newco...
MoinMoin/macro/PageList.py:54: 'arg': wikiutil.escape(arg)}
code duplication

http://codereview.appspot.com/6209085/diff/1/MoinMoin/search/results.py
File MoinMoin/search/results.py (right):

http://codereview.appspot.com/6209085/diff/1/MoinMoin/search/results.py#newco...
MoinMoin/search/results.py:617: @keyword do_highlight: highlight search query
matches
isn't that @param rather (with a default)?

AFAIK @keyword is rather used for stuff put into **kw.

[eSyr, 2012-05-20 18:56:02]

http://codereview.appspot.com/6209085/diff/1/MoinMoin/search/results.py
File MoinMoin/search/results.py (right):

http://codereview.appspot.com/6209085/diff/1/MoinMoin/search/results.py#newco...
MoinMoin/search/results.py:617: @keyword do_highlight: highlight search query
matches
On 2012/05/20 17:12:49, ThomasJWaldmann wrote:
> isn't that @param rather (with a default)?
> 
> AFAIK @keyword is rather used for stuff put into **kw.
Looks like there's no clear difference between them:
http://sphinx.pocoo.org/markup/desc.html#info-field-lists

If Moin has some convention, like mentioned above, it's fine.

[Reimar Bauer, 2012-05-20 19:14:44]

some comments

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/macro/FullSearch.py
File MoinMoin/macro/FullSearch.py (left):

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/macro/FullSearch.py#...
MoinMoin/macro/FullSearch.py:94: needle = '"%s"' %
macro.formatter.page.page_name
page_name can be utf.8

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/macro/FullSearch.py#...
MoinMoin/macro/FullSearch.py:101: return '<span class="error">%s</span>' % err
if err:

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/macro/FullSearch.py
File MoinMoin/macro/FullSearch.py (right):

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/macro/FullSearch.py#...
MoinMoin/macro/FullSearch.py:83: def execute(macro, needle, titlesearch=0,
case=0):
False ?

don't abuse integer for boolean

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/macro/FullSearch.py#...
MoinMoin/macro/FullSearch.py:153: ret = ''.join([macro.formatter.text('<<%s(' %
macro.name),
this can be utf-8 too

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/macro/PageList.py
File MoinMoin/macro/PageList.py (left):

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/macro/PageList.py#ol...
MoinMoin/macro/PageList.py:27: 
True

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/search/results.py
File MoinMoin/search/results.py (right):

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/search/results.py#ne...
MoinMoin/search/results.py:304: paging=True, hitsFrom=0, hitsInfo=0,
highlight_titles=1,

highlight_titles=True
highlight_pages=True

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/search/results.py#ne...
MoinMoin/search/results.py:389: highlight_titles=1, highlight_pages=1):
high... = True

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/search/results.py#ne...
MoinMoin/search/results.py:401: @param highlight_pages: add highlight parameter
to page URLs
hmm, I can guess what happens but this sentence did not help much, compared to
the line before I have the slight feeling that we need a different namend
parameter.

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/search/results.py#ne...
MoinMoin/search/results.py:610: def formatTitle(self, page, highlight_titles=1):
True

[ThomasJWaldmann, 2012-05-20 19:52:01]

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/macro/FullSearch.py
File MoinMoin/macro/FullSearch.py (right):

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/macro/FullSearch.py#...
MoinMoin/macro/FullSearch.py:110: val = int(arg[1].lower() in [u'1', u'true',
u'y'])
btw, str(True) == 'True'

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/search/results.py
File MoinMoin/search/results.py (right):

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/search/results.py#ne...
MoinMoin/search/results.py:315: @param highlight_titles: performa highlighting
in page list
perform highlighting ...

http://codereview.appspot.com/6209085/diff/7001/MoinMoin/search/results.py#ne...
MoinMoin/search/results.py:359: do_highlight=highlight_pages)
btw, we don't follow PEP8's 79(?) char long lines rule.

sometimes breaking a line just for a few chars makes it worse than just
finishing on same line.