> I want to call attention to this recent comment from Liz Smith that may have
> been missed:
>
>
https://github.com/php-fig/fig-standards/pull/490#issuecomment-94197428
>
> The renaming of Stream[able]Interface may be problematic long-term. Yes it
> would be in a different namespace but that's still potentially confusing,
> especially if the interfaces are not identical. (They likely would have
> subtle differences, at least.) I would recommend reverting this rename and
> letting internals "own" the name "StreamInterface".
I'm conflicted on this one.
I was against renaming in the first place, but several people were
adamant about the change, and I didn't have enough of an argument to
stand on. That said:
- As you and others already noted, it would be namespaced.
- What nobody else has brought up is that there are very few
interfaces(*) in core with the suffix "Interface", and I highly
suspect we won't see many, if any more. A more likely naming in Core
will be Streamable (ah, the irony!) or StreamWrapper (note: the class
in the documentation is an *example*, and not something defined in
core!).
* DateTimeInterface exists mainly because DateTime *already existed*;
the same is true of SessionSaveHandlerInterface.
Essentially, while I understand Liz's concern, I also think the
liklihood of an interface with the "Interface" extension in core is
unlikely, which means there won't really be a conflict.
> ----
>
> There's still a reference to "host segment" in the "Host header" section,
> even though Matthew's writeup above says that it should have been switched
> to "component".
Thanks! PR opened:
https://github.com/php-fig/fig-standards/pull/536
> ----
>
> The UploadedFileInterface::move() method feels poorly named. move() from?
> to? The parameter is simply named $path, which is not very descriptive. I
> would recommend simply renaming this method to moveTo(), which would give it
> a really nice readable style:
>
> $request->getUploadedFiles()['myfile']->moveTo($destination_path);
How about "move($destination)" or "move($target)"?
> ----
>
> The spec talks about the "tree structure" of uploaded files. To be honest, I
> have no idea what that means. I rarely deal with$_FILES directly, much to my
> joy. :-) What tree is it? The example implies it's a tree caused by PHP's
> stupid design of $_FILES, but the whole point seems to be to not deal with
> that stupid design.
>
> So the spec should either:
>
> 1) Clarify in much more detail what tree we're talking about, and how it
> works.
I'm really not sure what more we can do to detail this. Section 1.6
(
https://github.com/php-fig/fig-standards/blob/master/proposed/http-message.md#16-uploaded-files)
shows what $_FILES does for structure, vs what is expected (with the
expected being the normalized tree). (See the first two code examples
in that section.)
> 2) Normalize the upload data to a non-tree list.
>
> The most important part here, IMO, being that a consumer trying to read
> uploaded files needs as few if-statements as possible in order to do so. As
> is, I couldn't actually write a conforming implementation since I don't know
> what I'm supposed to do.
The idea for a consumer is that they can do this:
$file = $request->getUploadedFiles()['avatars']['logo'][1];
and they'll get an UploadedFileInterface instance.
For a conforming implementation, you would need to traverse the
$_FILES array and create UploadedFileInterface instances; in the case
of an array of files (again, see section 1.6), you would need to
aggregate the data appropriately.
I'll find you in IRC to see if we can hash out more verbiage to clarify this.
> ----
>
> The rewording to allow with*() methods to return $this is very subtle. It
> should be called out explicitly in the prose text somewhere, as otherwise I
> would have missed that it now allows return $this or in what cases.
https://github.com/php-fig/fig-standards/blob/master/proposed/http-message-meta.md#new-instances-vs-returning-this
Do you have any suggestions on making that more clear?
> ----
>
> getHeaderLine() now returns string|null. I generally find |null returns to
> be a huge PITA. At least it's a primitive and not an object so there's no
> "call to method on non-object" issues, but I still worry that it will force
> more "wrap stuff in if(is_null)) BS" cases on clients. Is there a reason to
> not return an empty string? In this case the distinction is very subtle, but
> the fewer nulls in my code the happier I am. The UriInterface methods all
> specify empty string if not defined, so we should do the same here.
I'd be fine with that change; it would still make conditionals work as expected:
if (! $message->getHeaderLine('X-Foo')) {
}
though, if empty strings are the return value, I'd likely want to
rewrite that to:
if (empty($message->getHeaderLine('X-Foo')) {
}
I've created a PR:
https://github.com/php-fig/fig-standards/pull/537
> ----
>
> Can we normalize all of the first-line descriptions of methods to a single
> line? A few wrap in the middle, which screws with some documentation
> standards and documentation parsers. (Eg, see getReasonPhrase())
I've created a PR for this:
https://github.com/php-fig/fig-standards/pull/538
> ----
>
> UriInterface::getPath() allowing a leading / or not = *cries*. The
> description of where it's different talks only of the empty case, ie, "" and
> "/". However, as written it would also allow returning both
> "/some/page/here" and "some/page/here". Can we at least normalize the
> non-empty case to a leading /? Otherwise, this is another case of "every
> project now has another 19 if statements in it, increasing the cyclomatic
> complexity of the PHP universe by several million". :-)
@Tobion and a few others made strong cases for this. I'm not terribly
happy with it, but if some member projects have demonstrated needs, I
think it makes sense to keep it in.
Side note: When integrating this change into Conduit, I discovered
that I only needed to change 4 lines of code to accommodate it. The
implication is that with good test cases, projects can likely find
expedient solutions.
> ----
>
> I recall there being some discussion about getQuery() returning a string vs.
> an array, since the data structure is naturally an array. Why is it still
> returning a string? The array seems far more useful to me, and it just
> forces everyone to parse that themselves. Or am I missing some other part
> of the spec now?
There are two related functionalities:
- UriInterface::getQuery()
- ServerRequestInterface::getQueryParams()
For server-side applications, you'll be using the second, as it's a
direct member of the request, and gives you the array structure you're
expecting. This is what you want to use server-side.
UriInterface::getQuery() is for returning the query string *for the
URI*. Having this return an array may not be a good idea; as Michael
Dowling has noted elsewhere, while the structure we have in PHP
typically can map to an associative array, query strings used for
other servers *may not*. As such, you need access to the string itself
so you can parse it somehow. This method is one you'll typically only
use for client applications.
--
Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/