Module order for hooks (event, theme, etc.)

--===============7105925538057290838==
Content-Type: multipart/alternative; boundary=001a11c2e4deabe54904e00adcac

--001a11c2e4deabe54904e00adcac
Content-Type: text/plain; charset=UTF-8

Hey everyone,

Following on recent discussions of routing, init files, hooks, etc., I
jumped into unifying Gallery's hook-calling system.  It turns out that the
module order is a bit deeper of a rabbit hole than I originally thought...

So, the idea of the cascading file system is to have multiple modules,
ordered by their "priority."  Let's say we have modules A, B, and C, where
A is highest and C is lowest priority.  Then, we'd setup our Kohana paths
in the following order:
- APPPATH
- active theme
- purifier
- module A
- module B
- module C
- gallery
- (formo, database, orm, etc.)
- SYSPATH

Then, Kohana uses them like this:
- autoloading classes: uses *forward* order, and stops when it finds
something.  So, any high-priority module can override something lower on
the list.
- config files: uses *reverse* order, and goes through whole list.  So, the
high-priority modules get the last word - whatever variable the
low-priority ones set, the high-priority ones can change.

This brings us to Gallery's hook system.  In Gallery 3.0.x, we used the
following order:
- gallery
- purifier
- module A
- module B
- module C
- active theme

IMHO, this seems a bit ad hoc and not at all consistent - it shuffles the
stack priorities given elsewhere, simply reversing gallery and the active
theme and leaving the rest alone.  It should be either the same as the path
search order, or the exact opposite...

... but which one?  Some examples from the core code:
- menu: *reverse* order.  Gallery's menu system (e.g.
GalleryEvent::site_menu()) relies on the gallery module going first.  It
builds the basic menu, which other modules can modify.
- theme: *reverse* order.  Gallery's theme system builds HTML blocks in the
module order.  Here it also makes sense for the higher-priority modules to
go last, as they can add JS that modifies things above it.
- routes: *forward* order.  Kohana's routing/request system follows the
init file order, which prioritizes the routes defined first, not last.
 Since we're doing away with init files, this doesn't help us.

So, it looks like we have a couple options:
- use *reverse* order by default, but give the option of running oppositely
if asked.  This is the most flexible approach, but I'm unsure if we really
need/want this flexibility.  Also, it implies that we'd run all of the
gallery_ready events opposite of all other events, which is weird.
- use *reverse* order always, and tell Kohana to look for routes in the
opposite order.  This is actually pretty easy to do:

class Gallery_Request extends Kohana_Request {
  public static function process(Request $request, $routes=null) {
    // Copy first line of Request::process(), but add array_reverse().
    $routes = (empty($routes)) ? array_reverse(Route::all()) :
array_reverse($routes);
    return parent::process($request, $routes);
  }
}

My vote is for the second option - it keeps us 100% consistent with the
idea that high-priority things get the last word, and cleanly fixes the
*one* case where somebody feels otherwise.  That said, I could understand
if people prefer the opposite approach...

One last note: this does have some minor implications for contrib modules.
 In particular, things that required imposing a module order to ensure
events happen in sequence (rawphoto and autorotate before exif, tags before
tag_albums) will be temporarily broken until their order is reversed.  This
is easily fixed with another stanza in GalleryInstaller::upgrade(),
ensuring nothing gets broken.

Thoughts?

Take care,
Shad

--001a11c2e4deabe54904e00adcac
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hey everyone,<div><br></div><div>Following on recent discussions of routing=
, init files, hooks, etc., I jumped into unifying Gallery&#39;s hook-callin=
g system. =C2=A0It turns out that the module order is a bit deeper of a rab=
bit hole than I originally thought...</div>

<div><br></div><div>So, the idea of the cascading file system is to have mu=
ltiple modules, ordered by their &quot;priority.&quot; =C2=A0Let&#39;s say =
we have modules A, B, and C, where A is highest and C is lowest priority. =
=C2=A0Then, we&#39;d setup our Kohana paths in the following order:</div>

<div>- APPPATH</div><div>- active theme</div><div>- purifier</div><div>- mo=
dule A</div><div>- module B</div><div>- module C</div><div>- gallery</div><=
div>- (formo, database, orm, etc.)</div><div>- SYSPATH</div><div><br></div>

<div>Then, Kohana uses them like this:</div><div>- autoloading classes: use=
s *forward* order, and stops when it finds something. =C2=A0So, any high-pr=
iority module can override something lower on the list.</div><div>- config =
files: uses *reverse* order, and goes through whole list. =C2=A0So, the hig=
h-priority modules get the last word - whatever variable the low-priority o=
nes set, the high-priority ones can change.</div>

<div><br></div><div>This brings us to Gallery&#39;s hook system. =C2=A0In G=
allery 3.0.x, we used the following order:</div><div><div>- gallery</div><d=
iv>- purifier</div><div>- module A</div><div>- module B</div><div>- module =
C</div>

<div><div>- active theme</div></div></div><div><br></div><div>IMHO, this se=
ems a bit ad hoc and not at all consistent - it shuffles the stack prioriti=
es given elsewhere, simply reversing gallery and the active theme and leavi=
ng the rest alone. =C2=A0It should be either the same as the path search or=
der, or the exact opposite...</div>

<div><br></div><div>... but which one? =C2=A0Some examples from the core co=
de:</div><div>- menu: *reverse* order. =C2=A0Gallery&#39;s menu system (e.g=
. GalleryEvent::site_menu()) relies on the gallery module going first. =C2=
=A0It builds the basic menu, which other modules can modify.</div>

<div>- theme: *reverse* order. =C2=A0Gallery&#39;s theme system builds HTML=
 blocks in the module order. =C2=A0Here it also makes sense for the higher-=
priority modules to go last, as they can add JS that modifies things above =
it.</div>

<div>- routes: *forward* order. =C2=A0Kohana&#39;s routing/request system f=
ollows the init file order, which prioritizes the routes defined first, not=
 last. =C2=A0Since we&#39;re doing away with init files, this doesn&#39;t h=
elp us.</div>

<div><br></div><div>So, it looks like we have a couple options:</div><div>-=
 use *reverse* order by default, but give the option of running oppositely =
if asked. =C2=A0This is the most flexible approach, but I&#39;m unsure if w=
e really need/want this flexibility. =C2=A0Also, it implies that we&#39;d r=
un all of the gallery_ready events opposite of all other events, which is w=
eird.</div>

<div>- use *reverse* order always, and tell Kohana to look for routes in th=
e opposite order. =C2=A0This is actually pretty easy to do:</div><div><br><=
/div><div><font face=3D"courier new, monospace">class Gallery_Request exten=
ds Kohana_Request {</font></div>

<div><font face=3D"courier new, monospace">=C2=A0 public static function pr=
ocess(Request $request, $routes=3Dnull) {</font></div><div><font face=3D"co=
urier new, monospace">=C2=A0 =C2=A0 // Copy first line of Request::process(=
), but add array_reverse().</font></div>

<div><font face=3D"courier new, monospace">=C2=A0 =C2=A0 $routes =3D (empty=
($routes)) ? array_reverse(Route::all()) : array_reverse($routes);</font></=
div><div><font face=3D"courier new, monospace">=C2=A0 =C2=A0 return parent:=
:process($request, $routes);</font></div>

<div><font face=3D"courier new, monospace">=C2=A0 }</font></div><div><font =
face=3D"courier new, monospace">}</font></div><div><font face=3D"courier ne=
w, monospace"><br></font></div><div>My vote is for the second option - it k=
eeps us 100% consistent with the idea that high-priority things get the las=
t word, and cleanly fixes the *one* case where somebody feels otherwise. =
=C2=A0That said, I could understand if people prefer the opposite approach.=
..</div>

<div><br></div><div>One last note: this does have some minor implications f=
or contrib modules. =C2=A0In particular, things that required imposing a mo=
dule order to ensure events happen in sequence (rawphoto and autorotate bef=
ore exif, tags before tag_albums) will be temporarily broken until their or=
der is reversed. =C2=A0This is easily fixed with another stanza in GalleryI=
nstaller::upgrade(), ensuring nothing gets broken.</div>

<div><br></div><div>Thoughts?</div><div><br></div><div>Take care,</div><div=
>Shad</div>

--001a11c2e4deabe54904e00adcac--


--===============7105925538057290838==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
--===============7105925538057290838==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

__[ g a l l e r y - d e v e l ]_________________________

[ list info/archive --> http://gallery.sf.net/lists.php ]
[ gallery info/FAQ/download --> http://gallery.sf.net ]
--===============7105925538057290838==--