Sup. Me again. Yep, this is what I do. Largely, I'm "learning" Advanced Forum - learning and understanding every line of code so that I can properly support it for my own users, but also give you smarter (and actually feature-based) patches and bugfixes. As I do that, I clean. You can expect more of these patches until I "finish", then things will become more substantial. This patch contains:

* init happens really early, so moved that to first.
* shortened the function by removing duplicate code.
* CSS now only loads on forum or node views.
* removed extra menu item stuff that didn't need to be there.
* added description to admin menu item (for proper display on /admin).
* shifted documentation in advanced_forum_markasread_access() around.
* moved the settings page into Drupal Hooks section; it kinda is.
* more whitespace fixes.

And a simple question: Could you tell me more about your hook_theme_registry_alter? $theme_registry isn't being passed in as a reference (&$theme_registry) so what you have in here isn't actually doing anything, it seems - you're modifying only a local copy and then never returning it. Is this code even necessary anymore?

CommentFileSizeAuthor
#1 269578_early_cleanup.patch5.34 KBmorbus iff
early_cleanup.patch5.37 KBmorbus iff

Comments

morbus iff’s picture

morbus iff
they/them
commented
StatusFileSize
new 269578_early_cleanup.patch5.34 KB

Small update.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented

re: hook_theme_registry_alter:

At the time I wrote this bit, this function was completely undocumented. IIRC, merlin suggested I look to Devel's themer for an example. If you look at it here you can see that it's not passed in by reference. TBH, I have no clue how this bit of code is working like that but it does. Must be some sort of magic. :)

For more information on what this is doing, see:

http://shellmultimedia.com/articles/hookthemeregistryalter-advanced-temp...

As I said on IRC, I'll apply this soon as I get the current bit I'm working on sorted.

Thanks,

Michelle

dami’s picture

dami commented

It's because drupal_alter() have some hacks in place to take care of pass-by-reference using call_user_func_array() for dispatch. See api doc for details, and here is an excerpt from php.net on call_user_func_array() itself:

Note: Referenced variables in param_arr are passed to the function by a reference, others are passed by a value. In other words, it does not depend on the function signature whether the parameter is passed by a value or by a reference.

morbus iff’s picture

morbus iff
they/them
commented

dami: beautiful work, thanks. For the sake of clarity, I'd still recommend we change the function to include the reference, just for visual awesomeness. Drupal core, even with cufa(), always uses the reference on the called function, even with the caring hacks implemented.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented

Oh, sure, I can add it. It was just one of those things that I copied from someone who knows more than me and it worked and I was afraid of breaking it. :)

Michelle

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Status: Active » Fixed

Long over due... Finally comitted.

Thanks,

Michelle

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.