Skip to content

Conversation

wildtangent
Copy link

Make the shortcode parser capable of supporting both quote types.

Since Wordpress can create shortcodes with either or both types of quotes in the same shortcode, performing a regex match instead of string match has more flexibility if processing old shortcodes where you may not be confident of the format.

Specs updated ands should pass. Docs updated.

Version bump on the development gems and Ruby version, as well as fixing spec_helper if using Bundler as Rails was not included as a development dependency

Hope you like it!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.79%) when pulling 6a737e7 on wildtangent:regex-match-quotes into 909fea5 on kernow:master.

@kernow
Copy link
Owner

kernow commented Oct 14, 2014

Thanks for the PR. The original reason for using a string and allowing them to be configurable was to avoid matching mixed quotes, for instance:

[quote author="Jamie Dyer' title='King of England"]A quote[/quote]

Although likely invalid should be matched as :author => "Jamie Dyer' title='King of England" and not :author => "Jamie Dyer", :title => "King of England". Allowing mixed quotes would break this behaviour.

If an attribute value starts with a single quote then only a single quote should be matched to identify the end of a value and the same for double quotes. For example:

attribute="Hello World" # matches
attribute='Hello World' # matches
attribute="Hello World' # does not match
attribute='Hello World" # does not match

The other reason for using string matching is due to performance. I've not tested string vs regex but I would expect a regex would be slower than matching a string. You can test performance or string vs regex by running the commented out specs in https://github.com/kernow/shortcode/blob/master/spec/performance_spec.rb

I'd be happy to accept a PR for this so long as there is enforcement of matching quote pairs and there is no significant degradation of performance due to using a regex.

@wildtangent
Copy link
Author

Hi james

Thanks I hadn't considered that angle. Will take a look and see if it can
be achieved and do some benchmarking when I get some free time.

On Tuesday, 14 October 2014, Jamie Dyer notifications@github.com wrote:

Thanks for the PR. The original reason for using a string and allowing
them to be configurable was to avoid matching mixed quotes, for instance:

[quote author="Jamie Dyer' title='King of England"]A quote[/quote]

Although likely invalid should be matched as :author => "Jamie Dyer'
title='King of England" and not :author => "Jamie Dyer", :title => "King
of England". Allowing mixed quotes would break this behaviour.

If an attribute value starts with a single quote then only a single quote
should be matched to identify the end of a value and the same for double
quotes. For example:

attribute="Hello World" # matches
attribute='Hello World' # matches
attribute="Hello World' # does not match
attribute='Hello World" # does not match

The other reason for using string matching is due to performance. I've not
tested string vs regex but I would expect a regex would be slower than
matching a string. You can test performance or string vs regex by running
the commented out specs in
https://github.com/kernow/shortcode/blob/master/spec/performance_spec.rb

I'd be happy to accept a PR for this so long as there is enforcement of
matching quote pairs and there is no significant degradation of performance
due to using a regex.


Reply to this email directly or view it on GitHub
#16 (comment).

Joe Connor

http://factorymedia.com/

*w: *factorymedia.com
*a: *primeandfire.com

Factory Media UK
Bike Magic http://bikemagic.com/ | BMX Talk http://bmxtalk.com/ |
Cooler http://cooler.mpora.com/ | Dig BMX http://digbmx.mpora.com/ |
Dirt http://dirt.mpora.com/ | Kingpin http://kingpin.mpora.com/ | Moto
http://moto.mpora.com/ | MPORA http://mpora.com/ | MX Trax
http://mxtrax.co.uk/ | Onboard http://onboard.mpora.com/ | OutsideTimes
http://outsidetimes.com/ | RCUK http://roadcyclinguk.com/ | RideUK BMX
http://rideukbmx.mpora.com/ | Sidewalk http://sidewalk.mpora.com/ | Ski
Union http://skiunion.com/ | Surf Europe
http://surfeurope.mpora.com/ | Surf
Europe FR http://surfeuropemag.fr/ | Surfers Path
http://surferspath.mpora.com/ | Total Women's Cycling
http://totalwomenscycling.com/

Factory Media GmbH
Dirt De http://dirt.mpora.de/ | freedombmx http://freedombmx.mpora.de/
| Moto X http://motoxmag.mpora.de/ | MPORA De http://mpora.de/ | Monster
Skateboard Magazine http://skateboardmsm.mpora.de/ | Skiing
http://skiingmag.mpora.de/ | Snowboarder MBM
http://snowboardermbm.mpora.de/ | Surfers http://surfersmag.mpora.de/

@kernow kernow closed this Jan 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants