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

Force Categories to be Strings #767

Merged
merged 2 commits into from Jan 26, 2013

Conversation

Projects
None yet
5 participants
@parkr
Member

parkr commented Jan 18, 2013

This PR fixes #759, which describes an error that is produced when a category is a number (and thus read in as a Fixnum). The error is caused by CGI.escape, which calls gsub on the input. Unfortunately, Fixnum#gsub doesn't exist, and so Jekyll throws this error:

/Users/parkermoore/code/jekyll/lib/jekyll/post.rb:157:in `block in url'
/Users/parkermoore/code/jekyll/lib/jekyll/post.rb:157:in `map'
/Users/parkermoore/code/jekyll/lib/jekyll/post.rb:157:in `url'
...

This fix and accompanying test ensures that all categories being passed to CGI.escape are instances of String, and are then added to the URL properly.

@tombell

This comment has been minimized.

Contributor

tombell commented Jan 20, 2013

I tried running the tests on 1.8.7-p371 to try and work out why it failed on Travis. I couldn't even run the tests until I removed the SimpleCov stuff in the test helper. Then when they did run they all passed.

@mattr-

This comment has been minimized.

Member

mattr- commented Jan 20, 2013

I think the real question is whether or not we want to support 1.8.7
anymore or not, because if we don't, we might as well remove it from our
Travis set up.

@parkr

This comment has been minimized.

Member

parkr commented Jan 20, 2013

I think if we can swing it, we should keep up support for 1.8.7. I only see one failing test, and it's a stack error. I thought I fixed it in 6a7a030 (it made the failing tests pass at the time) but it appears that it did not solve the problem in the long term.

Does this have anything to do with the RCov pull request which was accepted recently? Could it be a Travis problem?

parkr added a commit that referenced this pull request Jan 26, 2013

@parkr parkr merged commit 409eeda into master Jan 26, 2013

1 check failed

default The Travis build failed
Details

parkr added a commit that referenced this pull request Jan 26, 2013

@parkr parkr deleted the string-categories branch Jan 26, 2013

@jiguang

This comment has been minimized.

jiguang commented on cd05f6b Feb 26, 2013

nice job! fixed my problems, thank you!

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

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