Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.If at all possible, this module should attempt to use the same form element names and structure that node.module provides. That way, modules like Vertical Tabs and others that work on those options don't need to also write code to deal with the override node options module as well.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 644820-ono-D6.patch | 24.58 KB | dave reid |
Comments
Comment #1
dave reidSee #460700: Tab descriptions are broken when using "override node options" module for the original issue.
Comment #2
dave reidI want to write a patch, but I'm not sure which branch I should checkout for Drupal 6. Plus there is a *lot* of messy code. :/
Comment #3
joachim commentedThe branch hell has been fixed by your good self :)
And as for the messy code (agreed, there's some terrible stuff in there), I've made quite a few commits today towards that. More remains to be done though: see #365570: code style cleanup.
Oh and is this relevant to this issue? #476510: Using just hook_form_alter, getting rid of hook_nodeapi.
Comment #4
joachim commentedJust a note to say that I've added tests for this module.
These use the current special form element names, so changes made for this issue will need to alter the tests too. On the plus side though, they should make it much easier to check this sort of deep alteration to the module doesn't break anything!
Comment #5
dave reidPlease also help review this D7 issue to make this easier: #668806: Include revision form element with access control
Comment #6
dave reidWorking patch will all tests passing. I appreciate all the work that Joachim put into writing tests, they needed to be simplified, a lot. It's much easier to understand what is going on and work on the tests when we break them up into re-usable chunks, but not-too-general chunks that things don't make sense. Please test and review.
Comment #7
joachim commentedYou seem to be completely replacing the test file I made.
I understand that it may be harder to grasp initially -- though I did put plenty of comments in. It's actually fairly simple: one big array of settings, and then some classes do all the work. Your own work on this would have been extremely simple, as all you had to change in the .test file was some values in the data array:
-- the test classes would have done the rest for you.
The test file could then have been simplified as we no longer needed to work with two different form keys, so could replace 'form_name_override' everywhere with 'form_name_normal'.
If the only problem with my version of this was that you didn't manage to follow what was going on, then I'd rather you'd asked me to deal with the reworking it needed for this issue rather than have it completely changed; it's a little disheartening.
Comment #8
dave reidI didn't want or mean to discourage you. I know it's basically replacing the tests that you wrote. :/ I can follow tests pretty well, but the thing is that we don't really do tests this way as has been standardized in core. For instance there's specific things that needed to be tested with authoring information and validation that did not need to be run with other tests, and would not have been possible with the level of generalization that was currently in the tests. It's also much more difficult to debug if things go wrong.
Comment #9
dave reidCommitted to CVS.
http://drupal.org/cvs?commit=308680
Comment #10
joachim commentedJust took a look at the latest release -- our hook_form_alter is hugely slimmed down and much easier to follow. Nice job all round. And fair enough about the tests. :)