Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Thanks for producing role_weights, it provides a key tool in improving the "rolability" of Drupal.
A small request. I find it awkward that a structured array is returned by role_weights_get_highest(). I always want just the rid, and so have to include extra code to get at it. If we want to get the role name, that is always readily available through $user->roles or user_roles().
I'd be happy to submit a patch making this change.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | return_rid_only.patch.txt | 1.02 KB | pfaocle |
Comments
Comment #1
pfaocleHmm, that sounds OK to me. There was a similar discussion here about the return value. I think I agree - returning the role rid should be enough. Budda is using the module - any thoughts?
(nedjo - your input is much appreciated! I'm stuck in Windows until my new router arrives (Speedtouch modems, ArchLinux and CHAP anyone?), so will review/test your patches as soon as I can! Please feel free to roll a patch for this one too! Cheers!)
Comment #2
buddaJust a rid is what I originally expected, but having the text name available without having to do an additional query was handy.
I used it in the path_access module, and specifically made use of the role name returned, but looking back at the code briefly I'm not sure why i didn't just use the rid returned?!
I've not got time to change and test my module at the moment to work with just rid value.
Is there any benefit to just returning the rid from the function? ...apart from being able to use it inline for function calls/logic.
Comment #3
pfaocleThe only other thing I can think of was in my original use of the module - in a theme in which I needed the role name specifically. If we lost the role name returned here, then the theme function would have to do a bit more work with $user->roles or user_roles().
Not a major reason not to drop the role name returned, just a thought. Here's a patch against current cvs for further review anyway.
Comment #4
pfaocle... and the patch.
Comment #5
pfaocleApplied to HEAD: role_weights_get_highest() will now return simply a role rid.
Please update your code/modules as neccessary if you can help out testing the HEAD version. Thanks!
Comment #6
(not verified) commented