Revert "Merge pull request #522 from kleintom/single_result"

This reverts commit 7164d98, reversing
changes made to 97b3f38.

7164d98 should have had a format bump, but it
didn't.

dxr/app.py

@@ -149,19 +149,8 @@ def search(tree):
 
 def _search_json(query, tree, query_text, offset, limit, config):
     """Try a "direct search" (for exact identifier matches, etc.). If we have a direct hit,
-    then return {redirect: hit location}. If that doesn't work, fall back to a normal
-    search, and if that yields a single result and redirect is true then return
-    {redirect: hit location}, otherwise return the results as JSON.
-
-    'redirect=true' along with 'redirect_type={direct, single}' control the behavior
-    of jumping to results:
-        * 'redirect_type=direct' indicates a direct_result result and comes with
-          a bubble giving the option to switch to all results instead.
-        * 'redirect_type=single' indicates a unique search result and comes with
-          a bubble indicating as much.
-    We only redirect to a direct/unique result if the original query contained a
-    'redirect=true' parameter, which the user can elicit by hitting enter on the query
-    input."""
+    then return {redirect: hit location}.If that doesn't work, fall back to a normal search
+    and return the results as JSON."""
 
     # If we're asked to redirect and have a direct hit, then return the url to that.
     if request.values.get('redirect') == 'true':
@@ -172,24 +161,11 @@ def _search_json(query, tree, query_text, offset, limit, config):
             params = {
                 'tree': tree,
                 'path': path,
-                'q': query_text,
-                'redirect_type': 'direct'
+                'from': query_text
             }
             return jsonify({'redirect': url_for('.browse', _anchor=line, **params)})
     try:
         count_and_results = query.results(offset, limit)
-        # If we're asked to redirect and there's a single result, redirect to the result.
-        if (request.values.get('redirect') == 'true' and
-            count_and_results['result_count'] == 1):
-            _, path, line = next(count_and_results['results'])
-            line = line[0][0] if line else None
-            params = {
-                'tree': tree,
-                'path': path,
-                'q': query_text,
-                'redirect_type': 'single'
-            }
-            return jsonify({'redirect': url_for('.browse', _anchor=line, **params)})
         # Convert to dicts for ease of manipulation in JS:
         results = [{'icon': icon,
                     'path': path,
@@ -522,9 +498,7 @@ def sidebar_links(sections):
                            doc.get('annotations', []) + skim_annotations)
                           for doc, tags_in_line, offset, skim_annotations
                               in izip(line_docs, tags_per_line(tags), offsets, annotationses)],
-                'sections': sidebar_links(links + skim_links),
-                'query': request.args.get('q', ''),
-                'bubble': request.args.get('redirect_type')}))
+                'sections': sidebar_links(links + skim_links)}))
 
 
 @dxr_blueprint.route('/<tree>/rev/<revision>/<path:path>')

dxr/plugins/clang/menus.py

@@ -198,56 +198,33 @@ def _more_menu_items(self, (qualname,)):
                'icon': 'reference'}
 
 
-class FunctionRef(_ClangRef):
+class FunctionRef(_RefWithDefinition):
     """Ref for function definitions or references"""
 
     @classmethod
-    def from_condensed(cls, tree, prop):
-        # We ignore the def location clang gives us since it can be wrong, but
-        # its existence means this ref isn't itself a def.
-        search_for_def = prop.get('defloc') is not None
-        return cls(tree,
-                   (search_for_def, prop['qualname'],
-                    'has_overriddens' in prop, 'has_overrides' in prop),
-                   qualname=prop['qualname'])
-
-    def menu_items(self):
-        search_for_def, qualname = self.menu_data[:2]
-        if search_for_def:
-            menu = [{'html': "Jump to definition",
-                     'title': "Jump to definition",
-                     'href': '%s&%s' % (search_url(self.tree.name,
-                                                   '+function:%s' % quote(qualname)),
-                                        'redirect=true'),
-                     'icon': 'jump'}]
-        else:
-            menu = []
-        menu.extend(self._more_menu_items())
-        return menu
+    def _condensed_menu_data(cls, tree, prop):
+        return prop['qualname'], 'has_overriddens' in prop, 'has_overrides' in prop
 
-    def _more_menu_items(self):
-        qualname, has_overriddens, has_overrides = self.menu_data[1:]
-        qualname = quote(qualname)
-        tree = self.tree.name
+    def _more_menu_items(self, (qualname, has_overriddens, has_overrides)):
         yield {'html': "Find declarations",
                'title': "Find declarations of this function",
-               'href': search_url(tree, "+function-decl:%s" % qualname),
+               'href': search_url(self.tree.name, "+function-decl:%s" % quote(qualname)),
                'icon': 'reference'}
         yield {'html': "Find callers",
                'title': "Find functions that call this function",
-               'href': search_url(tree, "+callers:%s" % qualname),
+               'href': search_url(self.tree.name, "+callers:%s" % quote(qualname)),
                'icon': 'method'}
         yield {'html': "Find references",
                'title': "Find references to this function",
-               'href': search_url(tree, "+function-ref:%s" % qualname),
+               'href': search_url(self.tree.name, "+function-ref:%s" % quote(qualname)),
                'icon': 'reference'}
         if has_overriddens:
             yield {'html': "Find overridden",
                    'title': "Find functions that this function overrides",
-                   'href': search_url(tree, "+overridden:%s" % qualname),
+                   'href': search_url(self.tree.name, "+overridden:%s" % quote(qualname)),
                    'icon': 'method'}
         if has_overrides:
             yield {'html': "Find overrides",
                    'title': "Find overrides of this function",
-                   'href': search_url(tree, "+overrides:%s" % qualname),
+                   'href': search_url(self.tree.name, "+overrides:%s" % quote(qualname)),
                    'icon': 'method'}

dxr/plugins/clang/tests/test_direct.py

@@ -65,7 +65,7 @@ def test_scoped_function_name_sensitive(self):
     def test_qualified_function_name_multiple_matches(self):
         """Multiple matches on fully qualified function name should not yield
         a direct result."""
-        self.is_not_redirect_result('MemberFunction::member_FUNCTION(int)')
+        self.is_not_direct_result('MemberFunction::member_FUNCTION(int)')
 
     def test_qualified_type_name_insensitive(self):
         """A unique, case-insensitive match on fully qualified type name
@@ -84,15 +84,13 @@ def test_qualified_type_name_sensitive(self):
     def test_qualified_type_name_multiple_matches(self):
         """Multiple case-insensitive prefix matches on fully qualified type
         name should not yield a direct result."""
-        self.is_not_redirect_result('MemberFunction::InnerCLASS')
+        self.is_not_direct_result('MemberFunction::InnerCLASS')
 
     def test_type_sensitive(self):
         """If the query is an exact match for a class, we should jump there."""
         self.direct_result_eq('MemberFunction', 2)
 
 
-
-
 class MacroTypedefTests(CSingleFileTestCase):
     source = """
         #ifndef MACRO_NAME
@@ -129,7 +127,7 @@ def test_macro_name_sensitive(self):
     def test_macro_name_multiple_matches(self):
         """Multiple matches on a macro name should not yield a direct
         result."""
-        self.is_not_redirect_result('macro_NAME')
+        self.is_not_direct_result('macro_NAME')
 
     def test_typedef_name_insensitive(self):
         """A unique, case-insensitive match on a typedef name should take you
@@ -148,4 +146,4 @@ def test_typedef_name_sensitive(self):
     def test_typedef_name_multiple_matches(self):
         """Multiple matches on a typedef name should not yield a direct
         result."""
-        self.is_not_redirect_result('myTypeDef')
+        self.is_not_direct_result('myTypeDef')

dxr/plugins/clang/tests/test_menus/test_menus.py

@@ -30,21 +30,15 @@ def test_functions(self):
                  'href': '/code/search?q=%2Bfunction-decl%3Aanother_file%28%29'})
 
     def test_function_refs(self):
-        """Make sure qualname-using searches are properly constructed."""
+        """Make sure definitions are found and a representative qualname-using
+        search is properly constructed."""
         menu_on(self.source_page('main.cpp'),
                 'another_file',
                 {'html': 'Jump to definition',
-                 'href': '/code/search?q=%2Bfunction%3Aanother_file%28%29&redirect=true'},
+                 'href': '/code/source/extern.c#7'},
                 {'html': 'Find callers',
                  'href': '/code/search?q=%2Bcallers%3Aanother_file%28%29'})
 
-    def test_function_def_no_jump(self):
-        """Make sure a function definition doesn't get a 'Jump to definition'
-        entry."""
-        menu_item_not_on(self.source_page('extern.c'),
-                         'another_file',
-                         'Jump to definition')
-
     def test_variables(self):
         """Make sure var declarations are found and have a representative sane
         menu item."""
@@ -161,7 +155,7 @@ def test_method_decl(self):
         menu_on(self.source_page('extern.c'),
                 'fib',
                 {'html': 'Jump to definition',
-                 'href': '/code/search?q=%2Bfunction%3AMyClass%3A%3Afib%28int%29&redirect=true'},
+                 'href': '/code/source/extern.c#19'},
                 {'html': 'Find references',
                  'href': '/code/search?q=%2Bfunction-ref%3AMyClass%3A%3Afib%28int%29'})
 

dxr/static_unhashed/css/dxr.css

@@ -557,7 +557,6 @@ mark {
 }
 .bubble.info {
     background: white url('/static/images/info.gif') 12px 0 / 3ex no-repeat;
-    display: block;
 }
 .bubble.warning {
     background: white url('/static/images/warning.gif') 10px -2px / 3.5ex no-repeat;

dxr/static_unhashed/js/dxr.js

@@ -135,6 +135,17 @@ $(function() {
         defaultDataLimit = 100,
         lastURLWasSearch = false;  // Remember if the previous history URL was for a search (for popState).
 
+    // Has the user been redirected to a direct result?
+    var fromQuery = /[?&]?from=([^&]+)/.exec(location.search);
+    if (fromQuery !== null) {
+        // Offer the user the option to see all the results instead.
+        var viewResultsTxt = 'Showing a direct result. <a href="{{ url }}">Show all results instead.</a>';
+
+        var searchUrl = constants.data('search') + '?q=' + fromQuery[1];
+        queryField.val(decodeURIComponent(fromQuery[1]));
+        showBubble('info', viewResultsTxt.replace('{{ url }}', searchUrl));
+    }
+
     $(window).scroll(function() {
         didScroll = true;
     });
@@ -304,7 +315,7 @@ $(function() {
     /**
      * Queries and populates the results templates with the returned data.
      *
-     * @param {bool} [redirect] - Whether to redirect if we hit a direct or unique result.  Default is false.
+     * @param {bool} [redirect] - Whether to redirect if we hit a direct result.  Default is false.
      * @param {string} [queryString] - The url to which to send the request. If left out,
      * queryString will be constructed from the contents of the query field.
      * @param {bool} [appendResults] - Append new results to the current list if true,
@@ -365,7 +376,7 @@ $(function() {
             // tab feature on search pages (Chrome and Firefox).
             cache: appendResults,
             success: function (data) {
-                // Check whether to redirect to a direct or single hit.
+                // Check whether to redirect to a direct hit.
                 if (data.redirect) {
                     window.location.href = data.redirect;
                     lastURLWasSearch = false;
@@ -379,7 +390,13 @@ $(function() {
                     if (addToHistory) {
                         var pushHistory = function () {
                             // Strip off offset= and limit= when updating.
-                            var displayURL = removeParams(queryString, ['offset', 'limit']);
+                            function dropAmp(fullMatch, ampOrQuestion) {
+                                // Drop a leading ampersand, keep a leading question mark.
+                                return (ampOrQuestion === '?') ? '?' : '';
+                            }
+                            var displayURL = queryString.replace(/([&?])offset=\d+/, dropAmp).
+                                                         replace(/([&?])limit=\d+/, dropAmp).
+                                                         replace('?&', '?');
                             history.pushState({}, '', displayURL);
                             lastURLWasSearch = true;
                         };
@@ -524,15 +541,9 @@ $(function() {
     // If on load of the search endpoint we have a query string then we need to
     // load the results of the query and activate infinite scroll.
     window.addEventListener('load', function() {
-        var savedURL, noRedirectURL;
         lastURLWasSearch = locationIsSearch();
         if (lastURLWasSearch) {
-            savedURL = window.location.href;
-            if (/[&?]redirect=true/.test(window.location.href)) {
-                noRedirectURL = removeParams(window.location.href, ['redirect']);
-                history.replaceState({}, '', noRedirectURL);
-            }
-            doQuery(false, savedURL);
+            doQuery(false, window.location.href);
         }
     });
 });

dxr/static_unhashed/js/utils.js

@@ -85,19 +85,3 @@ function extension(path) {
     }
     return file.substring(lastDotPos + 1);
 }
-
-/**
- * Remove each parameter name in the params array from url's query parameters.
- */
-function removeParams(url, params) {
-    var i, paramRegex;
-    function dropAmp(fullMatch, ampOrQuestion) {
-        // Keep a leading question mark, drop a leading ampersand.
-        return (ampOrQuestion === '?') ? '?' : '';
-    }
-    for (i = 0; i < params.length; i++) {
-        paramRegex = new RegExp('([&?])' + params[i] + '=[^&]+');
-        url = url.replace(paramRegex, dropAmp);
-    }
-    return url.replace('?&', '?');
-}

dxr/templates/layout.html

@@ -35,10 +35,8 @@
                         <label for="query" class="query_label visually-hidden">Find</label>
                         <input type="text" name="q" {% if should_autofocus_query %}autofocus {% endif %}value="{{ query }}" maxlength="2048" id="query" class="query" accesskey="s" title="Search" placeholder="Search {{ tree }}" autocomplete="off" />
                         <div class="zero-size-container">
-                        {% block bubble %}
                           <div class="bubble">
                           </div>
-                        {% endblock %}
                         </div>
                         <section id="search-filter" class="search-filter">
                           <button type="button" class="sf-select-trigger" aria-label="Select Filter">

dxr/templates/text_file.html

@@ -5,22 +5,6 @@
 
   {% set state_eof = True %}
 
-  {% block bubble %}
-    {% if bubble %}
-      <div class="bubble info">
-        {% if bubble == 'single' %}
-          There was only 1 search result, so I took you straight to it.
-        {% else %}
-          Showing a direct result.
-          <a href="{{ url_for('.search', tree=tree, q=query) }}">
-          Show all results instead.</a>
-        {% endif %}
-      </div>
-    {% else %}
-      {{ super() }}
-    {% endif %}
-  {% endblock %}
-
   <div id="annotations">
     {% for line, annotations in lines %}
       <div class="annotation-set" id="aset-{{ loop.index }}">