Edgewall Software

Ticket #1269 (closed defect: fixed)

Opened 4 years ago

Last modified 3 years ago

Search an expression with CamelCase does not work

Reported by: oliver@… Owned by: cboos
Priority: normal Milestone: 0.10
Component: search system Version: 0.8.1
Severity: normal Keywords: search CamelCase review
Cc:

Description

If I search an expression with CamelCase I get a wiki page. I dont get a search result page. Is that the right behaviour ?

Attachments

quickjump-escape.diff (0.8 KB) - added by athomas 3 years ago.
Quickjump escape
quickjump.diff (2.3 KB) - added by athomas 3 years ago.
Quickjump only when using top search bar
quickjump+missing.diff (4.0 KB) - added by athomas 3 years ago.
As above plus add missing class to milestone and reports
quickjump+missing-r3431.diff (5.7 KB) - added by cboos 3 years ago.
Same as above, but the style is different: the quickjump results are more integrated with the other results, but still stand out on top of them.

Change History

Changed 4 years ago by cboos@…

At least, that's the intended behavior.

But I see your point. If your a looking for, e.g. a class name, you would like to see the commit messages or the tickets talking about that class, not the empty Wiki page for that name.

A simple fix would be to redirect to the wiki page only if such a page already exists.

In all cases, simply prefixing the CamelCase name by one or more spaces bypasses the redirection and does a regular search for that name.

Changed 4 years ago by cmlenz

The proper way to escape the quick search functionality is to prefix with an exclamation mark, e.g. CamelCase.

Changed 4 years ago by Matthew Good <trac matt-good net>

What cmlenz meant to say was !CamelCase

(! in the wiki escaped CamelCase from being a wiki link)

Changed 3 years ago by david.tulloh@…

This is particularily annoying when you use the advanced search and specifically uncheck wiki pages.

For example I was looking for PythonHandler? and ModPythonHandler? tickets to see if they relate to a problem I have. Getting empty wiki pages instead of tickets was very unexpected.

Changed 3 years ago by cboos

In the trunk, the search module has been reworked a bit, and the "quick jump" feature is now missing.

As this has to be reimplemented, we should take this opportunity to fix the problem you raised.

Possible solutions:

  • only quick jump if the corresponding object exists, otherwise revert to normal search.
  • same as above, but in addition, don't jump to the object, but list it at the top of the results. This would also make natural to simply enter a number, and see the links to the corresponding ticket, changeset and report having that number (if they exist)

The problem is that some people used the existing behavior on purpose, in order to create new Wiki pages quickly... But that feature was never advertised, and it's not a good Wiki practice anyway, so maybe it's better to change/fix that.

Changed 3 years ago by cmlenz

  • component changed from general to search system

Changed 3 years ago by cboos

  • owner changed from jonas to cboos
  • status changed from new to assigned
  • severity changed from normal to minor
  • milestone set to 0.10

So what about doing the quick jump only if the page exist?

  • trac/Search.py

     
    2222from trac.util import TracError, escape, format_datetime, Markup 
    2323from trac.web import IRequestHandler 
    2424from trac.web.chrome import add_link, add_stylesheet, INavigationContributor 
    25 from trac.wiki import IWikiSyntaxProvider 
     25from trac.wiki import WikiSystem, IWikiSyntaxProvider 
    2626 
    2727 
    2828class ISearchSource(Interface): 
     
    253253        elif len(kwd) > 1 and kwd[0].isupper() and kwd[1].islower(): 
    254254            r = "((^|(?<=[^A-Za-z]))[!]?[A-Z][a-z/]+(?:[A-Z][a-z/]+)+)" 
    255255            if re.match (r, kwd): 
    256                 return self.env.href.wiki(kwd) 
     256                if WikiSystem(self.env).has_page(kwd): 
     257                    return self.env.href.wiki(kwd) 
    257258 
    258259    # IWikiSyntaxProvider methods 

Changed 3 years ago by cboos

On top of r3350, doing a quickjump only if the page (or any other object) exists would be implemented like this:

  • trac/Search.py

     
    218218            return req.href.browser(kwd) 
    219219        link = wiki_to_link(kwd, self.env, req) 
    220220        if isinstance(link, Element): 
    221             return link.attr['href'] 
     221            if 'missing' not in link.attr.get('class_', ''): 
     222                return link.attr['href'] 
    222223 
    223224    # IWikiSyntaxProvider methods 

This will ignore redirections to objects that are known to be missing.

Also, it was suggested somewhere that we could disable the quickjump entirely in the search page, only have it when the search is triggered from the small form present on every page... opinions?

Changed 3 years ago by mgood

-1 on the last patch. Basing application logic on presentation information such as the CSS class is quite fragile.

Changed 3 years ago by cboos

I see this as an incremental approach: before r3350, you'd have to parse the resulting HTML string to retrieve that information.

In the future, the link variable above will not be an Element, but rather some kind of WOM element (as in Wiki Object Model), in which this missing information could be an attribute.

e.g.

    link = WikiParser(self.env, req).parse(kwd)
    if isinstance(link, Link) and not link.missing:
        return link.href

Changed 3 years ago by athomas

Seems there are two issues here:

  1. Disabling quick jumps (solved with the ! prefix)
  2. An extensible/consistent way of adding quick jumps

This ticket only deals with the first of these and is easily solvable:

  • trac/Search.py

     
    164164        if query: 
    165165            page = int(req.args.get('page', '1')) 
    166166            redir = self.quickjump(req, query) 
    167             if redir: 
     167            if query.startswith('!'): 
     168                query = query[1:] 
     169            elif redir: 
    168170                req.redirect(redir) 
    169             elif query.startswith('!'): 
    170                 query = query[1:] 
    171171            terms = search_terms(query) 
    172172            # Refuse queries that obviously would result in a huge result set 
    173173            if len(terms) == 1 and len(terms[0]) < 3: 

Perhaps we should close this for 0.10 and move discussion of 2 to another ticket?

Personally I'm -1 on adding a conditional check for existence because of the extra magic. For example, I'd prefer r9999 to tell me the changeset doesn't exist rather than perform a search.

Changed 3 years ago by athomas

Quickjump escape

Changed 3 years ago by athomas

Ignore that last comment, patch attached.

Changed 3 years ago by cboos

alect: is your patch supposed to fix a problem, or is it just an optimization? Anyway, that doesn't help with the issue of someone entering ClassName? and ending up with a Create ClassName? wiki page.

Changed 3 years ago by cboos

Forgot that your point 2. (An extensible/consistent way of adding quick jumps) was addressed, I believe. See r3350.

Changed 3 years ago by athomas

It depends on your definition of fix I guess...the default behaviour of coming up with the create page can be circumvented by prefixing the page name with a !. eg. !ClassName, but the default is still the create page.

I personally like this behaviour, but maybe it could be extended with a configuration option trac.quick_jumps = false which would disable quick jumps entirely?

Changed 3 years ago by cboos

What I meant is that without your patch, Trac already behaves like this: the ! will negate the wiki syntax, and no link will be created, therefore redir will be None... The optimization you do is to simply by-pass the Wiki formatting when there's a leading "!". Why not, I'm OK with that.

But again, I think that this ticket is not about using the "!", rather with the newcomer's surprise when searching for something and ending up with a create wiki page, as david.tulloh illustrated it above (hm... no comment: links on p.e.c yet :( ).

Quick jumps are pretty useful, so I don't see the point of disabling that feature entirely just to workaround the above problem, which is only a minor inconvenience.

What has been proposed so far:

  1. Doing a search if the target of the quickjump is known as missing (option which doesn't seem to be popular... ok)
  2. Disabling the quickjump on the search page, but keeping it when the search is triggered from the top search box

In addition, I'd like to propose a variation on 2:

  1. On the search page, retrieve the link given by the quickjump, but don't redirect to it, rather present a link to it before the search results, which will be gathered regardless of the redir value. The quickjump toplevel search box behaves as usual.

What do you think?

Changed 3 years ago by athomas

(Yes, quite true)

I like option 3, seems like the best of both worlds to me :)

Changed 3 years ago by athomas

Quickjump only when using top search bar

Changed 3 years ago by athomas

As above plus add missing class to milestone and reports

Changed 3 years ago by athomas

Added two attachments. The first simply implements option 3. I like it a lot.

The second is that, plus add the missing class to the links for milestone and report links. Should probably do the same for source links?

Changed 3 years ago by athomas

Also (and I know this is not supported by IE6) perhaps we should use the following rather than programatically adding the "?" to missing links? Opinions?

a.missing:after { content: "?" }

Changed 3 years ago by cboos

Should probably do the same for source links?

Careful with that... see #2891

I'll test the patches asap

Changed 3 years ago by cboos

Same as above, but the style is different: the quickjump results are more integrated with the other results, but still stand out on top of them.

Changed 3 years ago by cboos

  • keywords review added

I improved a bit the solution proposed by athomas. See attachment:quickjump+missing-r3431.diff.

Also, I'm +1 for the a.missing:after idea, since this would make it possible to print pages without that ? sign appended. It would also make it possible to avoid ? completely simply by making CSS customizations.

Changed 3 years ago by cboos

Note: need to change slightly the implementation, in order to support InterTrac's usage of quickjump. This can be done by having a hidden parameter noquickjump on the Search page, instead of having a quickjump hidden parameter in the header's quick search form.

Changed 3 years ago by cboos

  • status changed from assigned to closed
  • resolution set to fixed
  • severity changed from minor to normal

Latest patch applied in r3469, with the above notice taken into account.

Add/Change #1269 (Search an expression with CamelCase does not work)

Author



Change Properties
<Author field>
Action
as closed
Next status will be 'reopened'
to The owner will change from cboos. Next status will be 'closed'
 
Note: See TracTickets for help on using tickets.