On Wed, Mar 19, 2014 at 8:24 PM, Amir Taaki <genjix@???> wrote:
> btw why does uri_parse() need to be publically exposed when you already
> have uri_decode()? can we hide that implementation detail?

If anyone wants to extend the bitcoin URI scheme in the future, this
provides a way to get at the raw parameters. Otherwise, people would
be forced to patch uri_decode for every little thing, including things
that may be experimental or not appropriate for libwallet.

On Wed, Mar 19, 2014 at 8:21 PM, Amir Taaki <genjix@???> wrote:
> Pull request: https://github.com/spesmilo/libwallet/pull/9
> Accept if you like my changes.

Ooooh, boost::optional is exactly what I was looking for. I couldn't
find anything like it in the standard library, so I gave up and went
with the bool thing.

I come from an embedded firmware environment where memory is precious,
so I always go for things like the visitor pattern to avoid allocating
data structures. Your way is more straightforward, but now we have
this uri_parse_result that needs to be thrown away.

Some of the tests you commented out were actual edge cases, so I will
go back through and make sure we have comprehensive tests again.

Apart from the missing unit tests, I like your changes and will be
incorporating them shortly.

In the mean time, I figured out a better to handle malformed URI's.
The original solution felt wrong, since it gives the space character
special treatment for no good reason. My new solution adds a "strict"
flag, which determines whether the parser will follow the escaping
rules exactly, or whether it will completely ignore them in the case
of an unencoded parameter. Since I've pushed these changes up to
master, your pull request won't apply cleanly, so I might as well fix
the unit tests as long as I'm solving merge conflicts.

-William