The token system has "type"ing as part of the array definition of tokens, but they don't conform to our typed data system or data definition. We should rectify this so that we can reliably get tokens for typed data objects w/o the need to convert from typed data syntax to token syntax.

Eclipse

CommentFileSizeAuthor
#22 drupal_2163027_22.patch21.48 KBberdir
#18 drupal_2163027_18.patch21.81 KBxano
#13 2163027-13.patch21.65 KBeclipsegc
#5 d8-token-returns-2163027-5.patch20.11 KBjibran
#1 2163027.patch20.11 KBeclipsegc

Issue fork drupal-2163027

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

eclipsegc’s picture

eclipsegc commented
StatusFileSize
new 2163027.patch20.11 KB
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

test-bot:ping
pickup the patch.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Active

Trying status change.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Active » Needs review

Hmmm issue was posted NR. Let's see if test bot can pick it now.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
StatusFileSize
new d8-token-returns-2163027-5.patch20.11 KB

re-uploading the patch

The last submitted patch, 1: 2163027.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 5: d8-token-returns-2163027-5.patch, failed testing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs work » Needs review

5: d8-token-returns-2163027-5.patch queued for re-testing.

eclipsegc’s picture

eclipsegc commented

as I understand that particular test has issues right now unrelated to this patch. Anyone have thoughts on what I'm attempting to do here?

Eclipse

Status: Needs review » Needs work

The last submitted patch, 5: d8-token-returns-2163027-5.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

5: d8-token-returns-2163027-5.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 5: d8-token-returns-2163027-5.patch, failed testing.

eclipsegc’s picture

eclipsegc commented
Status: Needs work » Needs review
StatusFileSize
new 2163027-13.patch21.65 KB

I thought those errors were because of an issue in Views Area stuff I'd heard about. I was wrong, I think this should fix it.

Eclipse

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Needs review » Reviewed & tested by the community

I went through the patch and all looks good. This will help with #2164635: Automatically expose typed data to token API.

dawehner’s picture

dawehner
Primary language German
commented

I wonder whether we can use that somewhere in core.

fago’s picture

fago
Primary language German
Location Vienna
commented

great to see that - it makes a lot of sense!

@dawehner: Not sure, but e.g. if views fields would declare the data they provide using (typed) data definitions, it could be used to generate tokens while serving as possible input for VBO actions at the same time.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Reviewed & tested by the community » Needs review

I like the idea of this in principle but I don't think the implications are fully considered.

Doesn't this affect the actual token names? Or is this going to be causing a disconnect in the 'types' in hook_token_info() and the $type values checked in hook_tokens() (because I don't see any changes to hook_tokens() implementations)? If so, then that is an unacceptable DX regression.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
StatusFileSize
new drupal_2163027_18.patch21.81 KB

Re-roll.

Doesn't this affect the actual token names?

It may. Is it a problem if it does? We can always add aliases if we don't want to break BC, can't we?

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

18: drupal_2163027_18.patch queued for re-testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

18: drupal_2163027_18.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 18: drupal_2163027_18.patch, failed testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new drupal_2163027_22.patch21.48 KB

Re-roll. #2287733: Configure page title per display is a first example of integrating the token and context/typed-data system and this would make it much nicer...

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue tags: +Page Manager
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue tags: +API change
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work

And yeah, hook_tokens() is a mess now, because that is all still using the old token names, as are the tests.

The type right now maps directly to what is used in tokens and specified as available token types for e.g. the token tree UI.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

#1920688: Support multiple instances of the same token type in token replacement might help?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Priority: Normal » Major
Issue tags: +beta target

Bumping.

Status: Needs work » Needs review

fago queued 22: drupal_2163027_22.patch for re-testing.

fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Needs review » Needs work

Or is this going to be causing a disconnect in the 'types' in hook_token_info() and the $type values checked in hook_tokens() (because I don't see any changes to hook_tokens() implementations)?

As far as I can see it does not change the keys of $types, so the connection is still the same. But then it's weird that 'type' points to data type and not a token type. The best thing wold be align them, would means renaming some token types.

The patch misses documentation for the 'type' key being added there.

+++ b/core/modules/comment/comment.tokens.inc
@@ -92,21 +103,21 @@ function comment_token_info() {
-      'comment' => $comment,
+      'entity:comment' => $comment,

Great improvement, but a BC break now. :(

Not sure whether adding primitive types brings us anything though.

The last submitted patch, 22: drupal_2163027_22.patch, failed testing.

eclipsegc’s picture

eclipsegc commented

Adding primitive types lets you say "Plugin requires context of ContextDefinition('some:primitive')" and have it actually stick. You could generate a list of contexts from the primitives available from your tokens and actually figure out which ones are viable to use for context.

Eclipse

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue tags: -beta target

Is there a way for us to do this without breaking BC?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Version: 8.0.x-dev » 8.1.x-dev

No

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Looks the issue still makes sence but needs careful re-roll

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +Needs reroll

Bhanu951 made their first commit to this issue’s fork.

Bhanu951 opened merge request !2965

bhanu951’s picture

bhanu951 commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs issue summary update

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

This needs an issue summary update.

What's the proposed solution, remaining tasks.

Was tagged as an API change what was that? It will need a change record.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.