duckduckgo / zeroclickinfo-goodies

DuckDuckGo Instant Answers based on Perl & JavaScript
https://duckduckhack.com/
Other
979 stars 1.76k forks source link

POTUS: Should not trigger for "vice president of the united states" #1863

Closed countlurk closed 8 years ago

countlurk commented 8 years ago

@numbertheory Searching for "vice president of the united states" triggers this instant answer, giving the incorrect output of Barack Obama


IA Page: http://duck.co/ia/view/potus

jophab commented 8 years ago

Its because the potus is triggered if the query contain the substrings "president of the united states" or "president of the us" image

hemanth commented 8 years ago

It would nice to fix the RE to have a beings with check.

mintsoft commented 8 years ago

@hemanth should be as simple as adding a ^ to the front

hemanth commented 8 years ago

@mintsoft Yes, I am aware of that, giving a chance to the person who reported to do a PR ;)

moollaza commented 8 years ago

Looks like this still needs fixing @hemanth, care to fix it?

hemanth commented 8 years ago

@moollaza Sure, what the expected triggers here? In the RE just a beings with for who\s+(is|was)\s+the\s+ is enough?

moollaza commented 8 years ago

The triggers are OK, but we shouldn't just be stripping out expected query parts. We need to ensure that the whole query matches our regex:

handle query_lc => sub {
  return unless m/^(?:who (?:is|was) the )?(\d+(?:st|nd|rd|th))?(?:potus|president of the (?:us|united states|usa)) (\d+)?$/;
  ...

Something like that...

We should add tests to ensure this.

Visual: https://www.debuggex.com/r/Vif4k-n0uUt61lwv

moollaza commented 8 years ago

I would also just use triggers any => "potus", "president ... ";

hemanth commented 8 years ago
return unless my ($tense, $index, $text) = $_ =~ ^(?:who (?:is|was) the )?(\d+(?:st|nd|rd|th))?(?:potus|president of the (?:us|united states|usa)) ?(\d+)?$

Something like ^ and then fetch the required index from @presidents ?

moollaza commented 8 years ago

@hemanth sure -- not sure you need to capture anything besides the index though.

We can derive is/was if index is/isn't the current president.

hemanth commented 8 years ago

"Sorry, no hit for your instant answer"

Well with the modified R.E I see ^ missing something?

Update, I think it would make more sense to do a PR and take it from there. #1913

Where do I see the debug logs? The duckpan server doesn't respect print?

@moollaza

moollaza commented 8 years ago

@hemanth use warn -- you'll see it printed in DuckPAN's server output

hemanth commented 8 years ago

Used use warn; but it's not triggering instant answer for any of the valid phrases: POTUS et.al