[drupal-devel] [feature] Dependency system to solve forum
without comment problem
mystavash at animecards.org
mystavash at animecards.org
Sat Mar 26 15:33:37 UTC 2005
Someone called? ;)
Let me get this straight... This all started because the forums muck up without
the comments module enabled, so ya'll thought of an idea to deal with modules
that depend on each other in one fell swoop. I'm sure you already know what
you'll do if that module isn't preinstalled, or if it is, but the additional
tables haven't been added. I have faith. ;)
? Doesn't this message read wrong? Aren't you enabling a module automatically
because the module the user tried to install required it? Let's say Y requires
X, and you try to install Y. After installing/or during the installation of Y,
you get this message 'X module has been enabled for you because Y needs it to
work properly. There may be new permissions and settings available for X module.'
I think that is about as far as you can take that... As I understand it, it's
one of those notifications you see on the top of a page, right? You do want to
consider that this probably comes as a suprise to the user. You've done
something they didn't ask you to do. :)
This is a service, not a HAL takeover, so 'enabled for you' makes it sound
nicer. :)
It'd be even nicer if permissions and settings could be links, but I am not sure
where in the process the user will be at the time this notification is
displayed, and when you make more than one link, they might hit the back button
and muck up something. :)
I hope that helps. Not sure it does. I don't really have a visual fix on the
workflow of this system, so it might be a bit muddled in places. ^.^;;
Anisa.
Quoting stefan nagtegaal <drupal-devel at drupal.org>:
> Issue status update for http://drupal.org/node/18447
>
> Project: Drupal
> Version: cvs
> Component: system.module
> Category: feature requests
> Priority: normal
> Assigned to: chx
> Reported by: chx
> Updated by: stefan nagtegaal
> Status: patch
>
> Can't we refrase this:
>
> <?php
> t("The following module has been enabled since it's dependent on
> %module in order function properly: %dependents.", array ('%module' =>
> '<em>'. $module .'</em>', '%dependents' => '<em>'. implode(', ',
> $enabled) .'</em>')
> ?>
>
>
> I'm not sure why, but I don't think this is userfriendly enough..
> Anisa, (I'm not sur eshe is on this list) do you have a better idea?
>
>
> stefan nagtegaal
>
>
>
> Previous comments:
> ------------------------------------------------------------------------
>
> March 6, 2005 - 01:55 : chx
>
> Attachment: http://drupal.org/files/issues/depend.patch (3.22 KB)
>
> As discussed on the developer list. This was surprisingly easy. There is
> still an issue: t('The configuration options have been saved.') is
> displayed more than once if there is a dependency forced module switch
> on. I have tried something (you'll see) for some reason it does not
> work.
> Number of page reloads can be decreased if Drupal could be
> reinitialized after system_listing_save instead of redirecting but I
> doubt this would be feasible.
> Although I marked this is feature request, this is more a usability
> issue (for the forum-comment dependency) and as a side effect it is a
> useful feature, too (for any other module "bundles").
> I want criticism! I'd like to get torn apart by wild animals. Heavy!
> Eaten by some squirrels.
>
>
> ------------------------------------------------------------------------
>
> March 6, 2005 - 02:22 : clydefrog
>
> Forgive me if I'm mistaken, but shouldn't forum_depends return an array?
>
>
> ------------------------------------------------------------------------
>
> March 6, 2005 - 02:26 : javanaut
>
> I think allowing for both required dependencies as well as optional ones
> would make it practical for a package management tool to be created. I
> like it, though, and I think being able to programatically enforce (or
> at least notify users about) dependencies would assist users who refuse
> to RTFM.
> >From my drupal-devel comment, here's how I would suggest formatting the
> hook_depends:
>
> <?php
> function filestore2_depends() {
> return array('required'=>array('fscache'),
> 'optional'=>array('taxonomy','comments'));
> }
> ?>
>
>
>
>
> ------------------------------------------------------------------------
>
> March 6, 2005 - 02:29 : chx
>
> If you take a look at module_invoke_all, you will see that a string is
> fine, 'cos if you return a string it does $result[]
> However, an associative array is not good with module_invoke_all 'cos
> for returned arrays there is an array_merge where "If the input arrays
> have the same string keys, then the later value for that key will
> overwrite the previous one."
>
>
> ------------------------------------------------------------------------
>
> March 6, 2005 - 02:33 : javanaut
>
> Ok, how about it taking a parameter?
>
> <?php
> function filestore2_depends($dep_type='required') {
> $deps = array('required'=>array('fscache'),
> 'optional'=>array('taxonomy','comments'));
> return $deps[$dep_type];
> }
> ?>
>
>
> Then, for required dependencies, pass a 'required' argument, and
> optional ones could take an 'optional' argument.
>
>
> ------------------------------------------------------------------------
>
> March 6, 2005 - 03:00 : chx
>
> Attachment: http://drupal.org/files/issues/depend_0.patch (3.6 KB)
>
> I would leave associative arrays for the future. As we do not have a
> package tool, there is little use of 'optional' -- I'd like to get this
> into 4.6
> After testing the patch, I have changed something, which I should not
> have done, so another version is attached.
>
>
> ------------------------------------------------------------------------
>
> March 6, 2005 - 03:11 : chx
>
> Attachment: http://drupal.org/files/issues/depend_1.patch (4.37 KB)
>
> This one should really work...
>
>
> ------------------------------------------------------------------------
>
> March 6, 2005 - 03:12 : chx
>
> Attachment: http://drupal.org/files/issues/depend_2.patch (3.15 KB)
>
>
>
>
> ------------------------------------------------------------------------
>
> March 6, 2005 - 05:13 : gordon
>
> +1
> Not only is this something that I have wanted for filestore2 for quite
> a while, but has not been on my priority list. But this would be
> extremely useful for the ecommerce modules, as there seems to be alot
> of interdependancy.
> Also on a side note, it would also be good for a module to be able to
> reject an enabling. So if the module is going to stop drupal from
> running if it does have its tables created, it will inform the user and
> not be turned on. This could actually be done allowing the dependancy
> hook to do some checking and so stop the module from being able to be
> enabled.
>
>
> ------------------------------------------------------------------------
>
> March 6, 2005 - 09:19 : mathias
>
> This would make installing the ecommerce package easier given that
> modules are no longer listed alphabetically from within their
> subdirectories. The modules are scattered all over the page unless I
> adopt some sort of ec_ prefix to module naming.
> I wonder how Adrian's installer handles dependencies?
>
>
> ------------------------------------------------------------------------
>
> March 6, 2005 - 11:42 : adrian
>
> My installer doesn't currently handle dependencies, and I am not
> convinced this should go into 4.6.
> There's more to dependency checking than just 'depends' including fun
> things like circular dependencies and version checking.
> Once we have versioning for drupal modules / schemas .. we can look at
> this again.
> Vlado apparently has dependency checking code already built, that he is
> going to donate to the installer effort.
>
>
> ------------------------------------------------------------------------
>
> March 6, 2005 - 11:57 : chx
>
> Attachment: http://drupal.org/files/issues/depend_3.patch (3.26 KB)
>
> Well, I know there could be problems. However, circular dependecy is
> handled with my code. You have module A, this depends on B, this
> depends on C which depends on A. You tick A, press Save. Next listing
> will tick B instead of you, press Save. Next listing will tick C
> instead of you, press Save. Next listing will be happy, 'cos A is on.
> Adrian convinced me that taking a parameter is a good thing, so it's
> in. And he asked for calling the hook _info. So be it. And he pointed
> out that I do not understand drupal_set_message which was true. So now
> the status message is now displayed only once. Shall I protect the new
> error message from such a duplication?
> And please, get this simple system into 4.6 'cos (as already stated)
> the forum-comment dependency has threads on drupal.org already so it
> could be called a bug.
>
>
> ------------------------------------------------------------------------
>
> March 6, 2005 - 12:08 : chx
>
> Attachment: http://drupal.org/files/issues/depend_4.patch (3.27 KB)
>
> Minor things: default hook_info key shall be called 'depends' not
> 'required' (Adrian). Message should be 'enabled' for consistency
> reasons.
>
>
> ------------------------------------------------------------------------
>
> March 8, 2005 - 20:56 : Dries
>
> The status message needs work (incorrect use of em, confusing wording).
> Lastly, the way messages are handled and fiddled with is peculiar at
> best: the logic is spread out over different functions and complicated
> because of that. I'm not happy with the code as is.
>
>
> ------------------------------------------------------------------------
>
> March 12, 2005 - 11:37 : Dries
>
> Setting this 'active'. Waiting for new patch.
>
>
> ------------------------------------------------------------------------
>
> March 13, 2005 - 21:21 : chx
>
> Attachment: http://drupal.org/files/issues/depend_5.patch (3.3 KB)
>
> Here is new version. Hope you like it more than the previous one.
>
>
> ------------------------------------------------------------------------
>
> March 13, 2005 - 21:29 : chx
>
> One s too much...
>
>
> ------------------------------------------------------------------------
>
> March 13, 2005 - 21:31 : chx
>
> Attachment: http://drupal.org/files/issues/depend_6.patch (3.3 KB)
>
> grrrr... it submitted without attaching... is there some way to make
> preview required for me...?
>
>
> ------------------------------------------------------------------------
>
> March 14, 2005 - 11:28 : stefan nagtegaal
>
> chx: I do like the approach you took, but I think we should return more
> information to the administrator.
> You did:
>
> <?php
> drupal_set_message(t('%module is enabled because other modules depend
> on it.', array ('%module' =>'<em>'. $file->name .'</em>')), 'error');
> ?>
>
>
> First I don't think you need to have the $type = 'error' as the second
> argument.. Imo it is not an error, just a notification. Or am I wrong?
> And secondly, can't we do something like:
>
> <?php
> drupal_set_message(t("Because '%module' depends on functionality
> offered by '%modules', these modules are also enabled to omit possible
> problems.', array ('%module' => "<em>$file->name</em>", '%modules' =>
> "<em>$required_modules</em>")));
> ?>
>
>
> I would really appreciate feedback of any of you...
>
>
> ------------------------------------------------------------------------
>
> March 26, 2005 - 00:17 : chx
>
> Attachment: http://drupal.org/files/issues/dependency1.patch (2.34 KB)
>
> Whole new dependency system! This has nothing to do the with previous
> patches. Those were a bit hackish.
> hook_install('depends') may return a string or an array of strings. The
> modules whose name is returned, will be switched on. These new modules'
> dependencies are also dealt with. And so on.
> I have thought of using format_plural, but the arguments make that
> impossible.
>
>
> ------------------------------------------------------------------------
>
> March 26, 2005 - 02:46 : chx
>
> Attachment: http://drupal.org/files/issues/dependency1_0.patch (2.34 KB)
>
> A few 's are gone according to Steven's advice.
>
>
> ------------------------------------------------------------------------
>
> March 26, 2005 - 04:24 : mathias
>
> Attachment: http://drupal.org/files/issues/dependency.patch (2.62 KB)
>
> +1, and a slice of blueberry pie.
> I tested this patch against the ecommerce package and it worked as
> expected. My attached patch simply tweaks a couple minor code details
> and made the the dependency message a little easier to understand.
> Please add this to 4.6. This could be viewed as a major usability
> benefit for module developers and end users ;-)
>
>
>
More information about the drupal-devel
mailing list