Closed laurenancona closed 6 years ago
@laurenancona Thanks for reporting - I agree this should be triggering as you point out.
At the moment, there are two stages:
@places
) to execute the frontend JS codeThe triggers themselves are fine — indego bike share
would pass — so it probably needs a new check for the "indego" brand name but should also include "bike" or "ride", to exclude unrelated "indego" queries.
In psuedo-code, that might be something like:
return unless (place is valid and in @places array) OR (query includes "indego" and ("bike" or "ride"));
Alternatively, a more readable form could be:
if (place is valid and in @places array) {
return '';
} else if (query includes "indego" and ("bike" or "ride")) {
return '';
}
That's the best solution I can come up with. If you'd like to make a PR I'm sure we can get some eyes on it.
I'll have to look back at the code but, as I recall, the bikeshare search functionality is location based since this is not the only city for which bikeshare info is published on DDG. So an indication of the city in some way is the first trigger, then the bikeshare trigger, which could be the brand name. That's why something like "philly indego" and "Philadelphia indego" will work. It could also be the reverse. But, yeah, it could easily be revised. This is just the best we came up with at the time since we were working on a multi-city approach. But iterate away! On Sat, Jul 30, 2016 at 2:52 AM Daniel Davis notifications@github.com wrote:
@laurenancona https://github.com/laurenancona Thanks for reporting - I agree this should be triggering as you point out.
At the moment, there are two stages:
- Initial triggers to execute the backend Perl code
- A check for place names (@places) to execute the frontend JS code
The triggers themselves are fine — indego bike share would pass — so it probably needs a new check for the "indego" brand name but should also include "bike" or "ride", to exclude unrelated "indego" queries.
In psuedo-code, that might be something like:
return unless (place is valid and in @places array) OR (query includes "indego" and ("bike" or "ride"));
Alternatively, a more readable form could be:
if (place is valid and in @places array) { return ''; } else if (query includes "indego" and ("bike" or "ride")) { return ''; }
That's the best solution I can come up with. If you'd like to make a PR I'm sure we can get some eyes on it.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/duckduckgo/zeroclickinfo-spice/issues/2878#issuecomment-236348636, or mute the thread https://github.com/notifications/unsubscribe-auth/AEgw1HbytnaCM4VcwU952tw8V7qUcn2eks5qavSdgaJpZM4JEyIc .
Hey @laurenancona! I took a quick look at the code and this is the line I was referring to in my last comment:
One of the triggers is the place. It's what separates DDG from returning the philly bikeshare info from other cities. I suppose, an easy and probably not so appropriate fix would be to just add "indego" as a "place." Otherwise, a rewrite may be necessary, but if you take that approach, we might want to make sure the other bikeshare cities behave the same way.
On Sun, Jul 31, 2016 at 2:53 PM, Lauren Ancona notifications@github.com wrote:
Hi @AcriCAA https://github.com/AcriCAA!
Thanks all, I'll take a look.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/duckduckgo/zeroclickinfo-spice/issues/2878#issuecomment-236449535, or mute the thread https://github.com/notifications/unsubscribe-auth/AEgw1JgXEbBYhHQBBND4zvEIbscHL8oQks5qbO8MgaJpZM4JEyIc .
Closing due to DDH entering Maintenance Mode. We are now only accepting issues for essential bugs/fixes.
I'm not super familiar with the way triggering works with places - whether any permuting of keywords + place triggers is happening under the hood or not - either way, this isn't triggering on
indego bike share/ing
(official name of the program).Happy to PR, but wanted feedback to avoid keyword redundancy.
IA Page: http://duck.co/ia/view/bike_sharing_indego_phl Maintainer: @AcriCAA