Problem/Motivation
Comments are multilingual but no token provided
Proposed resolution
Add "langcode" token like node.tokens.inc
does
Remaining tasks
User interface changes
API changes
Beta phase evaluation
Issue category | Task because adding "langcode" token is a feature that good to have and can't be considered as a bug because without the token the system doesn't break. |
---|---|
Issue priority | Normal Task: This is nice to have but doesn't impact the system if it's not present. |
Prioritized changes | Making this issue as a prioritized change as it improves usability and user experience improvements by providing a new token to the system. |
Comment | File | Size | Author |
---|---|---|---|
#24 | langcode-token-to-comment-2378565-24.patch | 2.64 KB | adci_contributor |
#12 | langcode-token-to-comment-2378565-12.patch | 2.64 KB | aneek |
#12 | interdiff-2378565-8-12.txt | 797 bytes | aneek |
#8 | langcode-token-to-comment-2378565-8.patch | 2.15 KB | aneek |
#8 | langcode-token-to-comment-2378565-test-only.patch | 1019 bytes | aneek |
Comments
Comment #1
rpayanmTrying...
Comment #3
andypostA lot of unnecessary changes
should be a $comment
Comment #4
andypostA lot of unnecessary changes
should be a $comment
Comment #5
rpayanmOh yes! :)
Comment #7
andypostyes, also needs extend tests
unneeded change, addressed in own issue (see related issue)
Comment #8
aneek CreditAttribution: aneek commented@rpayanm,
Removed not needed code from your patch and added a check in the test "CommentTokenReplaceTest".
Review!!
Comment #9
aneek CreditAttribution: aneek commentedComment #11
andypost2 nitpicks, suppose there's no need in sanitization
not sure we need to sanitize the machine name
There's unsanitized test case around, maybe extend it too?
Comment #12
aneek CreditAttribution: aneek commented@andypost, I'd like to have the
String::checkPlain()
. I think while creating the patch @rpayanm followed the same pattern fromnode.token.inc
. So it also has the checkplain check for some reason (may be).But the "unsanitized test case" is added in this patch.
Comment #13
andypostI'm fine with current implementation
Comment #14
alexpottThis issue is a normal task so we need to outline how it fits within the allowable Drupal 8 beta criteria. Can someone add Drupal 8 beta phase evaluation template to the issue summary.
Comment #15
aneek CreditAttribution: aneek commentedComment #16
aneek CreditAttribution: aneek commentedComment #18
aneek CreditAttribution: aneek commentedgit apply --check showed no error. Strange!! Marking for review again.
Comment #20
aneek CreditAttribution: aneek commentedComment #22
adci_contributor CreditAttribution: adci_contributor commentedRerolled after #2380023: Clean-up Comment module Test members - ensure property definition and use of camelCase naming convention
Comment #24
adci_contributor CreditAttribution: adci_contributor commentedAgain
Comment #25
aneek CreditAttribution: aneek commentedComment #26
alexpottCommitted b3f2319 and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation for to the issue summary.
Comment #28
Gábor HojtsyFix tags.
Comment #29
aneek CreditAttribution: aneek commented