Posted by joelstein on October 8, 2009 at 3:29pm
| Project: | Devel |
| Version: | 7.x-1.x-dev |
| Component: | devel |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
On the "Switch user" block, I want the option to hide the list of users, and only show the autocomplete form. However, if "number of users to display in the list" is configured at 0, then the block doesn't appear at all.
I'm submitting a patch which displays the block when "number of users" is 0, so that the autocomplete form remains. If someone wants the block to disappear, disabling it is better and faster, since it avoids the unnecessary processing and database queries.
Additionally, I couldn't find any obvious reason to for the logic in devel_switch_user_list() to have its own function, so I merged it with the devel_block_switch_user() function. If that is a problem, I can roll another patch to maintain the two separate functions.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| devel.module.patch | 4.64 KB | Idle | FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in devel.module_24.patch. | View details |
Comments
#1
Sorry, I realized that I tagged this with the wrong version. Also, I added a field description on the block configuration form to explain this new behavior.
#2
The separate devel_switch_user_list() function might be useful to a module like admin_menu. I don't see eliminating this function as an improvement.
Your patch should be against the -dev version, otherwise we can't apply it.
The patch is surprisingly big. I agree with you that the current behavior is not useful, but I'd expect your change to be only a few lines...
Remember that our focus is at least as much on D7 as on D6. We won't commit a patch to D6 and not to D7. If your patch were really short and sweet, you might find someone willing to port it to D7, but this is not likely for a big one like yours. Your best bet for getting this in is to provide two slim patches against both -dev versions.
#3
Well, it looks big because I merged the two functions. But it's really quite simple. I'll resubmit a slimmer patch against dev. Thanks for the feedback.
#4
Here's two slimmed-down patches.
#5
It still seems overly complex.
<?phpif (!empty($links) || user_access('switch users')) {
?>
should do the same thing, no?
I have some doubts whether
theme('links', array());is safe though... Anyone?#6
Okay, here are the two patches, both just one line long. Sending theme_links() a blank array is safe (have a look at http://api.drupal.org/api/function/theme_links -- if the array is empty, it returns a blank string). Is this satisfactory to get committed?
#7
Hmm, somehow the d6 patch was messed up.
#8
Yes, this looks good to me.
#9
It's been a few months, but the solution hasn't changed. Here's the patch for D6 again, against dev. Should pass the tests. I'll try to get a D7 version soon, but it's just one line.
#10
Here's the D7 patch.
#11
@moshe: Do you see a problem with this?
#12
fine with me
#13
Committed to D7 and D6.
@moshe: Ok, thanks. I took #12 as permission to commit. You asked me not to commit things outside of DNA, so I've waited for you to do it. Was there a misunderstanding?
@joelstein: Thanks for your patience. You uploaded the D7 patch (against revision 1.411) twice, but it applied to D6 with some fuzz.
#14
@salvis. thanks for committing ... you are welcome to commit bug fixes to any part of devel package. significant new features should go through issue queue review.
#15
@moshe: great, thanks!
#16
Automatically closed -- issue fixed for 2 weeks with no activity.