[dev] Re: [cvs] commit: kronolith month.php [patch attached]

[Derek P. Moore]

> There's an unrelated change in there (is_null) - any reason for that?

I thought it enhanced readable and could be considered a worthwhile code
clean-up.  The various is*() tests are used throughout Horde, and they're PHP
language constructs, and they exist for readability purposes, so why not use
'em?  :)

> I think I would rather introduce another parameter; yes, it's awkward,
> but so is passing in an array or a string, and remember what order to put
> the darn array in.

Well, remembering the order of arguments is a pain in my arse either way.
Either the two conceptually related arguments are bundled together in the
second argument, and I have to struggle to remember in which order.  Or, the
two arguments are the second and fourth arguments, and I have to struggle to
remember in which order.

In either case, luckily it's all documented.

With Horde::img() in particular, it's nice to have short and sweet function
calls.  In other Horde functions, it's necessary to pad the call with nulls to
get to the argument you're interested in.  I'd like to see Horde::img() 
stay as
dense and short as possible.

> How about this: specify title="" in the $attr string. Have Horde::img()
> check to see if the string title=" appears in $attr. If it doesn't, insert
> title="$alt", otherwise, don't. That way we don't even need a new parameter
> and you can achieve the same thing.

That was exactly my original idea.  When I went to actually implement 
it, I came
up with a couple of good reasons to find another solution.  I can't remember
those reasons now.

[pauses to read the code and remember why I didn't do it that way]

Oh!  I remember the reasons now...  There are two reasons, really, but 
only one
really seals the fate of using the $attr argument.

1)  It increases code complexity.  Not too much, but just enough to make the
lazy programmer look for the next excuse so as not to implement all that
complexity...

2)  It's nearly impossible to internationalize the title attribute if 
you go the
way of $attr.

Well, I suppose it's not 100% impossible, but I would have to call
   Horde::img('new.png', '+', 'title="'._("Create a New Event").'"')
"nearly impossible".

You could also move internationalization of the title attribute into the
Horde::img() function itself.  But that would be so obscure for translators as
to be deemed "nearly impossible" as well.  (Besides, would something like
_($title) even be legal?)