I've had at least two conversations in irc with people very, very confused by the terms as both fieldable and field situation in D7, one with pwolanin, one with chx. pwolanin and chx being confused by something doesn't bode well for everyone else.

chx suggested changing 'taxonomy_term' to 'taxonomy_term_reference' to make it clear that terms are referenced by the field, rather than field values themselves (this is what differs it from a text field basically).

This kinda sorta changes some APIs - people altering stuff around the term field might need to alter the name, but I'd be surprised if anyone's doing anything that deep with term fields yet, so it'd probably be a transparent change in real terms.

CommentFileSizeAuthor
#18 term_reference.patch6.85 KBcatch
#15 term_reference.patch4.76 KBcatch
#13 term_reference.patch4.35 KBcatch
#10 taxonomy_field_name.patch3.89 KBcatch
#8 taxonomy_field_name.patch3.99 KBcatch
#2 taxonomy_field_name.patch4.1 KBcatch
#1 ttr.patch3.3 KBchx

Comments

chx’s picture

chx commented
Status: Active » Needs review
StatusFileSize
new ttr.patch3.3 KB
catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new taxonomy_field_name.patch4.1 KB

Removed all the field_attach_*() changes since that's about term as entity, not as field. Added a bunch of changes for the formatters. See what the bot thinks.

Anonymous’s picture

Anonymous (not verified) commented

will review today after coffee.

yched’s picture

yched commented

I think I approve - although to be fair, file and image fields are also actually 'reference' fields...

- The field type 'label' is still ''Taxonomy term', though ?
- Note that formatter names don't strictly need to be '[field_type]_[formatter_name]'. We just highly recommend prefixing by the module name to avoid clashes.

yched’s picture

yched commented

BTW, the entity type is 'term', so the field type name should probably just be 'term_reference' ?

catch’s picture

catch
he/him
Primary language English
commented

'Taxonomy term reference' seemed a bit long.

If we're prepared to make an exception in the module prefix stuff and call this "Term reference" both in UI and code, I'd be really happy with that.

yched’s picture

yched commented

I support #6.

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new taxonomy_field_name.patch3.99 KB

OK let's see if this works. Hope test bot comes back soon...

yched’s picture

yched commented 
+++ modules/taxonomy/taxonomy.module	21 Dec 2009 12:27:40 -0000
@@ -962,11 +962,11 @@ function taxonomy_implode_tags($tags, $v
-    'taxonomy_term' => array(
+    'term_reference' => array(
       'label' => t('Taxonomy term'),

Do we want to keep 'Taxonomy term' ? (appears in the 'Field type' dropdown select in the 'Add new field' UI)

I'm on crack. Are you, too?

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new taxonomy_field_name.patch3.89 KB

Argh, should review my own patches before posting :( I don't think we do, no.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Needs work

This patch needs to fix function rdf_field_attach_view_alter().

yched’s picture

yched commented

Side note @vangpound (& catch) about rdf_field_attach_view_alter(): #652834-35: Field formatters as render arrays
In short: remove the alter() implementation and append rdf data for the term directly in the taxo formatter(s).

If you guys think it's valid, it would be cool to open an issue and get scor comment on it ?

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new term_reference.patch4.35 KB

See what the bot thinks of this.

Status: Needs review » Needs work

The last submitted patch, term_reference.patch, failed testing.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new term_reference.patch4.76 KB

Status: Needs review » Needs work

The last submitted patch, term_reference.patch, failed testing.

chx’s picture

chx commented

Why term_reference ? yched suggested this because the entity type is term but instead the entity type should be taxonomy_term. Let's be consistent with our "prefix w module" ...?

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new term_reference.patch6.85 KB

New patch: in the code, we call it 'taxonomy_term_reference', in the UI, it's just "Term reference" - that's consistent with both module prefixes, and how we refer to terms in help text etc. and it's just abbreviated in the UI, not completely different. Hopefully that satisfies everyone here, and it makes the most sense for me (except for taxonomy_term_reference being a bit long, but that's not a big deal).

Also fixed all taxonomy test failures, didn't run whole test suite though.

chx’s picture

chx commented
Status: Needs review » Reviewed & tested by the community

Hm, this patch undermines my excellent business plan to operate an 1-900 phone line to explain how taxonomy works in D7. Pity :p

pwolanin’s picture

pwolanin commented

only $0.05 for the first minute and $50 for each additional minute...

sounds good to me too - I think people already get the concept of user reference or node reference, so this will make it more apparent that we also just reference the term object via a field.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Yep. Agreed with this change. The fact that you can tag tags in D7 is already a big enough brain-screw, we don't need to make the term itself confusing, too. ;)

Committed to HEAD.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.