Add a searchPrefix option to rbautocomplete.

Review Request #6807 — Created Jan. 18, 2015 and discarded

Information

Review Board
master

Reviewers

There are some installs of Review Board where the user list is so large,
with so many similar names, that it's necessary to find a new way of
differentiating them when searching for their entries for autocomplete.

An example of this is Mozilla's instance of Review Board. Mozilla's
convention is to have the fullnames of their users followed by their
IRC Nick, prefixed with a colon, for example:

Mike Conley (:mconley)

RB's autocomplete widget isn't prepared to handle searches for ":mconley",
and map them to the actual Review Board username "mconley". This works
around that issue.

Really, this just supports the perpetuation of a habit that Mozillians
have developed over time. It's true that they could just start entering
the IRC nick and get the result they're looking for, but the colon prefix
is what they're used to.

In a local instance, I made sure that searching for normal usernames (without a search prefix) worked properly to make sure we didn't regress. I tested both selecting an autocomplete entry by pressing "enter" and "tab", as well as selecting an entry with the mouse.

Then, in our RBMozUI extension, where we've forked rbautocomplete with this patch, I used searchPrefix to make sure the colon prefix work around still worked. I was able to autocomplete with the colon prefix, and was able to select a reviewer both by pressing "enter" and "tab", as well as selecting one with the mouse.

Description From Last Updated

In line with our style guide, shouldn't there be only one var declaration and it should occur at the top …

brenniebrennie

Shouldn't have a trailing comma here.

daviddavid

The var definition for trimmed should actually be inside here (since there's an anonymous function). I think the comment was …

daviddavid

Should use ===

daviddavid
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/lib/js/ui.autocomplete.js
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/lib/js/ui.autocomplete.js
    
    
  2. 
      
brennie
  1. 
      
  2. Show all issues

    In line with our style guide, shouldn't there be only one var declaration and it should occur at the top of the function?

    Given that this is a fork of some jQuery library, I'm not sure if this applies or not (and/or whether it applies to the var statements above).

    1. Thanks for the review! Which part of the style guide are you referring to? I checked here and didn't see anything in there about JavaScript, or location of variable declarations.

      I'm also not sure what we want to do with this fork of ui.autocomplete. It's kind of a jungle in here.

    2. Apparently we don't have a js style guide :). I was going by the existing code and changes (à la https://reviews.reviewboard.org/r/6768/diff/ diffFragmentQueueView.js).

    3. We generally try to make jshint happy. For this file I wouldn't worry too much because it started out as such a big mess.

    4. Alright then, feel free to drop this issue.

    5. Or rather, I'll drop it. Forgot I can :p

    6. It should though, yes. We declare vars at the top of the function (due to variable hoisting in JS) and we use only one var to help minifiers and just for consistency.

    7. Ah, alright. I'll update the review request then. I forgot about var hoisting. I can't wait until ES6 let is used everywhere.

  3. 
      
brennie
  1. Ship It!
  2. 
      
mike_conley
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/lib/js/ui.autocomplete.js
    
    
    
    Tool: Pyflakes
    Ignored Files:
        reviewboard/static/lib/js/ui.autocomplete.js
    
    
  2. 
      
david
  1. 
      
  2. Show all issues

    Shouldn't have a trailing comma here.

  3. Show all issues

    The var definition for trimmed should actually be inside here (since there's an anonymous function). I think the comment was really that words and result should be defined in a single var statement at the top of the trimWords function.

  4. Show all issues

    Should use ===

  5. 
      
david
Review request changed
Status:
Discarded