On Sat, Jul 1, 2017 at 4:42 PM, Zach Laine via Boost
* Is there a reason why buffer_cat_view and buffer_prefix_view are distinct types?
Yes, buffer_cat_view and buffer_prefix_view are two completely different things. A buffer_cat_view represents a buffer sequence of buffer sequences, while a buffer_prefix_view is just a range adapter over a single buffer sequence.
I think you just need to make a single type that owns a sequence of buffers, and maintains a pair of iterators or indices into the owned sequence.
You've more or less described buffer_prefix_view. A rough description of buffer_cat_view would be "a type that owns a heterogenous list of buffer sequences, whose iterators reflect the current position within a variant consisting of each iterator types from the list of buffer sequences." Quite different :)
* Similarly, as a user I'd prefer not to have string_body as a distinct type (unless it's a convenience typedef for dynamic_bodystd::string). If std::string is not a model of DynamicBuffer, I think DynamicBuffer could stand a slight reformulation.
std::string does not model DynamicBuffer, which itself is a Net-TS concept and thus immutable from Beast's perspective. Therefore beast::http::basic_dynamic_body cannot work with std::string and beast::http::string_body becomes necessary . I will likely add beast::http::basic_string_body to allow the allocator to be customized.
* I get why message<> has a Body template parameter, but why is Fields configurable? There is no message<> instantiation under examples/ or include/ that uses anything other than basic_fields<> that I could find. Do people ever use something else? Again, a brief rationale would be useful. I saw the "HTTP Message Container" page, btw, but while it suggested someone might want a different Fields template param, it didn't indicate why.
I have not done extensive work in this area of the library yet but I assure you it is a useful feature. A custom Fields implementation could support only a subset of all possible fields (for example it might only be capable of representing Content-Length, Transfer-Encodoing, Server, and User-Agent), storing the field name in a small integer enum instead of keeping the entire text. It could store Content-Length as an integer instead of a string. Custom Fields could use its own strategy for storing the field/value pairs. Most importantly, a custom Fields could be used to adapt another library's message model into something that Beast can consume. Or to allow a Beast message to be consumable by another library with a proper wrapper. All of these contemplated use-cases require the user to also provide a subclass of basic_parser, as the beast::http::parser is only usable with beast::http::basic_fields. These use-cases seem exotic but I feel that the niches they satisfy will come up and have merit. Think embedded devices or Internet of Things. Creating examples of custom Fields is on my to-do: https://github.com/vinniefalco/Beast/issues/504
* Allocators are gross. I know this is a controversial position,
I agree, that is controversial! And I'm not entirely happy with the interface of AllocatorAwareContainer but it is what it is. Which is to say it is standardized.
I think they're far more trouble than they're worth. Since you already have buffers that seem to me to cover all the major allocation strategies already, can't you get away with adding a buffer adapter that takes any random access container as its underlying storage, and be done with it?
That already exists, you can use beast::buffers_adapter on any linear buffer (including one provided by a container) and then use that adapter with beast::http::basic_dynamic_body. There are only a few AllocatorAwareContainer in Beast, those include basic_flat_buffer, basic_multi_buffer, but most importantly http::basic_fields. I don't think removing allocator support in http::basic_fields is such a good idea, especially when one of the examples already demonstrates how performance may be improved through its use: https://github.com/vinniefalco/Beast/blob/78a065ba39836d91d7e70d93de7f9140f5... Folks who want to get the most out of Beast will be taking advantage of Beast's allocator support so I must respectfully disagree with your position.
* The Quick Start contains its subsections in full, and then there are TOC links to each subsection separately. Probably one or the other would do.
This is intended, I want every example to have a ToC entry so that users can go to it quickly. Maybe if they forget the section it was in then the Toc entry will jog their memory.
* Each of the example links takes me to a GitHub page for the file linked. That should change to inline source like the Quick Start code.
Hmm I don't know about that, the server-framework is enormous. Including the example source code in bulk would make it hard to navigate. And I plan on more examples, I don't think that scales. Of course, if the consensus is that this change reflects an improvement then I will make it gladly. I have opened this issue for discussion: https://github.com/vinniefalco/Beast/issues/565
The same thing is true of all the reference documentation.
I'm not sure what you mean here.
* HTTP Crawl seems to be doing synchronous reads. Is there an example elsewhere that does a bunch of reads like this asynchronously?
Not yet but I can add something like that: https://github.com/vinniefalco/Beast/issues/566
* At least http_server_small.cpp (and maybe others? I didn't go back and check) has only Christopher M. Kohlhoff as an author. Vinnie, you probably need a copyright audit of all your files.
The copyright is correct, Chris is the author of http_server_small.cpp and http_server_fast.cpp
* I found the mix of function and type entries in "Table 6. Buffer Algorithms" to be initially quite confusing. Maybe separate them?
How about changing the title to "Buffer Algorithms and Types"?
* The buffer_prefix_view documentation in "Table 6. Buffer Algorithms" says "This is the type of buffer returned by buffer_prefix.", but 2/3 overloads with that name return something else.
Yeah that's a little confusing. I'll sort it: https://github.com/vinniefalco/Beast/issues/567
* HTTP Message Container says this: ... If it needs to access the other fields, it can do so using tag dispatch ... Why would I have a third overload and use tag dispatching? Most users are going to be really confused by what I quoted above, I think.
You're right. I think I had functions like this in mind: https://github.com/vinniefalco/Beast/blob/78a065ba39836d91d7e70d93de7f9140f5... The doc is a bit confusing, I will fix it.
* Please add an explicit Rationale section for smaller design choices. The FAQ and design justification sections that exist are fine, but they mostly cover the biggest-picture items. Each of those small design choices are going to go out the window if you can't remember a strong argument for one of them 5 years from now, when LEWG are asking you why a certain interface is the way it is.
Good advice but either vague or broad in scope (every design choice?), if there are specific design choices that should be explained then this becomes more actionable: https://github.com/vinniefalco/Beast/issues/569 Thanks