16:15 < killes> chx: please remove legacy.inc from the default includes.
16:15 < chx> killes: thanks for volunteering to roll a patch
16:15 < killes> chx: no problem, if it is for a good cause. :p
16:16 < killes> chx: would you want me to include it if legacy.module is enabled?
16:16 < chx> killes: the two are unrelated
16:16 < chx> killes: if a module needs it , then it should include it
16:16 < chx> killes: include_once , that is
16:17 < chx> killes: in an unusual gesture we are providing backwards compatibility
16:17 < killes> chx: why not remove it completely? Which module woudl need it?
16:17 < chx> killes: one that does hook into drupal but uses independent forms ...
16:18 < chx> killes: see, for example, flexinode 'add content type' screen
16:18 < chx> killes: that could work
16:18 < chx> killes: but as flexi would need an upgrade there is no excuse for not upgrading that screen, too
16:19 < chx> killes: now, if there would be module which does not use hook_form , hook_user 'form' , hook_nodeapi 'form and hook_settings THEN it can run off legacy
16:19 < killes> chx: so basically it is useless. :p
16:20 < chx> killes: basically, yes.
16:20 < chx> killes: if a module exists besides Drupal and not in, then...

So can we do a cvs rm -f includes/legacy.inc and aplly the atatched patch, pretty please?

CommentFileSizeAuthor
#5 legacy_must_die.patch846 byteskilles@www.drop.org
#1 remove_legacy.patch657 byteschx

Comments

chx’s picture

chx commented
Assigned: Unassigned » chx
Priority: Normal » Critical
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new remove_legacy.patch657 bytes

It was 'broken attachments' day and Gerhard forgot to upload the patch ever since. It's now critical because the new error handling broke legacy.inc . There are other ways to fix but I would rather not bother.

killes@www.drop.org’s picture

killes@www.drop.org commented
Category: task » bug

Now it i a bug. :)

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks.

dries’s picture

dries commented
Status: Fixed » Needs review

This appears to break HEAD.

Fatal error: Call to undefined function: fix_checkboxes() in /Users/dries/Sites/drupal-cvs/includes/common.inc on line 1369

killes@www.drop.org’s picture

killes@www.drop.org commented
StatusFileSize
new legacy_must_die.patch846 bytes

oops, sorry.

chx’s picture

chx commented
Status: Needs review » Postponed

In 4.8. We do bridge stuff for 4.7.

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Postponed » Reviewed & tested by the community

I disagree. Maintaning backwards compatibility is just a bad idea and an alien concept for Drupal.

chx’s picture

chx commented
Status: Reviewed & tested by the community » Active

We are changing the old form_ functions so they become arrays. Stay tuned. There is a need for upgrade but not the whole upgrade. Best of both worlds.

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Active » Reviewed & tested by the community

Whatever is going to be in legacy.inc, it should not be included by default. This is what this patch does.

Steven’s picture

Steven commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

guckie’s picture

guckie commented
Title: Remove legacy.inc » A little guidance, please?...

Hi,

I understand the reason behind removing legacy.inc.. I just need a little enlightenment on how to fix a symptom of its removal..

as of cvs-Oct28, I am getting a "call to underfined function form()" error from a (contributed) module which was working fine the day before, so I assume legacy.inc was previously providing support for a now-deprecated function

for some reason, I can't seem to find documentation on what function should replace form(). If someone could point me to that, I'd be happy to revise (& test) the contrib'd module & report to the module maintainer on the change... I'm not a programmer... am learning my way into this now, but not yet capable of creating a patch, otherwise I would.

Also... a question about legacy.module... does it only provide legacy handlers for core drupal modules or should it not also support legacy calls from contributed modules as well? (Just noting that the above-described symptom exists even though legacy.module is enabled).

Thanks in advance :-)

asimmonds’s picture

asimmonds commented
Title: A little guidance, please?... » Remove legacy.inc

Please do not change the title, it changes it for the thread

guckie’s picture

guckie commented

Doh! I'm sorry *blush*

Thanks for catching the title & correcting me.

Requested guidance will still be appreciated. :-)

sangamreddi’s picture

sangamreddi commented

Hi,

A patch can be found here for form error function. Comment 11
http://drupal.org/node/33992

After applying the path the form error message gone.

guckie’s picture

guckie commented

Thank you for your time to reply, sangamreddi. :-)

The information you gave is for a similar issue, but not quite what I'm experiencing. I do not have any issues with _form_get_error(), which is what that patch is intended for.

I am receiving 'undefined function' messages for form() in one contrib'd module and for form_select() in another contrib'd module.

For the record, I can still use help. Sorry.

Also, if someone can answer this question, it would help me understand the purpose of legacy.module better.

.. a question about legacy.module... does it only provide legacy handlers for core drupal modules or should it not also support legacy calls from contributed modules as well?

killes@www.drop.org’s picture

killes@www.drop.org commented

guckie: You need to convert the module to use the new forms API. As an alternative you can try to include legacy.inc from the module.

guckie’s picture

guckie commented

Ok, thank you, killes!

Is this relevant to the new forms API? (http://drupaldocs.org/api/head/group/form)

If not, could you point me to current documentation on it?

I guess part of my confusion is that at the above link, it says the location is legacy.inc for the two functions I'm getting undefined messages for. Are the functions form() and form_select() defined at all in the new forms API? If so, where (in which core file)? I think I am having trouble locating up-to-date documentation for it.

Sorry to be such a bother.
Once pointed in the right direction, I'll try to make it up by submitting updates for as many contrib'd modules as I can. :-)

killes@www.drop.org’s picture

killes@www.drop.org commented

Yes, the functions you seek are defined in legacy.inc, but legacy.inc is not included by default as it does not work in all cases. More info can be found here:

http://cvs.drupal.org/viewcvs/*checkout*/drupal/contributions/sandbox/we...

and there:

http://drupal.org/node/33338

guckie’s picture

guckie commented

Terrific! Thank you very much :-)

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Priority: Critical » Normal
Status: Closed (fixed) » Active

As far as I know, it's been decided that we're not working towards backwards compatibility for forms in Drupal 4.7? If so, then there really is no point in keeping legacy.inc is there? Or am I missing something?

chx’s picture

chx commented

I agree. Let's drop legacy.inc. Dries, please.

chx’s picture

chx commented
Status: Active » Reviewed & tested by the community
chx’s picture

chx commented

while there is no patch, how else could I signal for a file removal?

dries’s picture

dries commented

How about Adrian's magical code that provides backward compatibility?

chx’s picture

chx commented

That's stopped because slow/error prone. jjeff formupdater module obsoleted it anyways.

dries’s picture

dries commented

Where can I investigate jeff's form updater?

What are the implications of removing legacy.inc?

killes@www.drop.org’s picture

killes@www.drop.org commented

contributions/modules/formupdater/

There are no implications, as it isn't included by default from any place in core.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Removed.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)