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.

Sign up for GitHub

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

Jump to bottom

Update prawn-svg to 0.36.0 #2553

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

Update prawn-svg to 0.36.0 #2553

wants to merge 2 commits into from

Conversation

Fethbita
Copy link

@Fethbita Fethbita commented Jan 7, 2025 •
edited
Loading

Fixes #2551. Updated prawn-svg to 0.36.0.

One failing test was:

rspec ./spec/image_spec.rb:1164 # Asciidoctor::PDF::Converter - Image SVG should render linear gradient in SVG

But visually there isn't a difference. I added the regenerated file (image-svg-with-gradient.pdf) from under spec/output. The /Creator field was set to <feff> but I changed it to be the same value as the /Producer field.

The changes in the error values are expected, see prawn-svg 0.36.0 release notes:

Note that there has been a considerable change to the way properties are handled internally. There are no changes in the PDF files generated by the test suite, but if your SVG uses invalid property values then it's likely prawn-svg will treat it differently (hopefully more like a web browser does!) Please raise an issue if this version shows any regression for your specific use case.

Also the change in the gradient handling is expected, see prawn-svg 0.36.0 release notes:

The highlight change for this release is a huge rewrite of how gradients work to fix long-standing issues in the implementation as well as adding previously-unsupported properties

@Fethbita Fethbita changed the title Update prawn-svg to 0.36.0 Update prawn-svg to 0.36.0 (fixes #2551) Jan 7, 2025
@Fethbita Fethbita changed the title Update prawn-svg to 0.36.0 (fixes #2551) Update prawn-svg to 0.36.0 Jan 7, 2025
@mojavelinux
Copy link
Member

I'm curious why the error message changed. I'm not sure I understand why that was part of adding more complete gradient support. At least before the user had information about what was wrong with the SVG. I'm not opposed to accepting that the message is more generic, but it does concern me enough to want to follow up on it.

@Fethbita
Copy link
Author

Fethbita commented Jan 7, 2025

Created this issue to ask about this mogest/prawn-svg#178

@mojavelinux
Copy link
Member

Thanks! It looks like that got resolved quickly. I suppose we're just waiting on another release now?

@Fethbita
Copy link
Author

Fethbita commented Jan 8, 2025

I'm not sure when the next release will be for prawn-svg and the timeline for asciidoctor-pdf release. So it is up to you to merge this or wait.

I would be happy if this would get merged before the next asciidoctor-pdf release even if prawn-svg is not released. Once prawn-svg is released with the error message fix, it can get updated easily.

@mojavelinux
Copy link
Member

We cannot merge with a change to the error message, especially given we know it's been fixed upstream. Merging before then would just create unnecessary changes.

@Fethbita Fethbita mentioned this pull request Jan 10, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

prawn-svg v0.36.0 dependency update
2 participants
@Fethbita @mojavelinux