[drupal-devel] [feature] URL rewrite hook

[Jose A Reyero]

Issue status update for 
http://drupal.org/node/29030
Post a follow up: 
http://drupal.org/project/comments/add/29030

 Project:      Drupal
 Version:      cvs
 Component:    base system
 Category:     feature requests
 Priority:     normal
 Assigned to:  chx
 Reported by:  chx
 Updated by:   Jose A Reyero
 Status:       patch (code needs review)

Goba, yes, my idea was to provide a hook, so any module can mangle the
path/query string. What will happen with that incoming paths is a
different story, I mean any module can access the query string any time
later... And I'd like this to be separated from path aliasing, which can
be done in a different step...


Ideally, of course, this would be done for outgoing and incoming urls,
but there's a number of issues, like incoming path processing being
done before module loading, in common.inc... Another issue, IMHO, is
that current url handling is a bit messy and could be better
streamlined. 


But the first patch was simple enough and quite straight forward, at
least it works for *all* outgoing URLs, and allows to rewrite query
strings, while the second one, I see much more limited use for it....




Jose A Reyero



Previous comments:
------------------------------------------------------------------------

Wed, 17 Aug 2005 11:39:12 +0000 : chx

Attachment: http://drupal.org/files/issues/url_rewrite.patch (1.35 KB)

I need a URL rewrite hook sometimes, here is an implementation, with
ample comments. I looked into arg() to see whether it needs a reset
parameter, and it does not, but it contained a minor bug, which is also
fixed.




------------------------------------------------------------------------

Wed, 17 Aug 2005 11:46:45 +0000 : chx

Attachment: http://drupal.org/files/issues/url_rewrite_0.patch (1.73 KB)

Performance boost.




------------------------------------------------------------------------

Wed, 17 Aug 2005 11:48:51 +0000 : Jose A Reyero

+1


This is really needed by i18n module, and maybe other modules could use
it to add some extra information in the query string.




------------------------------------------------------------------------

Wed, 17 Aug 2005 11:50:16 +0000 : killes at www.drop.org

this seems just an evil plot to cater for the evil hack that i18n.module
is. --




------------------------------------------------------------------------

Wed, 17 Aug 2005 11:57:23 +0000 : chx

killes, while I know you do not like the current implementation of i18n,
that module exists and even works. If you do not like the current
approach, you can always write a better one. And also, please note we
try to introduce non-i18n specific solutions this time.




------------------------------------------------------------------------

Wed, 17 Aug 2005 12:05:10 +0000 : killes at www.drop.org

"it works" has never been a consideration in Drupal development, don't
let us start to use it. I don't need an i18n module. Had I had the urge
to write one, it would have been based on walkah's excellent start to be
seen here:


http://cvs.drupal.org/viewcvs/drupal/contributions/sandbox/walkah/translate_node/


I don't see anything non-i18n specific here. let us not pollute low
level functions such as url() with custom hacks.




------------------------------------------------------------------------

Wed, 17 Aug 2005 12:26:11 +0000 : Goba

Why extend url() and why not the alias retrieval functions? BTW at that
time it was decided that a single function should be used
(conf_url_rewrite()) and not a hook, because of performance reasons. If
you provide the functionality this was, then conf_url_rewrite() gets
confusing, and even meaningless if you do it in the alias retrieval
functions themselfs.




------------------------------------------------------------------------

Wed, 17 Aug 2005 12:49:30 +0000 : chx

Attachment: http://drupal.org/files/issues/url_rewrite_1.patch (1.95 KB)

Goba is so right, that I was already working on it.




------------------------------------------------------------------------

Thu, 18 Aug 2005 11:11:39 +0000 : Jose A Reyero

This second version is more limited, and wont be that useful, because in
case an alias exists, the rewritting is skipped.


IMHO, the aim for this patch should be allowing modules to add
information in the path or in the query string, for *all* the outgoing
urls. I was thinking of i18n module and language information, but this
can be useful for other modules too. 


So please, let's stick to the previous version of the patch




------------------------------------------------------------------------

Thu, 18 Aug 2005 11:45:23 +0000 : Goba

Jose, so you advocate only mangling URLs on output? Really?