Re: SPL Autoload Register Patch

On Jul 27, 2009, at 8:08 AM, Peter Cowburn wrote:

2009/7/27 Brandon Savage <brandon@brandonsavage.net>:

On Mon, Jul 27, 2009 at 7:43 AM, Hannes Magnusson <hannes.magnusson@gmail.com> wrote:

On Mon, Jul 27, 2009 at 13:36, Brandon Savage<brandon@brandonsavage.net

wrote:

On Mon, Jul 27, 2009 at 7:28 AM, Hannes Magnusson <hannes.magnusson@gmail.com> wrote:

Don't top-post. On Mon, Jul 27, 2009 at 13:24, Brandon Savage<brandon@brandonsavage.net> wrote:

On Mon, Jul 27, 2009 at 7:21 AM, Hannes Magnusson <hannes.magnusson@gmail.com> wrote:

Don't top-post :) On Mon, Jul 27, 2009 at 13:14, Brandon Savage<brandon@brandonsavage.net> wrote:

On Mon, Jul 27, 2009 at 3:39 AM, Hannes Magnusson <hannes.magnusson@gmail.com> wrote:

On Sat, Jul 25, 2009 at 18:59, Brandon Savage<brandon@brandonsavage.net> wrote:

This is my first time submitting a patch, but I was asked to add to the documentation and so here goes. Please let me know if I did anything incorrectly.

We try to stay away from "you this" and "you that".. The documentations should be using 3rd person :]

I fixed those issues. Let me know if there are any others.

Function names should be wrapped in <function> elements, i.e. "<function>spl_autoload_register</function> allows.."

Done. Can we make a comprehensive list of anything else that's wrong so we can avoid the back-and-forth?

() should not be used with <function>. The <function> element automatically generates those. <function> is also missing around __autoload. Lines should preferably not be longer then 80chars. Can't think of anything else.

Alright. Characters limited to 80, and () removed.

I'm unsure how the WS between <function> and the actual content of it will be handled, it will probably be trimmed away.. but it'll make grepping for <function>spl_autoload_register</function> a total nightmare. I'd recommend either break the 80chars limit "rule" or move <function> down to the new line too. -Hannes

I was concerned about that but I wasn't sure whether the 80-char rule was hard or soft. I moved it up and broke the rule, but that should make grepping easier (and keep us from finding out how WS affects it).

There is still whitespace to the left of the function name inside the <function> tags. Also, I'm not particularly keen on the way this is worded. The whole "If there must be..." sentence feels clunky and gives a feeling of "we'd prefer it if you didn't, but..." which I'm sure is not intentional. I'd prefer to be more concise with something like "<function>spl_autoload_register</function> allows multiple autoload handlers to be registered" (I used "handlers" simply because methods can be used, not just functions). Does it "effectively" create a queue, or actually do so? Finally, does spl_autoload_register run through the registered autoloaders in the order in which they're defined, or the order in which they are registered? Sorry for being particularly picky over a single paragraph!!

I spent a bit going over the wording with someone else before I submitted it. According to what I understand, it creates a stack, but doesn't behave in the way a stack does (last in first out). Effectively it's a queue but I believe it places the functions on the stack, so I'm not really sure. I do know it goes through then in a first-in, first-out sort of fashion. I don't see the WS you're referring to.