Can you add some screenshots of what this looks like?

Hrm. Does this render to a form with a bunch of input elements?
I think we want a more google-like experience with just the single search box that can search everything. Part of that might require the query parsing, though...

Hmm. We'll have to figure out where the right place to put this is for non-dev environments.

What is this file used for, and how is it referenced? I don't see this filename in any of your code.

Haystack loads HAYSTACK_CONNECTIONS from settings.py in its init.py. Overwriting HAYSTACK_CONNECTIONS through load_site_config() does not work for search because the new values are not reinitialized. See https://github.com/toastdriven/django-haystack/blob/master/haystack/init.py#L39

This is a little bit confusing to me, because when I see "depends" I think "dependency".
How about "An up-to-date index is required to provide useful search results. See :ref:search-indexing for more information"

Can we call this get_diff_filenames?

I'm not sure where the best place to put it is (here, in the view, in SearchQuerySet, etc), but at some point we should definitely filter based on result.is_accessible_by(request.user)

Revert this removed line.

How about just making filenames a set to begin with?

This should probably render a template with the error message instead of giving a (mostly) blank page. This text should also me marked for localization.

Do you need to include this method if it doesn't do anything?

Including this in here makes haystack a hard dependency. We should probably include it in setup.py and change the docs/UI to only talk about installing whoosh.

This was poorly localized to begin with, but it's worse now. How about using blocktrans instead?

{% blocktrans %}
<b>{{hits}}</b> results for <b>{{query}}</b>
{% endblocktrans %}

It's probably ok to have an empty div in the case that there are no page links. I don't think we need this check.

The text in here should be inside {% trans %} tags.

Oh, one more thing.

We should probably limit sqs here to some maximum number of results (maybe a couple hundred?). This could be a big performance hit if someone searches for "a" or "the" and it comes back with 500000 results, and we fetch them all from the db to make this list.
If we limit the list of results, we should include a small note on the results page telling the user that there are more potential hits but they need to make their query more specific.

A handful of comments, but I also have another thought:
We have some setups out there that are calling manage index, and I don't want these to break. As it is, upgrading to this release will cause errors. I think, ideally, we'd continue to have a management command called index that supported the old parameters, but called the new ones. We could mark this as deprecated for, say, 2.0, and then 2.1 or 3.0 or whatever could remove it, giving people time to migrate.

Can we just depend on these as part of Review Board? I think, given that these are pure-Python, that it's worth just offering search out of the box.

Can we move the import up around the others?

If we make these dependencies, this whole function and all conditionals involving it can just go away.

Mind adding a comment just explaining what we're doing here?

Not sure I love having this here, but I understand why..
Can we rename this to get_all_diff_filenames?

You can reduce the number of queries to 1 by doing:
for filediff in FileDiff.objects.filter(diffset_history=self.diffset_history_id):

In fact, you can probably simplify this whole thing even more by doing something like:
q = FileDiff.objects.filter(diffset_history=self.diffset_history_id)
return set(q.values_list('source_file', 'dest_file', flat=True))

This is a start, but it's not enough.
We also need to limit this only to those review requests that are not private to private repositories or invite-only groups.
Fortunately, we have some nice query methods that take care of this for you. Just change this to:
return self.get_model().objects.public(
    status=None,
    extra_query=Q(status='P') | Q(status='S'))

That should do the job.

This template is old, but the indentation for template tags and HTML should be independent.
HTML should be indented relative to their parent tags by one space.
Template tags should be indented 1 relative to their parent template tag, but that indentation should occur within the tag body. So:
{% block ... %}
{%  if foo %}
{%   for blah %}
{%   endfor %}
{%  endif %}
{% endblock %}

No spaces around variable names. So, {{hits}}, not {{ hits }}.

No spaces.
What is this file for, btw?

Should be one statement, and immediately after the {% extends %}.

Should apply indentation rules here too.

Does assigning these to self.results_per_page/max_search_results not work?
Might as well also move this below the search_enable check.

Can you prefix this comment with "XXX:" to make it searchable as something we should fix in the future?
This comment should probably also move to be right above the result_ids = set(...) line.

Hmm. Does SearchQuerySet order by relevance? This probably loses the ordering.
We might want to turn it into a normal list, and then call distinct() on the QuerySet to remove duplicates.

This looks really good.
So, the duplicates are fixed. What else is left for this change? Tell me again what the issue is with the number of records?

Can you put a blank line above this?

Ship It!