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

Escape non ascii-characters while image downlading from wordpress #531

Merged
merged 2 commits into from
Dec 5, 2023

Conversation

ahmetozer
Copy link
Contributor

While importing one of the Turkish websites, I noticed ruby HTTP library was not able to download because of the non-ASCII characters.

So I used escape while download process.

An example command

ruby -r rubygems -e 'require "jekyll-import";JekyllImport::Importers::WordpressDotCom.run({"source" => "wordpress.xml","no_fetch_images" => false, "assets_folder" => "assets" })'

@mattr-
Copy link
Member

mattr- commented Dec 1, 2023

Thanks for contributing! ❤️

Could you address the comment from the code style check action and update your PR? I'll be happy to merge it after that.

Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to see a test but I know this project doesn't have great test coverage so approving anyway.

@ahmetozer
Copy link
Contributor Author

Hello @parkr and @mattr- thank you for reviewing, do I need to do anything?
I am not familiar with this programming language, but if it's a must for this repo I can take a look at the test part.
Thanks 😊

@parkr
Copy link
Member

parkr commented Dec 5, 2023

@jekyllbot: merge +fix

@jekyllbot jekyllbot merged commit 3f03bdb into jekyll:master Dec 5, 2023
6 checks passed
jekyllbot added a commit that referenced this pull request Dec 5, 2023
github-actions bot pushed a commit that referenced this pull request Dec 5, 2023
Ahmet ÖZER: Escape non ascii-characters while image downlading from wordpress (#531)

Merge pull request 531
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants