Custom key name for #text_format.

This is critical.

#text_format itself is an awful hack, so I fail to see why this is critical.

It is critical because it is becoming a blocker for the progress of Fields in core, see http://drupal.org/node/375907#comment-1392522. To have body as a field we need #text_format to be smarter.

I think you may have attached the wrong patch? :)

Alternative implementation.

This makes something fugly even fuglier. There's plenty of time until the code freeze to shoot #text_format in the face and go with a sane approach, ie: 1) give a richtext element its rightful place in the FAPI elements pantheon. And 2) always carry format and text around together (object / array).

Now with full tests.

And, yes, I don't like internal form processing. We want to exactly test what developers expect in their form submit handlers.

At first glance, @yched's patch in #6 looks better to me because it's nice and simple. Whereas if #text_format is sometimes an integer and sometimes an array, it seems like that will tend to lead to buggy code. For example, if I'm writing a hook_form_alter() implementation to change the default text format, I'd like to just be able to change it -- but now it seems in order to avoid bugs I'd have to write separate logic to change it differently based on whether it is an array or not. What is the proposed benefit of the more complicated approach?

And yes, the whole #text_format thing itself is a hack - see #125315: Textarea with input format attached for voluminous discussion, alternative implementations, etc. However, it's better than Drupal 6, which is what counts :) If someone wants to reopen the other issue I think that would be absolutely great, but that shouldn't stop this issue from going forward, IMO.

I can't say I love #text_format meaning two different things. If we do need that array in the form, define a new property but do not double load one property. In this regard, I dig #6 more because it does add a new property. It might be that #6 is not enough and we need all three which is fine. Do we need all three?

Well, also allowing to alter the weight was my reasoning.

Anyway, if all of you prefer a separate FAPI property, then yeah.

So #13 is basically the same as #6 with more documentation and tests :) Anyway, I agree with this -- keep it simple, especially if there will be followup discussions about ways to clean the whole process up.

+/**
+ * Submit handler for form_test_text_format_form().
+ */
+function form_test_text_format_form_submit($form, &$form_state) {
+  $output = '';
+  $output .= $form_state['values']['body'] . ':' . $form_state['values']['body_format'] . "\n";
+  $output .= $form_state['values']['another_body'] . ':' . $form_state['values']['another_body_format'] . "\n";
+  $output .= $form_state['values']['complex']['field_custom'] . ':' . $form_state['values']['complex']['complex_format_value'] . "\n";
+  echo $output;
+  exit;
+}

Please just JSON $form_state['values'] and do proper testing on the parent side.

Why another layer of indirection? So if conversion to JSON is broken, this test will pass? Where's the logic in that?

This is still a hack on a hack. Let's do that properly, and postpone this in favor of #414424: Introduce Form API #type 'text_format'.

I disagree.

We want to proceed with #375907: Make text field use D7 '#text_format' selector, which depends on this issue. I'll take care of #414424: Introduce Form API #type 'text_format' once the relevant issues (listed over there) have been committed.

Also, I still don't see why and how testing values in JSON would be better here. The test, as it stands, tests _exactly_ what developers expect, without another layer of indirection. If anything in this process does not work as it should, this test will expose it.

Re-rolled, since #304330: Text format widget has landed.

I'd prefer to see us focus on #414424: Introduce Form API #type 'text_format' first but I'm comfortable committing this because it comes with tests. Parts of these tests are re-usable, and the parts that fail will tell us what code to undo/redo. Ultimately, it comes down to timing. If someone volunteers to start work on #414424 in the next day or two, I might hold back this patch, but in absence of that, I will proceed with this.

testbot?

This was never committed, and in the meantime, it seems we have a much better solution to the overall problem over in #414424: Introduce Form API #type 'text_format'.

I need reviews over there.