New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix JIT error with regex recipes #110

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@tmslnz
Contributor

tmslnz commented Aug 12, 2015

break 2 is causing regex mode to fail. I am not sure why though as it makes sense to break out the foreach if the $url-parameter matches.

I’m using capture groups. Symphony errors out in both 2.3.6 (!) and 2.6.2. In both cases removing the break argument 2 resolves the issue. I don’t have the error log for 2.6.2 at hand, but 2.3.6 prints the following:

2015/08/12 23:48:43 > Warning: 2 - preg_match(): Unknown modifier 'r' in file /home/user/webapps/site/symphony/extensions/jit_image_manipulation/lib/image.php on line 63
2015/08/12 23:48:43 > Notice: Image class param dump - mode: 0, width: 0, height: 0, position: 0, background: 0, file: , external: 0, raw input: resize-width-huge-0/images/image.jpg

The JIT recipe file is this:

<?php

    $recipes = array(

        ########
        array(
            'mode' => 'regex',
            'name' => 'Resize Width Small',
            'url-parameter' => '/resize-width-small-([0-1])/i',
            'jit-parameter' => '1/800/0/$1',
            'quality' => '85',
        ),
        ########

        ########
        array(
            'mode' => 'regex',
            'name' => 'Resize Width Medium',
            'url-parameter' => '/resize-width-medium-([0-1])/i',
            'jit-parameter' => '1/1200/0/$1',
            'quality' => '85',
        ),
        ########

        ########
        array(
            'mode' => 'regex',
            'name' => 'Resize Width Large',
            'url-parameter' => '/resize-width-large-([0-1])/i',
            'jit-parameter' => '1/1800/0/$1',
            'quality' => '80',
        ),
        ########

        ########
        array(
            'mode' => 'regex',
            'name' => 'Resize Width Huge',
            'url-parameter' => '/resize-width-huge-([0-1])/i',
            'jit-parameter' => '1/2400/0/$1',
            'quality' => '75',
        ),
        ########

    );
Fix JIT error with regex
`break 2` is causing regex mode to fail. I am not sure why though as it makes sense to break out the `foreach` if the `$url-parameter` matches.

I’m using capture groups. Symphony errors out in both 2.3.6 (!) and 2.6.2. In both cases removing the break argument `2` resolves the issue. I don’t have the error log for 2.6.2 at hand, but 2.3.6 prints the following:

```txt
2015/08/12 23:48:43 > Warning: 2 - preg_match(): Unknown modifier 'r' in file /home/user/webapps/site/symphony/extensions/jit_image_manipulation/lib/image.php on line 63
2015/08/12 23:48:43 > Notice: Image class param dump - mode: 0, width: 0, height: 0, position: 0, background: 0, file: , external: 0, raw input: resize-width-huge-0/images/image.jpg
```

The JIT recipe file is this:

```php
<?php

	$recipes = array(

		########
		array(
			'mode' => 'regex',
			'name' => 'Resize Width Small',
			'url-parameter' => '/resize-width-small-([0-1])/i',
			'jit-parameter' => '1/800/0/$1',
			'quality' => '85',
		),
		########

		########
		array(
			'mode' => 'regex',
			'name' => 'Resize Width Medium',
			'url-parameter' => '/resize-width-medium-([0-1])/i',
			'jit-parameter' => '1/1200/0/$1',
			'quality' => '85',
		),
		########

		########
		array(
			'mode' => 'regex',
			'name' => 'Resize Width Large',
			'url-parameter' => '/resize-width-large-([0-1])/i',
			'jit-parameter' => '1/1800/0/$1',
			'quality' => '80',
		),
		########

		########
		array(
			'mode' => 'regex',
			'name' => 'Resize Width Huge',
			'url-parameter' => '/resize-width-huge-([0-1])/i',
			'jit-parameter' => '1/2400/0/$1',
			'quality' => '75',
		),
		########

	);

```
@brendo

This comment has been minimized.

Show comment
Hide comment
@brendo

brendo Aug 12, 2015

Member

Nice catch, break makes sense to jump out of the foreach, but break 2 makes absolutely no sense as there is not a second loop. Are you able to push this PR to integration rather than master?

Thanks!

Member

brendo commented Aug 12, 2015

Nice catch, break makes sense to jump out of the foreach, but break 2 makes absolutely no sense as there is not a second loop. Are you able to push this PR to integration rather than master?

Thanks!

@tmslnz

This comment has been minimized.

Show comment
Hide comment
@tmslnz

tmslnz Aug 12, 2015

Contributor

So I close this and re-PR on your integration branch?

Contributor

tmslnz commented Aug 12, 2015

So I close this and re-PR on your integration branch?

@tmslnz

This comment has been minimized.

Show comment
Hide comment
@tmslnz

tmslnz Aug 12, 2015

Contributor

On a side note, does anyone use regex recipes or I am the only crazy :) ? Seems weird that no-one has stumbled on this until now…

Contributor

tmslnz commented Aug 12, 2015

On a side note, does anyone use regex recipes or I am the only crazy :) ? Seems weird that no-one has stumbled on this until now…

@brendo

This comment has been minimized.

Show comment
Hide comment
@brendo

brendo Aug 13, 2015

Member

Many thanks, all merged on integration now. I haven't used the regex functionality on JIT for quite some time to be honest!

Member

brendo commented Aug 13, 2015

Many thanks, all merged on integration now. I haven't used the regex functionality on JIT for quite some time to be honest!

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Aug 13, 2015

Member

does anyone use regex recipes

I never did... but seems like option (I should use it!)

Member

nitriques commented Aug 13, 2015

does anyone use regex recipes

I never did... but seems like option (I should use it!)

@tmslnz

This comment has been minimized.

Show comment
Hide comment
@tmslnz

tmslnz Aug 13, 2015

Contributor

Quite handy to avoid repeating rules.
If you see my recipe it's nothing mind-bending, just using it to swap internal/external image

On 13 Aug 2015, at 16:36, Nicolas Brassard notifications@github.com wrote:

does anyone use regex recipes

I never did... but seems like option (I should use it!)


Reply to this email directly or view it on GitHub.

Contributor

tmslnz commented Aug 13, 2015

Quite handy to avoid repeating rules.
If you see my recipe it's nothing mind-bending, just using it to swap internal/external image

On 13 Aug 2015, at 16:36, Nicolas Brassard notifications@github.com wrote:

does anyone use regex recipes

I never did... but seems like option (I should use it!)


Reply to this email directly or view it on GitHub.

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Aug 13, 2015

Member

Quite handy to avoid repeating rules.

Indeed. But we have xsl templates that deals with this and never thought about replacing it. (but maybe we should)

Member

nitriques commented Aug 13, 2015

Quite handy to avoid repeating rules.

Indeed. But we have xsl templates that deals with this and never thought about replacing it. (but maybe we should)

@tmslnz

This comment has been minimized.

Show comment
Hide comment
@tmslnz

tmslnz Aug 13, 2015

Contributor

True that. Though we use Symphony as a pure data backend for node apps, no templating involved on symphony’s end, just XML.
Also prefer the natural “obscurity” of recipes vs. the standard JIT URI in the HTML source.

On 13 Aug 2015, at 16:43, Nicolas Brassard notifications@github.com wrote:

Quite handy to avoid repeating rules.

Indeed. But we have xsl templates that deals with this and never thought about replacing it. (but maybe we should)


Reply to this email directly or view it on GitHub.

Contributor

tmslnz commented Aug 13, 2015

True that. Though we use Symphony as a pure data backend for node apps, no templating involved on symphony’s end, just XML.
Also prefer the natural “obscurity” of recipes vs. the standard JIT URI in the HTML source.

On 13 Aug 2015, at 16:43, Nicolas Brassard notifications@github.com wrote:

Quite handy to avoid repeating rules.

Indeed. But we have xsl templates that deals with this and never thought about replacing it. (but maybe we should)


Reply to this email directly or view it on GitHub.

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Aug 13, 2015

Member

Yeah, you're right.

Member

nitriques commented Aug 13, 2015

Yeah, you're right.

@brendo brendo closed this Aug 24, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment