changeset 1499:ffa0d1f81059

final polishing round adding docstrings, comments and fixing small issues
author Franz Pletz <fpletz AT franz-pletz DOT org>
date Sat, 26 Aug 2006 20:20:52 +0200
parents 2807382fec1f
children 3b37616c6b57
files MoinMoin/action/fullsearch.py MoinMoin/macro/AdvancedSearch.py MoinMoin/search/Xapian.py MoinMoin/search/builtin.py MoinMoin/search/queryparser.py MoinMoin/search/results.py
diffstat 6 files changed, 131 insertions(+), 62 deletions(-) [+]
line wrap: on
line diff
--- a/MoinMoin/action/fullsearch.py	Sat Aug 26 18:47:58 2006 +0200
+++ b/MoinMoin/action/fullsearch.py	Sat Aug 26 20:20:52 2006 +0200
@@ -20,7 +20,7 @@
     When used in FullSearch macro, we have 'titlesearch' parameter with
     '0' or '1'. In standard search, we have either 'titlesearch' or
     'fullsearch' with localized string. If both missing, default to
-    True (might happen with Safari).
+    True (might happen with Safari) if this isn't an advanced search.
     """
     try:
         return int(request.form['titlesearch'][0])
@@ -30,12 +30,15 @@
         return 'fullsearch' not in request.form and \
                 not isAdvancedSearch(request)
 
+
 def isAdvancedSearch(request):
+    """ Return True if advanced search is requested """
     try:
         return int(request.form['advancedsearch'][0])
     except KeyError:
         return False
 
+
 def execute(pagename, request, fieldname='value', titlesearch=0):
     _ = request.getText
     titlesearch = isTitleSearch(request)
@@ -61,6 +64,7 @@
 
     max_context = 1 # only show first `max_context` contexts XXX still unused
 
+    # if advanced search is enabled we construct our own search query
     if advancedsearch:
         and_terms = request.form.get('and_terms', [''])[0].strip()
         or_terms = request.form.get('or_terms', [''])[0].strip()
@@ -86,7 +90,7 @@
                         'and is therefore not considered for the search '
                         'results!')
                 mtime = None
-        
+
         word_re = re.compile(r'(\"[\w\s]+"|\w+)')
         needle = ''
         if language:
@@ -121,7 +125,10 @@
         title = _('Title Search: "%s"')
         sort = 'page_name'
     else:
-        title = _('Full Text Search: "%s"')
+        if advancedsearch:
+            title = _('Adanced Search: "%s"')
+        else:
+            title = _('Full Text Search: "%s"')
         sort = 'weight'
 
     # search the pages
@@ -130,14 +137,14 @@
         query = QueryParser(case=case, regex=regex,
                 titlesearch=titlesearch).parse_query(needle)
         results = searchPages(request, query, sort, mtime, historysearch)
-    except ValueError:
+    except ValueError: # catch errors in the search query
         err = _('Your search query {{{"%s"}}} is invalid. Please refer to '
                 'HelpOnSearching for more information.') % needle
         Page(request, pagename).send_page(request, msg=err)
         return
 
     # directly show a single hit
-    # XXX won't work with attachment search
+    # Note: can't work with attachment search
     # improve if we have one...
     if len(results.hits) == 1:
         page = results.hits[0]
@@ -146,8 +153,7 @@
             url = page.url(request, querystr={'highlight': query.highlight_re()}, escape=0, relative=False)
             request.http_redirect(url)
             return
-    # no hits?
-    elif not results.hits:
+    elif not results.hits: # no hits?
         f = request.formatter
         querydict = wikiutil.parseQueryString(request.query_string)
         querydict.update({'titlesearch': 0})
--- a/MoinMoin/macro/AdvancedSearch.py	Sat Aug 26 18:47:58 2006 +0200
+++ b/MoinMoin/macro/AdvancedSearch.py	Sat Aug 26 20:20:52 2006 +0200
@@ -1,14 +1,10 @@
 # -*- coding: iso-8859-1 -*-
-'''
+"""
     MoinMoin - AdvancedSearch Macro
 
     [[AdvancedSearch]]
         displays advanced search dialog.
-
-    MAYBE:
-    [[AdvancedSearch(Help)]]
-        embed results of an advanced search (use more parameters...)
-'''
+"""
 
 from MoinMoin import config, wikiutil, search
 from MoinMoin.i18n import languages
@@ -22,6 +18,7 @@
 def form_get(request, name, default=''):
     return request.form.get(name, [default])[0]
 
+
 def advanced_ui(macro):
     _ = macro._
     f = macro.formatter
@@ -67,6 +64,7 @@
         )])
     ])
 
+    # language selection
     searchedlang = form_get(request, 'language')
     langs = dict([(lang, lmeta['x-language-in-english'])
         for lang, lmeta in languages.items()])
@@ -82,6 +80,7 @@
         u'</select>',
     ])
 
+    # mimetype selection
     mimetype = form_get(request, 'mimetype')
     ft_dropdown = ''.join([
         u'<select name="mimetype" size="1"%s>' % disabledIfMoinSearch,
@@ -93,6 +92,7 @@
         u'</select>',
     ])
 
+    # misc search options (checkboxes)
     search_options = ''.join([
         ''.join([
             f.table_row(1),
@@ -131,7 +131,8 @@
                      _('Search in all page revisions')))
             )
     ])
-    
+
+    # the dialogue
     html = [
         u'<form method="get" action="">',
         u'<div>',
@@ -154,13 +155,6 @@
 
 
 def execute(macro, needle):
-    request = macro.request
-    _ = request.getText
+    # for now, just show the advanced ui
+    return advanced_ui(macro)
 
-    # no args given
-    if needle is None:
-        return advanced_ui(macro)
-
-    return macro.formatter.rawHTML('wooza!')
-        
-
--- a/MoinMoin/search/Xapian.py	Sat Aug 26 18:47:58 2006 +0200
+++ b/MoinMoin/search/Xapian.py	Sat Aug 26 20:20:52 2006 +0200
@@ -31,6 +31,9 @@
 class UnicodeQuery(xapian.Query):
     """ Xapian query object which automatically encodes unicode strings """
     def __init__(self, *args, **kwargs):
+        """
+        @keyword encoding: specifiy the encoding manually (default: value of config.charset)
+        """
         self.encoding = kwargs.get('encoding', config.charset)
 
         nargs = []
@@ -88,6 +91,10 @@
     # WORD_RE = re.compile('\\w{1,%i}' % MAX_KEY_LEN, re.U)
 
     def __init__(self, request=None, language=None):
+        """
+        @param request: current request
+        @param language: if given, the language in which to stem words
+        """
         if request and request.cfg.xapian_stemming and language:
             self.stemmer = Stemmer(language)
         else:
@@ -212,10 +219,9 @@
 
     def _check_version(self):
         """ Checks if the correct version of Xapian is installed """
-        # XXX this check cries for troubles in future!
-        if xapian.xapian_major_version() == 0 and \
-                xapian.xapian_minor_version() == 9 \
-                and xapian.xapian_revision() >= 6:
+        # every version greater than or equal to 0.9.6 is allowed for now
+        # Note: fails if crossing the 10.x barrier
+        if xapian.xapian_version_string() >= '0.9.6':
             return
 
         from MoinMoin.error import ConfigurationError
@@ -236,8 +242,13 @@
         """ Check if the Xapian index exists """
         return BaseIndex.exists(self) and os.listdir(self.dir)
 
-    def _search(self, query, sort=None, historysearch=0):
-        """ Perform the search using xapian (read-lock acquired) """
+    def _search(self, query, sort='weight', historysearch=0):
+        """ Perform the search using xapian (read-lock acquired)
+        
+        @param query: the search query objects
+        @keyword sort: the sorting of the results (default: 'weight')
+        @keyword historysearch: whether to search in all page revisions (default: 0)
+        """
         while True:
             try:
                 searcher, timestamp = self.request.cfg.xapian_searchers.pop()
@@ -435,11 +446,11 @@
 
     def _index_page(self, writer, page, mode='update'):
         """ Index a page - assumes that the write lock is acquired
-            @arg writer: the index writer object
-            @arg page: a page object
-            @arg mode: 'add' = just add, no checks
-                       'update' = check if already in index and update if needed (mtime)
-            
+
+        @arg writer: the index writer object
+        @arg page: a page object
+        @arg mode: 'add' = just add, no checks
+                   'update' = check if already in index and update if needed (mtime)
         """
         request = page.request
         wikiname = request.cfg.interwikiname or "Self"
@@ -464,7 +475,7 @@
                 doc = docs[0] # there should be only one
                 uid = doc['uid']
                 docmtime = long(doc['values']['mtime'])
-                updated = True # XXX: forcing, mtime > docmtime
+                updated = mtime > docmtime
                 if debug: request.log("uid %r: mtime %r > docmtime %r == updated %r" % (uid, mtime, docmtime, updated))
             else:
                 uid = None
--- a/MoinMoin/search/builtin.py	Sat Aug 26 18:47:58 2006 +0200
+++ b/MoinMoin/search/builtin.py	Sat Aug 26 20:20:52 2006 +0200
@@ -22,10 +22,14 @@
 ##############################################################################
 
 class UpdateQueue:
-    """ Represents a locked Page queue on the disk """
+    """ Represents a locked page queue on the disk """
 
-    def __init__(self, file, lock_dir):
-        self.file = file
+    def __init__(self, f, lock_dir):
+        """
+        @param f: file to write to
+        @param lock_dir: directory to save the lock files
+        """
+        self.file = f
         self.writeLock = lock.WriteLock(lock_dir, timeout=10.0)
         self.readLock = lock.ReadLock(lock_dir, timeout=10.0)
 
@@ -34,7 +38,10 @@
         return os.path.exists(self.file)
 
     def append(self, pagename):
-        """ Append a page to queue """
+        """ Append a page to queue
+        
+        @param pagename: string to save
+        """
         if not self.writeLock.acquire(60.0):
             request.log("can't add %r to xapian update queue: can't lock queue" %
                         pagename)
@@ -62,6 +69,8 @@
         
         When the queue is empty, the queue file is removed, so exists()
         can tell if there is something waiting in the queue.
+
+        @param pages: list of pagenames to remove
         """
         if self.writeLock.acquire(30.0):
             try:
@@ -83,12 +92,18 @@
     # Private -------------------------------------------------------
 
     def _decode(self, data):
-        """ Decode queue data """
+        """ Decode queue data
+        
+        @param data: the data to decode
+        """
         pages = data.splitlines()
         return self._filterDuplicates(pages)
 
     def _filterDuplicates(self, pages):
-        """ Filter duplicates in page list, keeping the order """
+        """ Filter duplicates in page list, keeping the order
+        
+        @param pages: list of pages to filter
+        """
         unique = []
         seen = {}
         for name in pages:
@@ -118,6 +133,8 @@
         """ Write pages to queue file
         
         Requires queue write locking.
+
+        @param pages: list of pages to write
         """
         # XXX use tmpfile/move for atomic replace on real operating systems
         data = '\n'.join(pages) + '\n'
@@ -138,6 +155,7 @@
             if err.errno != errno.ENOENT:
                 raise
 
+
 class BaseIndex:
     """ Represents a search engine index """
 
@@ -183,13 +201,16 @@
         os.utime(self.dir, None)
 
     def _search(self, query):
-        """ Actually perfom the search (read-lock acquired) """
+        """ Actually perfom the search (read-lock acquired)
+        
+        @param query: the search query objects tree
+        """
         raise NotImplemented('...')
 
     def search(self, query, **kw):
         """ Search for items in the index
         
-        @param query: the query to pass to the index
+        @param query: the search query objects to pass to the index
         """
         #if not self.read_lock.acquire(1.0):
         #    raise self.LockedException
@@ -331,7 +352,11 @@
             raise
 
     def _do_queued_updates(self, request, amount=5):
-        """ Perform updates in the queues (read-lock acquired) """
+        """ Perform updates in the queues (read-lock acquired)
+        
+        @param request: the current request
+        @keyword amount: how many updates to perform at once (default: 5)
+        """
         raise NotImplemented('...')
 
     def optimize(self):
@@ -339,7 +364,10 @@
         raise NotImplemented('...')
 
     def contentfilter(self, filename):
-        """ Get a filter for content of filename and return unicode content. """
+        """ Get a filter for content of filename and return unicode content.
+        
+        @param filename: name of the file
+        """
         request = self.request
         mt = wikiutil.MimeType(filename=filename)
         for modulename in mt.module_name():
@@ -404,6 +432,13 @@
 
     def __init__(self, request, query, sort='weight', mtime=None,
             historysearch=0):
+        """
+        @param request: current request
+        @param query: search query objects tree
+        @keyword sort: the sorting of the results (default: 'weight')
+        @keyword mtime: only show items newer than this timestamp (default: None)
+        @keyword historysearch: whether to show old revisions of a page (default: 0)
+        """
         self.request = request
         self.query = query
         self.sort = sort
--- a/MoinMoin/search/queryparser.py	Sat Aug 26 18:47:58 2006 +0200
+++ b/MoinMoin/search/queryparser.py	Sat Aug 26 20:20:52 2006 +0200
@@ -221,12 +221,14 @@
     operator = ' or '
 
     def search(self, page):
-        """ Search page with terms, cheap terms first
+        """ Search page with terms
+        
+        @param page: the page instance
+        """
 
-        XXX Do we have any reason to sort here? we are not breaking out
-        of the search in any case.
-        """
-        self.sortByCost()
+        # XXX Do we have any reason to sort here? we are not breaking out
+        # of the search in any case.
+        #self.sortByCost()
         matches = []
         for term in self._subterms:
             result = term.search(page)
--- a/MoinMoin/search/results.py	Sat Aug 26 18:47:58 2006 +0200
+++ b/MoinMoin/search/results.py	Sat Aug 26 20:20:52 2006 +0200
@@ -327,7 +327,6 @@
         if self.hits:
             write(list(1))
 
-            # XXX: Do some xapian magic here
             if paging:
                 hitsTo = hitsFrom + request.cfg.search_results_per_page
                 displayHits = self.hits[hitsFrom:hitsTo]
@@ -401,7 +400,6 @@
         if self.hits:
             write(f.definition_list(1))
 
-            # XXX: Do some xapian magic here
             if paging:
                 hitsTo = hitsFrom+request.cfg.search_results_per_page
                 displayHits = self.hits[hitsFrom:hitsTo]
@@ -678,17 +676,18 @@
         if pages - int(pages) > 0.0:
             pages = int(pages) + 1
         cur_page = hitsFrom / hitsPerPage
-        l = [] # XXX do not use single letter names, esp. not "l"
+
+        textlinks = []
 
         # previous page available
         if cur_page > 0:
-            l.append(''.join([
+            textlinks.append(''.join([
                 f.url(1, href=page_url(cur_page-1)),
                 f.text(_('Previous')),
                 f.url(0)
             ]))
         else:
-            l.append('')
+            textlinks.append('')
 
         # list of pages to be shown
         page_range = range(*(
@@ -696,7 +695,7 @@
                 (0, pages > 10 and 10 or pages) or
                 (cur_page - 5, cur_page + 6 > pages and
                     pages or cur_page + 6)))
-        l.extend([''.join([
+        textlinks.extend([''.join([
                 i != cur_page and f.url(1, href=page_url(i)) or '',
                 f.text(str(i+1)),
                 i != cur_page and f.url(0) or '',
@@ -704,26 +703,30 @@
 
         # next page available
         if cur_page < pages-1:
-            l.append(''.join([
+            textlinks.append(''.join([
                 f.url(1, href=page_url(cur_page+1)),
                 f.text(_('Next')),
                 f.url(0)
             ]))
         else:
-            l.append('')
+            textlinks.append('')
 
         return ''.join([
             f.table(1, attrs={'tableclass': 'searchpages'}),
             f.table_row(1),
                 f.table_cell(1),
                 # textlinks
-                (f.table_cell(0) + f.table_cell(1)).join(l),
+                (f.table_cell(0) + f.table_cell(1)).join(textlinks),
                 f.table_cell(0),
             f.table_row(0),
             f.table(0),
         ])
 
     def formatHitInfoBar(self, page):
+        """ Returns the code for the information below a search hit
+
+        @param page: the page instance
+        """
         f = self.formatter
         _ = self.request.getText
         request = self.request
@@ -748,17 +751,23 @@
         ])
 
     def querystring(self, querydict=None):
-        """ Return query string, used in the page link """
+        """ Return query string, used in the page link
+        
+        @keyword querydict: use these parameters (default: None)
+        """
         if querydict is None:
             querydict = {}
         if 'action' not in querydict or querydict['action'] == 'AttachFile':
             querydict.update({'highlight': self.query.highlight_re()})
         querystr = wikiutil.makeQueryString(querydict)
-        #querystr = wikiutil.escape(querystr)
         return querystr
 
     def formatInfo(self, formatter, page):
-        """ Return formatted match info """
+        """ Return formatted match info
+        
+        @param formatter: the formatter instance to use
+        @param page: the current page instance
+        """
         template = u' . . . %s %s'
         template = u"%s%s%s" % (formatter.span(1, css_class="info"),
                                 template,
@@ -785,6 +794,9 @@
 
         Each request might need different translations or other user
         preferences.
+
+        @param request: current request
+        @param formatter: the formatter instance to use
         """
         self.buffer = StringIO.StringIO()
         self.formatter = formatter
@@ -795,6 +807,15 @@
 
 
 def getSearchResults(request, query, hits, start, sort, estimated_hits):
+    """ Return a SearchResults object with the specified properties
+
+    @param request: current request
+    @param query: the search query object tree
+    @param hits: list of hits
+    @param start: position to start showing the hits
+    @param sort: sorting of the results, either 'weight' or 'page_name'
+    @param estimated_hits: if true, use this estimated hit count
+    """
     result_hits = []
     for wikiname, page, attachment, match in hits:
         if wikiname in (request.cfg.interwikiname, 'Self'): # a local match