Navigation Menu

Skip to content
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

Content type matching problem with LinkedIn #35

Merged
merged 1 commit into from Nov 10, 2014

Conversation

ratcristian
Copy link
Contributor

I've recently encountered a problem using the LinkedIn configuration for this plugin. The rule for matching the content type should look like this:

m!^(application/json|text/javascript)(;\s*charset=\S+)?$!

because in the LinkedIn case there is no space between the content type and the charset definition. Also maybe you should include an else statement for this rule in case is not matched.

I've recently encountered a problem using the LinkedIn configuration for this plugin. The rule for matching the content type should look like this:

m!^(application/json|text/javascript)(;\s*charset=\S+)?$!

because in the LinkedIn case there is no space between the content type and the charset definition. Also maybe you should include an else statement for this rule in case is not matched.
@akarelas
Copy link
Contributor

How about we allow overriding the regex in each provider's configuration hashref? So that we don't "contaminate" the main regex too much.

@marcusramberg
Copy link
Owner

Supporting overrides in the provider hashref seems like the best solution
to me as well.

Marcus

On Mon Nov 10 2014 at 3:40:37 PM Alexander Karelas notifications@github.com
wrote:

How about we allow overriding the regex in each provider's configuration
hashref? So that we don't "contaminate" the main regex too much.


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

@marghidanu
Copy link

But the solution proposed by @ratcristian should work just fine. It's actually completing the original expression.

@marcusramberg
Copy link
Owner

After thinking it through, I'm leaning towards considering this a bugfix of the original regex, as @marghidanu suggests. I guess we can also add support for overriding it later if anyone has a valid use case for that.

marcusramberg added a commit that referenced this pull request Nov 10, 2014
Content type matching problem with LinkedIn
@marcusramberg marcusramberg merged commit 0dabbfc into marcusramberg:master Nov 10, 2014
@marghidanu
Copy link

Will this result in a CPAN release?

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.

None yet

4 participants