[dev] [commits] Horde branch master updated. 89bca7f749db1780dca80363d6f3d8c89f899df4
chuck at horde.org
Fri Mar 12 04:43:45 UTC 2010
Quoting Jan Schneider <jan at horde.org>:
> Zitat von Michael M Slusarz <slusarz at horde.org>:
>> Quoting Jan Schneider <jan at horde.org>:
>>> Zitat von Michael M Slusarz <slusarz at horde.org>:
>>>> commit 97fd203dcaaf5ddf7c9c2252eb44ac76e8d84846
>>>> Author: Michael M Slusarz <slusarz at curecanti.org>
>>>> Date: Sun Mar 7 20:15:31 2010 -0700
>>>> Add Horde_Themes:: library.
>>>> Move all themes related code to a single place (includes both image and
>>>> Horde::img(), through Horde_Themes::img(), will now attempt to auto
>>>> determine a graphic location. It looks in the following directories with
>>>> this priority:
>>>> 1. App theme
>>>> 2. Horde theme
>>>> 3. App default
>>>> 4. Horde default
>>>> This prevents us from having to copy all over all graphics for a theme -
>>>> now, only need the graphics that differ from defaults (caveat: if
>>>> loading graphics from CSS, copies of graphics need to continue to live
>>>> in theme directory).
>>> Please revert, at least the new autodetection code. Using
>>> themed_graphics and caching it in the session is absolutely
>>> necessary for perfomance reasons. Without that we are filestat'ing
>>> for directories and images in *every* img() call, which puts a
>>> huge burden on the filesystem IO, compared to the once-per-session
>>> stat'ing of themed_graphics.
>>> This was added after benchmarking massive performance degration
>>> when introducing themes with graphics.
>> I was not aware of the crippling IO issue. My next job was going
>> to be to save the image data via Horde_Cache (similar to how we
>> cache CSS scripts). As mentioned in the past, it has already
>> become almost mandatory to have a Horde_Cache driver installed for
>> various Horde functions for any sort of decent performance, so this
>> is not a dealbreaker.
>> But I am open to other ways of making this behavior better. But
>> not returning to the old behavior, which was defective in many ways:
>> 1) Absolute overkill when it comes to having to copy *every*
>> graphic to *every* theme that wants to add 1 graphic.
> I don't see this is much of an issue. Disk space is not an argument,
> this is a one-time action when adding a new icon, and it only
> requires some discipline for the developers/designers. I never felt
> like this was really annoying.
>> 2) #2 makes it impossible to reliably determine which graphics have
>> been overriden in a theme.
> That's true, but I fail to see why this is a problem. For a theme
> designer it's not important to know which icons are really overriden
> or just copies from the original icon theme. They only have to be
> happy with their own icon theme. And actually it might be easier for
> them to verify that everything is in place if they have the
> *complete* icon set in one directory.
>> 3) #2 makes it virtually impossible to keep themes up-to-date.
> See 1).
>> 4) It was impossible to override a graphic via an app theme if the
>> Horde::img() call specifically referenced a horde graphic (which
>> sort of defeats the whole point of themes).
> Yes, true, but if you don't want to use the Horde icon, why would
> you point to it? Maybe I'm missing something, if I understood you
> correctly, you are talking about the case where an application (like
> IMP) is using a Horde icon, but an application view (like DIMP)
> should be using a different one? Beside that this seems a bit
> obscure, you can still point to the sub-application's icon from the
> application view template.
>> 5) There is no concept of default graphics imported from horde.
>> This is counter-intuitive as compared with the way CSS files work.
>> However, I'll admit that there are powerful incentives to require
>> *all* graphics used by an app to have a physical copy of the app
>> (i.e. an app never loads graphics from horde). First, this makes
>> the applications more self-contained. More importantly, there is
>> no practical way to load graphics from CSS files outside of making
>> it an absolute requirement that apps MUST live under Horde.
> That's an interesting approach. It would produce even more duplicate
> icons, but makes it much clearer where the icons are coming from,
> i.e. where to look for them. We could still keep often used icons in
> Horde, so that application developers have a repository to pick (and
> copy) from. Or we could outsource them to a separate Git module.
> Especially the increasing use of graphics in CSS would benefit from that.
>> Brainstorming, I have the following ideas:
>> 1) Caching as described above.
>> Pros: automatically determined (themes are drop-in replaceable).
>> Cons: adds a bit of overhead the first time a graphic is loaded;
>> fallback to file stat() calls on Horde_Cache. As mentioned
>> previously, the latter issue is not critical IMHO.
> Unless I'm missing something, you would either have to manually
> invalidate the cache if any icon is changed, or we still have to
> stat each icon to see if has changed, which wouldn't give us anything.
>> 2) Add theme definition script. The idea: after changing theme
>> graphics, this script is run and it produces a PHP data file that
>> contains the graphics list for the theme (i.e. which location each
>> graphic of the theme is located.)
>> Pro: Arguably identical overhead than the old method - instead of
>> doing a file_exists() call, we simply include 1 or 2 PHP files per
>> page load.
>> Cons: This script would have to be run manually whenever a graphic
>> is changed. However, themes/graphics theoretically won't be changed
>> all that often, and this script could be called as part of the
>> build process. Additionally, this is no more burdensome than the
>> previous method (requiring calling a script to determine which
>> graphics are missing from a particular theme).
> I can't help it, but this seems more work and overhead than what you
> called overkill in your very first point.
> I still think that the original approach with themed_graphics was
> pretty smart and solid. If inheritation and loading of Horde icons
> from CSS files is an issue, we could require each application to
> keep a copy of *any* icon it needs. In the case of sub-application I
> still might be missing the point, maybe you could make this clearer
> with an example.
I agree with Jan's summary. I don't think duplicating images is a big
deal. And for theme designers, if we want a tool to say which images
have been overridden, we can write something that uses md5sums to
check. Lots of overhead, but not a big deal in a theme-designing
I'm also fine with requiring applications to contain all of their own
graphics. Does seem like it would simplify a lot of things.
More information about the dev