Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search #183

Merged
merged 11 commits into from Jan 2, 2015
Merged

Search #183

merged 11 commits into from Jan 2, 2015

Conversation

Malabarba
Copy link
Collaborator

Implements searching.
Accepts query, tags, and excluded tags.

Would be nice to have tag completion here, but obviously relies on #137.

@vermiculus
Copy link
Owner

I don't suppose there is any way we can test this… I honestly can't think of a way that is guaranteed not to break. We could take a bet on the search results of, say, grandma on TeX.SX and see if we get the appropriate question's ID.

@Malabarba
Copy link
Collaborator Author

That should be fine, I think. Search for grandma, check if any of the results is the one we expected.

There are two very basic sanity checks we can do:

  • perform a simple search, just to make sure it doesn't error.
  • do a search with excluded-tags, but without tags nor query, and make sure it does error.

Other reasonable tests:

  • search for a popular tag on SO (like c++) and verify the length of the returned vector (which should be a full page).
  • make an obviously crazy query, and make sure it's empty.
  • search for the org tag on Emacs.SE, but exclude another tag like ox-html, and check that none of the results contain the excluded tag.

Interactively, the user is asked for SITE and QUERY. With a
prefix argument, the user is asked for everything."
(interactive
(let ((site (sx--interactive-site-prompt))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be

(if current-prefix-arg
    (sx--interactive-site-prompt)
  (or sx-question-list--site
      (sx--interactive-site-prompt)))

Or something to this effect? (I feel like the logic above can be simplified, though.)

(setq sx-question-list--next-page-function
(lambda (page)
(sx-search-get-questions
sx-question-list--site page
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the question-list--site variable means that the user can switch site in the search-results buffer.
It's not an incredibly useful feature, but who knows... it could come in handy to repeat a search on a different site.

Arose from comment discussion at [1].  This function allows for
simplified syntax when the use case wants to retrieve a site token in an
interactive context.  See docstring for details.

[1]: https://github.com/vermiculus/sx.el/pull/183/files#r22403919
Interactively, the user is asked for SITE and QUERY. With a
prefix argument, the user is asked for everything."
(interactive
(let ((site (sx--maybe-site-prompt current-prefix-arg))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this look?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@Malabarba
Copy link
Collaborator Author

You had asked something about completing read. We should use it once we have the tag lists. Right now there's nothing to complete against.

@vermiculus
Copy link
Owner

Right now there's nothing to complete against.

Fair enough :) We should have tag lists pretty soon -- work on the bot can continue now that #187 is (hopefully) ready.

Branch: search
@vermiculus
Copy link
Owner

Also, sorry about any Branch: cruft -- it's something I need to do for work in my commits. It won't be the case with the new job 😄 (On the other hand, I'll be using svn... 😦)

vermiculus added a commit that referenced this pull request Jan 2, 2015
@vermiculus vermiculus merged commit 9a420bf into master Jan 2, 2015
@vermiculus vermiculus deleted the search branch January 2, 2015 17:13
@Malabarba
Copy link
Collaborator Author

How good is your svn knowledge?

@vermiculus
Copy link
Owner

Poor, but TortoiseSVN will likely be good enough for my purposes. Knowing how I act though in new environments, I'll likely just pick up bits and pieces over the next few months until I have a good knowledge of the internals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants