Autocomplete widget for taxonomy term fields

"It should be able to work for term and list fields"
I'd even say it would make sense for regular text fields as well... Even though those don't have a closed list of 'allowed' values, you might still want to get feedback about existing values...

+++ modules/field/modules/options/options.module	9 Aug 2009 16:29:06 -0000
@@ -92,6 +116,11 @@ function options_elements() {
       ),
+    'options_autocomplete' => array(
+      '#input' => TRUE,
+      '#columns' => array('value'), '#delta' => 0,
+      '#process' => array('options_autocomplete_elements_process'),
+    ),
     );

Indentation is wrong

+++ modules/field/modules/options/options.module	9 Aug 2009 16:29:06 -0000
@@ -103,6 +132,7 @@ function options_field_widget(&$form, &$
+  // TODO: autocomplete textfield needs to do something here.

not sure what you mean

+++ modules/field/modules/options/options.module	9 Aug 2009 16:29:06 -0000
@@ -255,6 +285,131 @@ function options_autocomplete_elements_process(
+    foreach ($element['#default_value'] as $item) {
+      // TODO: do i need to load the term? when is the term not available?
+      $tags[$item['value']] = isset($item['taxonomy_term']) ? $item['taxonomy_term'] : taxonomy_term_load($item['value']);
+    }
+    $element['#value'] = taxonomy_implode_tags($tags);

also only works for taxonomy
+ I don't think the load is needed. The items here come from a field_attach_load(), so the full terms should be loaded, right ?

+++ modules/field/modules/options/options.module	9 Aug 2009 16:29:06 -0000
@@ -255,6 +285,131 @@ function options_autocomplete_elements_process(
+  $element[$field_key]['#maxlength'] = !empty($field['settings']['max_length']) ? $field['settings']['max_length'] : NULL;

not needed, I think, this is for text fields

+++ modules/field/modules/options/options.module	9 Aug 2009 16:29:06 -0000
@@ -255,6 +285,131 @@ function options_autocomplete_elements_process(
+// Set #element_validate in a way that it will not wipe out other
+  // validation functions already set by other modules.

indentation

+++ modules/field/modules/options/options.module	9 Aug 2009 16:29:06 -0000
@@ -255,6 +285,131 @@ function options_autocomplete_elements_process(
+    /**
+     * @todo Use the same module or API hook required for autocomplete, which
+     * must also support multiple values so validation, transformation can be
+     * quick.
+     * @see options_autocomplete
+     */

Hm, not easy indeed.
Well, maybe there's a clever way to do this, but I don't see it right now. IMO it's perfectly OK to forget about abstracting to 'list' fields as well, and provide an autocomplete widget just for term fields.
If list fields want an autocomplete, at best we can figure a clever way to merge this back into a single widget within code freeze (/me is dubious), at worst list.module or a contrib module will provide a dedicated widget for list fields, no big deal. Sorry for the suggestion (er - actually no, it was your idea initially ;-), I "merely" suggested we extend it to regular text fields as well. Anyway)

20 days to code freeze. Better review yourself.

Also :

+++ modules/field/modules/options/options.pages.inc	9 Aug 2009 16:29:06 -0000
@@ -0,0 +1,88 @@
+  $instance = field_read_instance($field_name, $bundle);
+  $field = field_read_field($field_name);

Use field_info_field() and field_info_instance() instead. Those are the cached, updated to for current execution context, versions. field_read_*() cause a db hit each + some array massaging. Making the distinction cleaner in PHPdocs is on my personal TODO after #367753: Bulk deletion lands.

Beer-o-mania starts in 20 days! Don't drink and patch.

Previewing nodes meant the formatters received just the field values (term IDs) and not the whole term object. I loaded these in the element's validate function. Is that the right thing to do?

Good call, previewed values don't come from field_attach_load(). Bummer.
Loading those in a validate function sounds like a hack. All 'term field' widgets will have the same issue, BTW, not only the autocomplete one, right ?
I'd say it belongs to the formatter to load the data it needs if not available...

Then again: the widget layer is supposed to hand out 'ready to store' values, meaning : similar to the ones we take from the stored data. Maybe field API should automatically run hook_field_load() on those on preview, before handing them to the display layer. That way the workflow on 'preview' would be much more similar to the workflow on regular 'load-then-display'. Not sure of the best way to tie this in Field API right now.

The other points are more taxonomy-related, I'll pass on those ;-)

tagging

Just a very minor DBTNG thing...

+      ->condition('t.vid', $vids, 'IN')

I think it defaults to IN now for arrays of values, so you don't need to specify it explictly.

@bangpund: you can roughly test the approach I describe in #14 by adding

_field_invoke_multiple('load', 'node', array($node->nid => $node));

at the beginning of node_preview() - obviously not the final code, but should allow us to test the approach at least.

Discussed with catch on IRC: problem with running hook_field_load() on previews is that newly created tags don't exist yet, so term_field_load() wouldn't get the tag name and probably discard the entry.
Still, I think running hook_field_load() on previews makes tons of sense, so we agreed one workaround would be to have the widget validation step automatically create the new tag even on Preview, and pass on the tid. Then, the tag exists and hook_field_load() will load the additional data needed for preview display.

[edit: I think that IRC discussion happened after #19 ;-) ]

Subscribing for now!

See #549710: Fields not rendered properly during comment preview for the 'real' version of "run hook_field_load() on previews".
Depending on which patch goes in first, the other one will adapt and cleanup.

Note: I'm tired and also don't quite understand yched's #28. So feel free to disregard any of these comments that don't really make sense.

+++ modules/taxonomy/taxonomy.module	14 Aug 2009 17:53:58 -0000
@@ -2052,3 +2091,144 @@ function _taxonomy_clean_field_cache($te
+  $field_key  = $element['#columns'][0];

Extra space here before =.

+++ modules/taxonomy/taxonomy.module	14 Aug 2009 17:53:58 -0000
@@ -2052,3 +2091,144 @@ function _taxonomy_clean_field_cache($te
+ * Process an individual element.
+ *
+ * Build the form element. When creating a form using FAPI #process,
+ * note that $element['#value'] is already set.
+ *
+ * The $field and $instance arrays are in $form['#fields'][$element['#field_name']].

Process an individual what element? A term? An autocomplete widget?

In general, it sounds like this documentation might be useful to someone who understands field API but it's completely greek to me. :\ Would this be more at home in the inline comments, or does it make sense here? Is there any way you can provide more context to someone who might need to call this function?

+++ modules/taxonomy/taxonomy.module	14 Aug 2009 17:53:58 -0000
@@ -2052,3 +2091,144 @@ function _taxonomy_clean_field_cache($te
+ * @todo Determine whether usability is harmed by allowing "multiple" to be 
+ * configurable.

Are we able to force multiple? That certainly seems like it would harm usability, forcing people to make the choice manually.

+++ modules/taxonomy/taxonomy.module	14 Aug 2009 17:53:58 -0000
@@ -2052,3 +2091,144 @@ function _taxonomy_clean_field_cache($te
+      // TODO: do i need to load the term? when is the term not available?

This sounds like it needs to be sorted out in this patch, or not?

+++ modules/taxonomy/taxonomy.module	14 Aug 2009 17:53:58 -0000
@@ -2052,3 +2091,144 @@ function _taxonomy_clean_field_cache($te
+    '#autocomplete_path' => 'taxonomy/autocomplete/'. $element['#field_name'] .'/'. $element['#bundle'],

Do we need to run that through check_url() or is that just being stupidly paranoid?

+++ modules/taxonomy/taxonomy.module	14 Aug 2009 17:53:58 -0000
@@ -2052,3 +2091,144 @@ function _taxonomy_clean_field_cache($te
+  if (empty($element['#element_validate'])) {
+    $element['#element_validate'] = array();
+  }

Hm. This property is not instantiated for you by FAPI? We might want to make a follow-up issue for that, since that seems like an annoying bit of code to have to keep in every element definition function.

+++ modules/taxonomy/taxonomy.module	14 Aug 2009 17:53:58 -0000
@@ -2052,3 +2091,144 @@ function _taxonomy_clean_field_cache($te
+  // Free tagging vocabularies do not send their tids in the form,
+  // so we'll detect them here and process them independently.

Sad panda. :( No way to couple these together?

+++ modules/taxonomy/taxonomy.module	14 Aug 2009 17:53:58 -0000
@@ -2052,3 +2091,144 @@ function _taxonomy_clean_field_cache($te
+      $typed_term_tid = NULL; // tid match, if any.

Please don't put comments to the right, only on top of the line to which they refer.

+++ modules/taxonomy/taxonomy.module	14 Aug 2009 17:53:58 -0000
@@ -2052,3 +2091,144 @@ function _taxonomy_clean_field_cache($te
+        $term = (object)$edit;

Space between the cast operator and the variable.

+++ modules/taxonomy/taxonomy.module	14 Aug 2009 17:53:58 -0000
@@ -2052,3 +2091,144 @@ function _taxonomy_clean_field_cache($te
+function taxonomy_elements() {
+  return array(
+    'taxonomy_autocomplete' => array(

Too bad we couldn't do a general autocomplete, but I understand the difficulties you hit.

Beer-o-mania starts in 16 days! Don't drink and patch.

The task mentioned in the comment block implies that there's some undesirable tight coupling between the element and the widget, such that the only valid way to call this now is to use the Field API widget

That TODO is already present in text and option modules, I don't think we need to reflect it everywhere. I'm also not sure we can get that task done in time, I let #362058: Field Types: Make widget FAPI elements field agnostic. drift off my radar :-(

There's no #multiple setting on text fields for autocomplete. Really we're talking about the cardinality of the field. Can a widget be restricted only to fields that support multiple values? Or can a widget force a field to accept multiple values? I don't really know.

No on both points. Any field can be single or multiple and any widget handling multiple values ($widget_type['behaviors']['multiple values'] = FIELD_BEHAVIOR_CUSTOM) should handle the case where the field is actually single valued (i.e. in our case, fail validation if user input is "foo, bar")

#element_validate is not instantiated automatically with an empty array, but it could arrive with some already-defined validation functions. I can't tell you whether it's a good idea to set up the element arrays with an empty array for #element_validate. I'll keep it in for now.

Yeah, that's over-cautious code coming from CCK D6. I I saw it mentioned in other threads that a straight $array[] = 'foo' even if $array is not initialized as an array is tolerated, because the 'clean' way makes dull and verbose code.

I guess you could have a use case for a non-freetag taxo field with cardinality 1 or n (pick one value in this list). Obviously a 'taxo term' field on a freetag vocab with cardinality = 1 would be strange, but I don't think we can or want to introduce the notion of 'some field types under some conditions on field settings (vid) should always be multiple'.

At best, I guess the 'settings' form provided by Field UI could be form_altered to prevent or guide away from silly choices ?

The autocomplete callbacks might need a small re-roll after #544418: Integrate CTools AJAX framework with Drupal to extend (and replace) existing ahah framework.

However, the more immediate problem is I tried this with the #516138: Move CCK HEAD in core patch and no field is appearing where I think there should be one:

I'm sorry, but I don't know enough about the various interactions to know if this is a bug in Field UI or a bug in taxonomy autocomplete, but I can't commit this yet. :(

re webchick #35 - no widget: hm, I guess that depends on the order in which you apply the patches, but a manual registry rebuild and cache clear are probably needed. Should be OK in a real-life scenario.

Duh. Clear caches! What a n00b! :)

When using this with the Field UI patch, and adding a "Tagz" field to Page nodes, I'm still getting a few errors/weirdness. Please let me know which of these all under the purview of stuff that can be dealt with here and which do not.

--

I need to manually select "Unlimited" in the field settings in order for this field to act the way I would anticipate. But more troubling, when I attached a field that allowed 1 value to users, the data I entered just got silently truncated after the first value. EEK! I would expect an error message here. But I think this might be a bug in user module's Field API implementation.

Upon creating a field, after saving its final settings form:

Notice: Undefined index: field_tagz in taxonomy_autocomplete_validate() (line 2207 of /Users/webchick/Sites/core/modules/taxonomy/taxonomy.module).

function taxonomy_autocomplete_validate($element, &$form_state) {
  // Autocomplete widgets do not send their tids in the form, so we must detect
  // them here and process them independently.
  if ($form_state['values'][$element['#field_name']]['value']) { ## This is line 2207.

By just *visiting* the Page node add form, on the following page load, I get:

    * Notice: Array to string conversion in drupal_validate_utf8() (line 1128 of /Users/webchick/Sites/core/includes/bootstrap.inc).
    * Warning: preg_match() expects parameter 2 to be string, array given in drupal_validate_utf8() (line 1134 of /Users/webchick/Sites/core/includes/bootstrap.inc).

The tags I enter in one of my Page's tags field do not show up in my Article field (which comes from core taxonomy) as something to select from. I assume this is because this patch has to change the storage from the term_data table to the entity_field_whatever table.

re Webchick #38: I'm not where I can apply and test the patch, but it does have some code to raise an error if too many values are entered. Field validation on users is indeed borched, see #549726: user_profile_form_validate() not called when submitting user_profile_form.
More generally, this 'Check we don't exceed the allowed number of values ' code in taxonomy_field_validate() is absolutely not specific to Term fields or this autocomplete widget. I'm thinking this check should in fact be performed on all fields in a field_default_validate() function in field.default.inc. Followup patch.

About default cardinality set to '1': As I said in #33, I think that's an UI / form_alter issue. That's a bit complex though, because IMO having the cardinality default to 1 or unlimited depending on 'some criteria' might be equally confusing to users...

Hm. I tried the code at #39 in a PHP node and just ended up with a WSOD.

Since testing bot is fine, and so are at least two other people reviewing this patch, I'm going to chalk this up to some sort of weird environment thing I've got going here, probably related to remnants of one of the 70,000 patches I've tested on this installation. Once #516138: Move CCK HEAD in core is in, it'll be much easier to track down any problems this patch may or may not have introduced. Tests will be updated in the patch that converts legacy taxonomy_node_X cruft to proper Field API, and then we'll be able to shake out anything else.

Therefore... committed to HEAD! :) Let the taxonomy evisceration continue! ;)

Followup as discussed in #40 : #552436: Validation of the number of values in field_default_validate()

I'm still getting the warning that webchick saw in #38 after adding a term field to a node (using the autocomplete widget):
Notice: Undefined index: field_tags in taxonomy_autocomplete_validate() (line 2207 of /Users/amorton/Sites/dh/modules/taxonomy/taxonomy.module).