Skip to content
This repository has been archived by the owner on Oct 15, 2022. It is now read-only.

Bitcoin Spice #320

Merged
merged 12 commits into from Nov 21, 2013
Merged

Bitcoin Spice #320

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7bf6d2c
bitcoin spice
jmg Nov 8, 2013
4efafd4
some fixes and improves
jmg Nov 12, 2013
f14f604
some changes
jmg Nov 12, 2013
8e64522
javascript lint
jmg Nov 12, 2013
1ddb309
allow currency anywhere in the parameters array
jmg Nov 12, 2013
3dfcd5a
added currency name in header
jmg Nov 12, 2013
ff5eabb
Bitcoin: No cache on api calls
jmg Nov 12, 2013
6539b77
Bitcoin: symbols at the end for the corresponding currencies
jmg Nov 12, 2013
7ece2e1
Bitcoin: Added test cases for different queries and triggers. Added 2…
jmg Nov 14, 2013
826aeac
Bitcoin: New trigger and better validations on query parameters
jmg Nov 18, 2013
2e7dd37
Bitcoin: Improve query parsing with decodeURIComponent
jmg Nov 19, 2013
66c89d9
Bitcoin: Allow just one parameter. Query should be either [trigger] […
jmg Nov 19, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 27 additions & 0 deletions lib/DDG/Spice/Bitcoin.pm
View file
Open in desktop
@@ -0,0 +1,27 @@
package DDG::Spice::Bitcoin;

use DDG::Spice;

primary_example_queries "bitcoin";
secondary_example_queries "bitcoin eur", "bitcoin cny";
description "Get Bitcoin Exchange Rate";
name "Bitcoin";
source "http://blockchain.info";
code_url "https://github.com/duckduckgo/zeroclickinfo-spice/blob/master/lib/DDG/Spice/Bitcoin.pm";
topics "economy_and_finance";
category "conversions";

attribution github => ['https://github.com/jmg','Juan Manuel García'],
email => ['jmg.utn@gmail.com','Juan Manuel García'];

spice to => 'http://blockchain.info/ticker';
spice wrap_jsonp_callback => 1;

Copy link
Member

Choose a reason for hiding this comment

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

We can probably add triggers start => "bitcoin exchange in"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I've no problem. But would the triggers any => "bitcoin", "bit coin", "bitcoin exchange", "bit coin exchange" also work? Or you think it's better to just trigger it when the query starts explicitly with "bitcoin exchange in"?

Copy link
Member

Choose a reason for hiding this comment

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

Oh weird. Looks like when I added triggers start => "bitcoin exchange in", some trigger words in triggers any stopped working. It might be a bug. But for now, having both triggers start => "bitcoin exchange in" and triggers startend => "bitcoin", ... should work for our purposes here. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I tried and wasn't working well so didn't know if this was the bug or "triggers start" and "triggers any" couldn't be together. Will use triggers start => "bitcoin exchange in" and triggers startend => "bitcoin" then.

Copy link
Member

Choose a reason for hiding this comment

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

Nice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe I could open a bug for the weird behavior of "trigger any" and "trigger start" working togheter

Copy link
Member

Choose a reason for hiding this comment

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

@jmg this is actually a known bug -- we're working on it :)

triggers any => "bitcoin", "bit coin";
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about adding "bitcoin exchange", "bit coin exchange" here?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, "bitcoin market", too. But I'm not sure how common that is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added "bitcoin exchange", "bit coin exchange" :)


handle remainder => sub {

return $_;
};

1;
11 changes: 11 additions & 0 deletions share/spice/bitcoin/bitcoin.css
View file
Open in desktop
@@ -0,0 +1,11 @@
#spice_bitcoin .spice_pane {
display: inline-block;
width: 40%;
vertical-align: top;
margin: 10px 10px;
font-size: 24px;
}

#spice_bitcoin .spice_pane span {
font-weight: bold;
}
2 changes: 2 additions & 0 deletions share/spice/bitcoin/bitcoin.handlebars
View file
Open in desktop
@@ -0,0 +1,2 @@
{{title}}: <span>{{symbol}}{{price}}</span>

48 changes: 48 additions & 0 deletions share/spice/bitcoin/bitcoin.js
View file
Open in desktop
@@ -0,0 +1,48 @@
function ddg_spice_bitcoin (api_result) {

if (!api_result) return;

var DEFAULT_CURRENCY = "USD";

var query = DDG.get_query();
var params = query.split(/\s+/g);

if (params.length > 1) {
var currency = params[1].toUpperCase();
Copy link
Member

Choose a reason for hiding this comment

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

What if I search "jpy bitcoin"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This doesn't contemplate that case so it will return the amount in dollars as it's default when it doesn't recognize the currency. But maybe we can search for the currency parameter either on the first or on the second position inside the query array. Would that work?

Copy link
Member

Choose a reason for hiding this comment

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

That sounds reasonable. I think having "jpy bitcoin" and "bitcoin jpy" both working would be great. :)

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! I improved it. Now it searches through all the parameters array so you put the currency anywhere on the query string and you'll get the right answer. All "jpy bitcoin", "bitcoin jpy", "get bitcoin jpy", "bitcoin convert jpy", etc will work.

Copy link
Member

Choose a reason for hiding this comment

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

Awesome!

} else {
var currency = DEFAULT_CURRENCY;
}

var prices = api_result[currency];
if (!prices) {
prices = api_result[DEFAULT_CURRENCY];
}

var buy = {
price: prices.buy.toFixed(2),
symbol: prices.symbol,
title: "Buy"
}
Copy link
Member

Choose a reason for hiding this comment

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

Small nitpick: Semicolon to be consistent. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry about those, I just ran jslint to ensure consistency :)


var sell = {
price: prices.sell.toFixed(2),
symbol: prices.symbol,
title: "Sell"
}

Spice.render({
header1 : "Bitcoin Exchange Prices",
source_name : "Blockchain",
spice_name : "bitcoin",
source_url : 'http://markets.blockchain.info/',
force_favicon_url : "http://blockchain.info/favicon.ico",

template_frame : "twopane",
template_options : {
left : { template: "bitcoin", data: buy },
right : { template: "bitcoin", data: sell },
Copy link
Member

Choose a reason for hiding this comment

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

Trailing comma.

},
force_no_fold : true,
force_big_header : true
Copy link
Member

Choose a reason for hiding this comment

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

You can add force_favicon_url: "https://icons.duckduckgo.com/i/bitcoin.org.ico" to add a favicon.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, it might be better to use "http://blockchain.info/favicon.ico" since they are the provider.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, no problem. Just did it!

});
}
18 changes: 18 additions & 0 deletions t/Bitcoin.t
View file
Open in desktop
@@ -0,0 +1,18 @@
#!/usr/bin/env perl

use strict;
use warnings;
use Test::More;
use DDG::Test::Spice;

ddg_spice_test(
[qw( DDG::Spice::Bitcoin )],
'bitcoin' => test_spice(
'/js/spice/bitcoin/',
call_type => 'include',
caller => 'DDG::Spice::Bitcoin'
),
Copy link
Member

Choose a reason for hiding this comment

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

Add tests please for all the different variations. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do! 👍

);

done_testing;