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

Support docx files without footnotes #3

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

Conversation

brent-p
Copy link

@brent-p brent-p commented Dec 4, 2016

Firstly thanks for this awesome gem!!
I ran into a small issue on some of the docx files I tried to parse where they didn't have any footnotes. So I have added some small checks to prevent runtime errors when trying to parse these files.

Side note:
I am not sure if this should be a separate pull request because I don't know a lot about ruby conventions but I had some small issues trying to get this gem to work and solved them by adding the following to my gemfile:

gem 'rubyzip', '< 1.0.0'
gem 'publishr',
gem 'docx_converter', '~> 1.0'

The newer rubyzip no longer supports this line in lib/docx_converter.rb:
require 'zip/zipfilesystem'
So setting it to require an older version fixed this issue.

The publisher gem is required in lib/docx_converter.rb but the publisher gem is not mentioned in the gemspec file, should this be added to the gemspec? or is it expected that users should add this manually?

When I tried adding just gem 'docx_converter' to my side project, bundler complained that it couldn't find the gem docx_converter but changing it to use a specific version as mentioned above fixed that problem so I am not sure if this should be updated in the readme.

Thanks again for this gem and please go easy on me as this is my very first pull request :)
Brent

@Jack12816
Copy link

I can confirm both issues. (footnotes, installation)
We should also take care of the rmagick requirement. (lib/docx_converter)

[DEPRECATION] requiring "RMagick" is deprecated. Use "rmagick" instead

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.

2 participants