Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I would like to be able to configure that the include pattern has priority over the exclude pattern.
The module now assumes the opposite.
Anyone that could point me in the right direction to start on this? Or this is a really bad idea?
Comment | File | Size | Author |
---|---|---|---|
#1 | regex_logic-1206276-1.patch | 863 bytes | monotaga |
Comments
Comment #1
monotaga CreditAttribution: monotaga commentedweseze,
I also agree that the "inclusion" of certain links as external should trump those links "excluded" (and therefore considered internal). It strikes me that a user is less likely to desire an internal link to be considered external, so these special cases should be given priority in the logic. Furthermore, by listing "Exclude links matching the pattern" first in the UI, the impression given to the user is that the this regex will be evaluated first and then the "Include links matching the pattern" will be evaluated, so that a URL matching both regex would ultimately be considered external.
If there's disagreement on this issue, then rather than committing a patch like the one I propose (attached), we could easily add a radio group in the admin UI asking which regex should be given priority. Wouldn't be hard at all.
Thoughts?
Comment #2
quicksketchI'd prefer to avoid adding an option and stick with the approach that seems to make the most sense. I'm fairly sure I didn't give as much thought to this as you have, considering I don't think I've ever used the inclusion/exclusion patterns at all, they were added by feature request. Let's just get some appropriate testing on this and we can add it to the next version.
Comment #3
monotaga CreditAttribution: monotaga commentedGreat. Thanks, quicksketch!
Test this patch, folks!
Comment #4
quinns CreditAttribution: quinns commentedI found I was able to work around this problem by installing the Menu Attributes module.
Comment #5
quinns CreditAttribution: quinns commentedComment #6
pbfleetwood CreditAttribution: pbfleetwood commentedThe patch fixes the problem.
Looking at the code, there really isn't anything that can go wrong with it; so it really ought to go into the next commit. Anyone who uses lots of modules ends up with far too many patches to manage. I have several that are for really simple code changes like this one.
Thank you.
Comment #7
pbfleetwood CreditAttribution: pbfleetwood commented@quinns, please don't change an issue that has a patch awaiting review form 'Needs Review' to 'Active'. People need to see that there is a patch pending. I think most people would prefer a committed fix over a workaround, which is not to say that workarounds aren't appreciated.
Comment #8
pbfleetwood CreditAttribution: pbfleetwood commentedI should mention that I tested the code in the patch (I applied the code manually) with 7.x-1.12. It has been working flawlessly. We've upgraded to Drupal core v.7.14 and this module continues to work well. Please apply this code change to the 7.x branch of this module. Thank you.
Comment #9
weseze CreditAttribution: weseze commentedDoes the patch apply cleanly to the D7 version as well?
Comment #10
elachlan CreditAttribution: elachlan commentedLooks like its RTBC to me.
Comment #11
quicksketch@elachlan: Feel free to commit to all versions. Thanks for your work on this module!
Comment #12
elachlan CreditAttribution: elachlan commentedCommitted to Git.
Comment #13
elachlan CreditAttribution: elachlan commentedComment #14
elachlan CreditAttribution: elachlan commentedRegression in all versions.
Comment #15
elachlan CreditAttribution: elachlan commentedFixed in all branches. Committed to Git.