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

Improve check for paths as Hashes in CaptureOptions #582

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

stevenday
Copy link

@stevenday stevenday commented Sep 6, 2018

Hi,

Thanks for the awesome project!

This is my first PR and my Ruby is a little bit rusty, so please excuse any idiosyncrasies! I'm also running windows locally so I couldn't get the whole dev environment up and running to test everything before pushing, but I'll fix up whatever Travis finds.

Without knowing a whole lot about how this was supposed to be working, hopefully I've captured the intent of the code a bit better and fixed the issue reported in #536 at the same time.

I went for options.fetch so that it raised a KeyError, because I figured it'd make it more obvious that you're supposed to provide a path key if your path is a hash, rather than silently returning nil.

Fixes #536 and #466

@stevenday
Copy link
Author

I just noticed that a related bug was present in the GalleryGenerator too, so I've added a fix for that in 0c39b03.

The code duplication suggests that the GalleryGenerator should use the CaptureOptions to parse the options, but I didn't want to make such a big change here just to fix this small bug.

To match the changes to CaptureOptions.

For bbc#536
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.

Wraith fails on any URL containing "path", eg "/some-path"
1 participant