There's an important core patch: #374463: Alter comment links.

After having core link_alter available, we could reduce code complexity with this patch. Rebuilding links multiple times in rendering will improve performance.

Please support the original issue to get committed as it solves many issues in interaction between
forum access, advanced forum, and flag.

After this the commit, the setup is pretty sane again and beautifully drupal like.
Port to advanced_forum for 2.x-dev branch outstanding.

CommentFileSizeAuthor
#5 advanced_forum_link_alter_2.patch2.19 KBmiro_dietiker
advanced_forum_link_alter.patch1.59 KBmiro_dietiker

Comments

michelle’s picture

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

I appreciate the patch but I do wish you would have used the existing issue... Now I've got notes in one place and a patch in the other. Will link the other here and mark it dupe.

#594206: Investigate moving theme_links modifications up to http://api.drupal.org/api/function/hook_link_alter

I don't know what more support you're expecting me to give on the core issue... It's already RTBC.

Michelle

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

oops, didn't realize that one.

Right then, time to wait. Poor killed kittens...

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Needs work

This should use

version_compare(VERSION, '6.16', '>')

to conditionally execute the code in the proper location. You cannot guarantee that everyone runs the latest version of core. Will save you a couple of bogus bug reports.

michelle’s picture

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

Thanks, I didn't even realize you could check the core version. I don't plan on using this patch as is, anyway, but it's a start. That whole area needs to be re-thought out, not just moved. It was some of my earlier code and isn't all that good.

Michelle

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
StatusFileSize
new advanced_forum_link_alter_2.patch2.19 KB

Added version with core check, assuming this will get into 6.17.
No more cleanup regarding filesize/code overhead but still reduced code execution.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs work » Needs review

Please note that the core was fixed right now.
Ready to update the module?

michelle’s picture

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

LOL! It _just_ got fixed today. No, I'm not ready yet. It's a holiday weekend so not sure how much I'll be on the computer but this issue is a priority for me.

Michelle

locomo’s picture

locomo commented

subscribe - sorry, Michelle - i know you hate these subscribes, but i want to keep an eye on this one

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Version: 6.x-1.x-dev » 6.x-2.x-dev

Setting the branch correctly so I don't keep losing track of this. Hope to get to it soon.

Michelle

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Version: 6.x-2.x-dev » 6.x-1.x-dev
Status: Needs review » Patch (to be ported)

This is now fixed in 2.x. Bumping it back to 1.x to see if scoobie wants to either backport my method or use miro_dietiker's patch.

Michelle

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Title: Switch to hook_link_alter after core update » Use hook_link & alter for manipulating node/comment links
Version: 6.x-1.x-dev » 6.x-2.x-dev
Status: Patch (to be ported) » Active

Looks like I was too hasty. There are issues with this and Node Comments. I've been working all day on fixing them but am not quite there, yet. Taking this issue back to the 2.x queue until it's all done.

Michelle

michelle’s picture

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

Well, I improved the situation some and it now works with Node Comment but the links are in a different order on the topic starting node compared to the reply nodes. Need to fix that, yet.

Michelle

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

Glad to see you working (hard) on this, michelle, thanks! :-)

AntiNSA’s picture

AntiNSA commented

subscribe

kompressaur’s picture

kompressaur commented

subscribe

michelle’s picture

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

Well, I don't know when it happened but the links are in the right order now. Possibly something on the nodecomment end.

Michelle

Status: Fixed » Closed (fixed)

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

Wolfflow’s picture

Wolfflow commented

Subscribe