Hi, A few quick responses,
On Tue, Jan 19, 2010 at 15:51, Dean Michael Berris <[email protected]> wrote: > Hi Jeroen, > > On Tue, Jan 19, 2010 at 1:08 AM, Jeroen Habraken <[email protected]> wrote: >> >> I just pushed a major rewrite of the URI parser to my github fork, and >> there are a couple of things to take into consideration for the >> moment. Also, Dean, could you please verify I pushed it to the right >> branch and everything this time. >> > > I see it in your 0.5-devel branch. Hopefully that'd be easier to merge > into my 0.5-devel branch but I have a few concerns. > >> Here's a quick list, in no specific order: >> - HTTP support is broken at the moment, which breaks a lot of other >> code, this should be fixed soon and is a priority. > > When do you expect this to be available? I'm curious to see how that's > going to shape up granted that there's no more way to customize the > parsing of the specific parts. I've nearly got this working, so either today or tomorrow I hope. >> - The parser doesn't have support for IP's yet, thus >> scheme://localhost/ works, scheme://127.0.0.1/ doesn't, whilst both >> are valid > > These were valid before IIRC and there must be tests to show that they > are supported. They are indeed valid, but I've not written the parser rules for them. I shall add a couple of tests reflecting this. >> - A lot more tests need to be added > > I would better prefer that there be tests written first before any > work is done so that I can look at the tests and see whether the > feature to be added makes sense. The way I do it is I force test > failures first before I go write functionality. > > Please look up Test Driven Development and see if you can follow that > process instead. For one thing I will not commit a merge if tests are > failing. Yes, and I'm aware of TDD, but unfamiliar with Boost.Test. There are a couple of tests I've written outside the framework which need to be merged, for this I'll have to look into Boost.Test though (not that that's a bad thing). >> - There's been an API change, protocol() has been renamed to scheme() >> as it's what the name the RFC uses, and according to Wikipedia scheme >> and protocol are two different things > > Alright, but I see that 'rest()' was removed too. The value of getting > the raw scheme-specific part of a URI is something I personally > require and prefer be still there. The newer RFC (http://www.ietf.org/rfc/rfc3986.txt) which deprecates RFC1738 has no notion of a scheme-specfic-part, which makes this rather hard. Could you please explain why you need it? > This is the main reason why I put that protocol()/scheme() and rest() > functions in the basic_uri type. The raw() accessor should also be > there so that a URI's raw string representation can be retrieved even > after the parts have been parsed already. I don't believe it was there initially, but it's trivial to add and makes sense. >> - Documentation needs to be updated, and an the example should be >> expanded to show the use of scheme() for example > > Yes, if you can do that too before I merge your work that would be > most appreciated too. Sure. >> - Another API change is coming up, which will expose the optional >> parameters as boost::optionals, instead of returning an empty default >> value >> > > Please update the tests and documentation to show that things have changed. Of course. > Thanks for putting in the work and I hope this helps! > > -- > Dean Michael Berris > cplusplus-soup.com | twitter.com/deanberris > linkedin.com/in/mikhailberis | facebook.com/dean.berris | deanberris.com > I know this is a rather large overhaul which influences the whole library, please bear with me and thank you for the response. Jeroen ------------------------------------------------------------------------------ Throughout its 18-year history, RSA Conference consistently attracts the world's best and brightest in the field, creating opportunities for Conference attendees to learn about information security's most important issues through interactions with peers, luminaries and emerging and established companies. http://p.sf.net/sfu/rsaconf-dev2dev _______________________________________________ Cpp-netlib-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/cpp-netlib-devel
