Wedge

Re: too many blocks -- yeah, I suppose index.template.php is really a special case... And a dozen extra blocks aren't going to kill performance -- it's all very fast. Although we might wanna do a get_defined_functions() once and for all, and then test through isset() instead of function_exists()...

Re: injection, as I answered somewhere else, one can easily hook their function into 'display_post_done' and use $counter to determine the position in the list. :)

Re: addon code, I'm looking through it again (still over 3000 lines to go through, WTF...), and noticed the menu entry for addons has a lot of content commented out. I suppose you're aware of that, but you might wanna clean it up... I generally dislike commented out code being committed without a good reason :P

Yeah, I am aware of that code, and as the rest of the add-on system is implemented, that'll be phased out in stages (along with Packages and Subs-Packages)

Okay, good then.

Looking through Subs-BBC. You added a hook right before the code that caches parsed data. Wouldn't it make more sense to add it after, so that the hook is always called, even if the post is cached? (One would have to call the same hook much earlier when returning the cached data, of course.)
Although I'm not exactly sure of the point of caching parsed data for only six minutes... It's very likely to be called only once or twice in that time...

Also, I'm doing the language files...

fatal_install_error_minmysql and minphp have two sprintf params, %1$d and %2$s, which both correspond to a version number. Wouldn't it make more sense to have %1$s for the first as well...?

fatal_install_error_missinghook has a typo (avaialble). Fixed on my side. I'm saying that but there's bound to be typos in my translation as well, given how fast I'm working on it... :whistle:

fatal_install_remove_missing is using the term 'disabled' instead of 'removed' or 'uninstalled'.

Why did you remove the media_welcome mention from the help files...? (media_admin_settings_welcome_subtext)

Template stuff:

+      // Add-on buttons. They're floated right, so need to be first. Besides which, the floating means they need to be in reverse order :/

Why not use inline-blocks then...?

I think that's all for now... I'm committing my changes thus far. 2800KB left to check......... :( :( :(

Oh, and do you know your Dragooon-contributed code in rev 1032 uses the word 'plugin'? :P :P

rev 1032 has:

+            break; // I'd use break 2 but that's deprecated in PHP 5.4.

Actually they removed break $var, not break (constant number)... :^^;:

(And yes it's not something that was added in 1032.)

The same function with 'break 2' defines a variable that's never used -- $can_use... I'm removing it.
Just saying ;)

The idea was to parse the post then cache it, which seemed more logical to me than force whatever changes there were every time. In the case which prompted that, the hook makes DB queries.

Yes, it should be $s not $d, oops.

Fairly sure I just reused the same language string there, out of laziness. The whole disable/remove part is distinctly less polished.

I changed the mention because it referenced Modifications.english.php which doesn't exist now.

They're addon-item which derives from admin-item which is used in the main menu, and should be defined as inline block. The idea is that they're inline, but the entire container is right aligned, so the container is floated right, but I found the only way to make then work properly was to do it how I did it; if one item has 3 buttons and another only has 2, that item will have a space to the right so enable/disable buttons always line up.[nb]Though in hindsight, it's only ever going to be 1 icon or 2, not 2 or 3, because the combinations are (do-enable + remove, or do-disable + optional settings)

I'm no template designer :P

Re PHP 5.4 stuff, I read it as break x being broken, not break $var but it's easily possible that I misread it.

I must have missed the use of plugin at the time but it was quite late at night :P And $can_use... pass, can't remember, sorry.

There are 7 instances of plugin in ManageAddons.php :p

As long as it's not an oversight... :)Fixed locally. (Wasn't sure whether to fix in rev 1034.)Fixed locally.Oh... All right. I forgot about that one... But then, where do we add this string? Lol. I liked it because it allowed me to have multilingual welcome messages in the media area without the chance of them being removed with an upgrade... As of now, it's no longer possible to define a string, except by implementing the 'lang' tag, which isn't the case yet...There's a stackoverflow topic about it and it says break x isn't broken. I would be surprised myself that they remove support for it. Might as well remove support for break entirely, then... Break is spaghetti code! Man up and keep going through the loop until the end, computers are here to compute for you!! :lol:I still haven't given up... :niark:It was in your very first commit of the file. I suspect it's something you used before, but then you did it differently and forgot to remove the var, just like I forgot to remove the $context['layer_hints'] definition a few days ago.It's time to pull the PLUG... :eheh:

I think you're in the minority on that one, unfortunately. I suspect most people don't really care :(

As for $can_use, you're probably right.

And really, if you do want to rename it, just do it! :P As per WP:BOLD, and not even with a [citation needed] in sight :P

Yeah, I suppose they don't care...
Let's just say it'll be up to that lang tag.

WP:BOLD I can understand through googling, but citation needed? Don't see the relationship with what we're talking about -- even with Wikipedia in mind.
Still, I can't suddenly rename everything when most people voted for 'add-on', including *you*... That makes a clear majority for add-ons. I just hate that hyphen, and even if I remove it, the word looks odd to me :P

The whole cit-needed was simply to indicate that it was Wikipedia not WordPress ;) I guess I'm thinking of meta things right now (it's all Dr Who's fault with the finale this weekend!)

I'm fine with renaming it to addon (instead of add-on, even though my autocorrect hates it)

We are talking about renaming to plugins not addons :P

But you said you hated the hyphen, so remove it :P

At this point I'm beyond worrying about what it's called and concentrating on making it work and work well. Perhaps we should put in a poll for plugin, add-on and Wedget or Wedgelet, and disallow mod because it implies modifying files, which add-ons shouldn't generally be doing.

I noticed you used this a few times:

filetype($addonsdir . '/' . $folder) == 'dir'

Why not just use is_dir()...? Is it any faster?

Another commented out line of code (multiple, actually):
//libxml_use_internal_errors(true);

Doesn't say whether you're planning to restore them or delete them, so I thought I'd mention them...

Typo...? (Used twice.)
// Users might be insert 5 or 5.3 or 5.3.0.

I can't remember why I used filetype instead of is_dir, performance will be comparable though.

The libxml lines will be restored in due course, but while I'm working on it and debugging it, I wanted any libxml/SimpleXML errors to actually be thrown, as opposed to caught separately (which is what will eventually happen)

Nope, not a typo. The idea is that users, including plugin add-on contributing authors, are inherently lazy. If they require PHP 5.4, say, should they use 5.4 or 5.4.0? version_compare expressly considers 5.4 to be different to 5.4.0 so the idea is to normalise them as best possible before calling version_compare, and well, I did the PHP ones first, then copied it for MySQL.