| Project: | Google Translate Links |
| Version: | 6.x-6.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
I love this module, but don't like the fact that all the configuration is done in the module directory. I therefore re-wrote the majority of the module to implement a settings page where users can specify a folder within the files directory where their flags are stored. This means that when this module is updated, users don't have to redo all their flags.
I have some ideas about giving all the settings a GUI interface, but I'll leave that for a separate issue where we can discuss it in detail (I'll post a link when I've created it).
For now, I've attached the .info and .module files of the code I was working on for your consideration. I didn't bother with a patch since pretty much everything has changed...
This may be better off as a separate version (6.x-3.x?) because of the major changes and because it doesn't support Drupal 5. I also got rid of the curl stuff since I didn't know what it was and since it seems to work fine without it...
What do you think?
| Attachment | Size |
|---|---|
| gtranslate_links.info | 111 bytes |
| gtranslate_links.module.txt | 3.58 KB |
Comments
#1
Ok, here's the discussion page for a new user interface: #592052: User interface design
#2
A more elaborate response will have to wait until I find some time, but there's two points I'd like to address:
You don't need to, just untar the tarball over your current module and you're good to go. Only when naming conventions change, something which warrants a major number increase (and does not seem likely atm) will you need to reconfigure.
It has not always been like this, however. In the first releases there was a sample config present, which, in retrospect, was a stupid idea exactly because of those upgrade issues.
The 'enabledlangs' directory is absent from most releases but the 2.x ones, in which it's empty (well, there's a '.keep' file as the drupal packaging script seems to purge empty directories).
As for curl: that was an order of magnitude faster when I tested it. (I might try to get some actual numbers on that). Most of the PHP installations I encounter have libcurl-support. Thankfully, yours didn't, otherwise it would have taken another while for the drupal_http_request() invalid argument bug to surface ;-)
Overall I like your code. On the other hand, I rather like the KISSyness of the current GUI-less codebase, but we'll have that discussion in a couple of days in the topic you created for that purpose.
#3
#4
I realise you can just copy the updated module over the old one, but what if there are files in an old version you remove in a newer one? Copying over the top will keep the old files which could be problematic ()...
It's generally good practice to keep all site-specific files in the files directory. Most people backup their files directory, I don't think there are many that backup their modules directory too. Even if for no other reason, it gives users a choice.
Glad you like it :)
#5
Theoretically possible, yes, though not very likely at the moment. OTOH the current situation might require users to adapt their standard upgrading procedure, as in your case. Conformity could add value.
Well, *I* keep my flag icons directory in the files directory thankyouverymuch ;-) I symlink to that dir from the module's dir. Problem solved ;-)
But then again, not everyone knows what a symlink is or how to use them to their advantage.
So good point about keeping site-specific files in the site-specific files directory. Right now, a multisite installation is not possible. Furthermore, content managers do not necessarily have file system access or the skills to properly use a filesystem. So I see the added value of a GUI if it means that people don't have to touch the file system anymore. I'll address that in the other topic.
#6
Updated the module to fix a possible issue between two different variables named the same (i.e. changed the second $url to $link) :)
#7
I was about to merge your code but I have some questions/remarks:
strpos(referer_uri(), 'translate.google.com')instead ofstrstr($_SERVER["HTTP_VIA"], 'translate.google.com')to determine whether the request came in via Google's proxy. Why? Is it more robust that way?Cheers!
#8
Hmmm, I've never used a private file system, so don't really know how it works or how to code for it. As for the referer_uri(), I had the thought I read something at the time that recommended using that function over the $_SERVER variable, but I can't now find any references to it... If fact, referer_uri() has been dropped from D7: http://drupal.org/update/modules/6/7#referer_uri