Open Bug 855072 Opened 7 years ago Updated 5 years ago

t/012throwables.t should allow for error tags to be contained by single quotes and not just double

Categories

(Bugzilla :: Testing Suite, enhancement)

enhancement
Not set

Tracking

()

ASSIGNED

People

(Reporter: dkl, Assigned: dkl)

Details

Attachments

(1 file, 1 obsolete file)

Currently t/012throwables.t will find error tag names in templates that are surrounded by double quotes but not single quotes. Example:

extensions/Example/template/en/default/hook/global/user-error-errors.html.tmpl

PASS:
[% IF error == "example_my_error" %]

FAIL:
[% IF error == 'example_my_error' %]

Both are considered valid by TT2 and so the test should not fail.

Patch coming.
dkl
Attachment #729801 - Flags: review?(LpSolit)
You have to make sure you balance " with another ", and a ' with another '.

["']...["'] doesn't enforce that.
Severity: normal → minor
OS: Linux → All
Hardware: x86_64 → All
(In reply to Frédéric Buclin from comment #2)
> You have to make sure you balance " with another ", and a ' with another '.
> 
> ["']...["'] doesn't enforce that.


Wouldn't this result in a syntax error anyway, if they weren't balanced?
(In reply to Marc Schumann [:Wurblzap] from comment #3)
> (In reply to Frédéric Buclin from comment #2)
> > You have to make sure you balance " with another ", and a ' with another '.
> > 
> > ["']...["'] doesn't enforce that.
> 
> 
> Wouldn't this result in a syntax error anyway, if they weren't balanced?

True. Wouldn't t/004template.t catch those before t/012throwables.t?

dkl
"What's wrong with that?" is correctly balanced, but your regexp would catch "What' only.
Depends on: 855414
Sorry wrong bug id.
No longer depends on: 855414
Can't think of a reason anyone should ever use ' or " in an error tag name but suppose it could happen. This patch works around that issue by using a negative look around.

dkl
Attachment #729801 - Attachment is obsolete: true
Attachment #729801 - Flags: review?(LpSolit)
Attachment #730698 - Flags: review?(LpSolit)
Comment on attachment 730698 [details] [diff] [review]
Patch to t/012throwables.t to allow for single quotes (v2)

>+        if ($line =~ /\[%\s[A-Z]+\s*error\s*==\s*["']+((?!["']+\s*%\]]).+)["']+\s*%\]/) {
>             my $errtag = $1;

That's way too complicated, and ["']+ is not correct here. Simply use \1 to catch exactly what has been caught initially:

(["'])(\s+)\1

Here \1 will be replaced by what has been caught by ["'], so that it remains correctly balanced. This also means that $errtag must now look at $2.
Attachment #730698 - Flags: review?(LpSolit) → review-
Severity: minor → enhancement
Target Milestone: --- → Bugzilla 5.0
> (["'])(\s+)\1

Err... I meant (["'])(.+)\1 of course.
Target Milestone: Bugzilla 5.0 → ---
You need to log in before you can comment on or make changes to this bug.