For example, adding a colon right after keyword+field pair completely brakes this keyword (the keyword+field and extra colon will be replaced with empty string).

Try this one (without quotes) as a title pattern for node_view page: "%node:title: hello". It will always be " hello", but it shouldn't.

Patch follows.

Files: 
CommentFileSizeAuthor
#26 ctools-1727804-26.patch1.17 KBsylus
PASSED: [[SimpleTest]]: [MySQL] 68 pass(es).
[ View ]
#12 1727804-test-only.patch619 bytesandypost
FAILED: [[SimpleTest]]: [MySQL] 67 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#6 ctools-1727804-6.patch1.69 KBDmitriy.trt
PASSED: [[SimpleTest]]: [MySQL] 68 pass(es).
[ View ]
#1 ctools-1727804-1-test-only.patch2.84 KBDmitriy.trt
FAILED: [[SimpleTest]]: [MySQL] 63 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
#1 ctools-1727804-1.patch3.45 KBDmitriy.trt
PASSED: [[SimpleTest]]: [MySQL] 67 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new3.45 KB
PASSED: [[SimpleTest]]: [MySQL] 67 pass(es).
[ View ]
new2.84 KB
FAILED: [[SimpleTest]]: [MySQL] 63 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Test without the fix and with it.

Status:Needs review» Fixed

Nice! It's not often someone provides tests!!

Status:Fixed» Needs work

This has caused downstream (untested) problems with tokens - see #1741064-1: Context token substitutions. Substitutions such as '%node:field-address:postal_code' (using the Address Field module) no longer work.

Not so sure this patch to this issue has really helped: having a ':' at the end of a token (eg, %node:title: hello) is plainly a syntax error made by a human, a little bit beyond an edge-case?

Feel free to slap my wrists, by re-opening and effectively duplicating the issue...

Sorry, my mistake. This code needs a rewrite to support tokens with multiple levels and don't break on some edge cases. Unfortunately, don't have time to do it right now, but I'll find some time on this week.

@arrrgh Sometimes a colon right after the token is what you want, not a mistake. For example, you want to print "Company title: product name" using substitutions and you can't do it with previous regexp! There is no escape character or some other way to avoid breaking whole token by this colon.

And here we enter Regex hell...

I'm tied up for next 2 weeks too, but did a quick 'undo' patch (as this has been committed to 7.x-1.2 release) at #1741064-2: Context token substitutions

Status:Needs work» Needs review
StatusFileSize
new1.69 KB
PASSED: [[SimpleTest]]: [MySQL] 68 pass(es).
[ View ]

Patch with the fix of multi-level tokens broken by previous one. Also, adds test for this case.

Status:Needs review» Reviewed & tested by the community

All looks fine with me - thanks for the quick rework @Dmitriy.trt. I daresay there are a few really extreme edge cases out there where the colon will screw things up... but I couldn't find them.

Considering that @merlinofchaos already committed the original change, and this was a minor additional tweak, I thinks it's safe to RTBC.

Patch in #6 works for me (using with Panels), RTBC +1.

Priority:Normal» Critical

+1 RTBC all site:* & node:* tokens are broken!!! This really critical because leads to data loss because tokens are not availiable

Suppose better to use site:*token for test because node:* tokens could not be available (node module for D8 optional)

Patch in #6 works for me as well, thank you!
RTBC +1

Priority:Critical» Major
Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs tests

This should have simpletests.

Status:Needs work» Needs review
StatusFileSize
new619 bytes
FAILED: [[SimpleTest]]: [MySQL] 67 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Patch with only fixed test from #6 - should fail

The last submitted patch, 1727804-test-only.patch, failed testing.

Status:Needs review» Reviewed & tested by the community
Issue tags:-Needs tests

Current functionality already covered with tests. #6 just extends this with multilevel token

Any idea if this patch will be committed? DEV snapshot on 25 Sep doesn't reflect the patch at #6.

What else needs doing?

Can we get #6 commited?

#6: ctools-1727804-6.patch queued for re-testing.

#6 is working great for me, and it includes a test for the regression caused by #1. #1 provided tests for other situations.

So this has tests and is RTBC. Is it missing anything?

I can confirm too, that #6 works as expected.

I can confirm that #6 fixes this problem.
RTBC +1

Confirm, the patch in #6 works as expected.

Patch in #6 is RTBC. Please, commit it ASAP.

Thanks for patch #6, it work like a charm.

Any progress in this issue?

Has the patch in #6 been committed?

Please advise.

Thanks

StatusFileSize
new1.17 KB
PASSED: [[SimpleTest]]: [MySQL] 68 pass(es).
[ View ]

Updating patch for latest dev sha: be26071

Status:Reviewed & tested by the community» Fixed

Doh. And here I thought this had been fixed.

Committed and pushed. Thanks!

Status:Fixed» Closed (fixed)

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

Issue summary:View changes

Correct unexpected result