Add <p> tags around text in Who's Online block

Did you name your images backwards (before is actually after)? I like the before output better...

I think you have the before & after screenshots mixed up?

What about running this through a theme function?

Makes perfect sense to me. Reviewed & tested the patch and found no issues, and I personally thinks this makes for cleaner, better, markup...

@ultimateboy: since this method of wrapping messages in <p>-tags is used throughout core and since we're talking about a single paragraph of text, wouldn't it be overkill to run this through a theme function?

First, I agree that this text needs to be contained in a paragraph by default.

Though, I do have to agree with Ultimateboy in the need for a theme function. Yes, it seems silly to do such a thing for a piece of text. There is so much more to this that needs to be custom and reworkable. This block right now just kicks out a list of people's names in a list format. If it was a theme function, the list could be reworked to include pictures and/or other pieces of information from that person's profile.

Right now this is a very static and seemingly forgotten block in the system.

Here is my attempt at implementing what i am talking about. It is my first ever patch, so I kind of expect problems and am willing to attempt to remedy them.

Lets see what testbot says.

Already spotted an error. This re-roll should fix it.

Testbot...

As I said on IRC: I think it is overkill to be putting this through a template file. My reasoning for suggesting a theme function was to allow for easy alteration of the outputted text and, depending on needs, giving the ability to add a wrapper or two. In this case, you (Zarabadoo) have made that quite difficult by hiding the text in preprocess. While I understand why you have done this (in order to keep the template file clean and without the format_plural() logic) I think it is overkill. This is a perfect situation for a theme function, imo.

As a counter-point, I feel this is a more flexible solution. If someone just wants to provide wrappers to the elements and not alter the text, this is a much simpler option. In that case I would much prefer to just use a tpl, add my wrappers and get on with my day. The theme function would be a bit too beastly to copy over just to add those.

If someone really did want to alter this text, it is still available to be changed. All pieces of the logic are available in preprocess. This seems complex enough that a novice will have the same amount of trouble a theme function.

Checked out a clean head and applied the patch, and it still seems to work. Run it again jsut to see if it is the bot freaking out a bit.

Any chance we can just go with the patch in the original post (which IMO fixes a bug) and move the theme function to a separate issue (more a feature request if you ask me)? The original patch was a quick fix and RTBC, and unless we go with that I don't see this making it into D7...

I agree with #17.

It would be good to at least fix this in D7. We can discuss alternatives for D8. Here's a reroll of the original patch.

Let's get the patch from #19 committed...

+1 for patch in #19, this is a quickfix!

I think this was committed.