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

Fix panic by setting default_width to Some(1.0) #83

Closed
wants to merge 1 commit into from

Conversation

prscoelho
Copy link
Contributor

default_width is never changed from its initial value of None, so for pdfs that end up unwrapping this value, it always results in a panic. Therefore, assume a default_width of Some(1.0)

This change yields correct results for my pdf that was previously panicking.

default_width is never changed from its initial value of None,
so for pdfs that end up unwrapping this value, it always results
in a panic. Therefore, assume a default_width of 1.0.
@jrmuizel
Copy link
Owner

Do you have some reference for the choice of 1.0 as the default? Can you link to an example PDF that shows this problem?

@prscoelho
Copy link
Contributor Author

I have no reference for the value of 1.0 as default, it was just that the alternative was unwrapping a None value. It's entirely possible that the problem should be solved somewhere else in the code.

This pdf that I have is private, and it also relies on #82 (decrypting with empty password)

But I will see if I can edit this to remove the encryption and remove some private information. Is there some place I can send you this pdf privately?

@jrmuizel
Copy link
Owner

You can send it to [email protected]

@prscoelho
Copy link
Contributor Author

I sent you the pdf. Upon closer look I think this character is just broken :( Some(1.0) avoids a panic, but it's likely not the correct value..

@jrmuizel
Copy link
Owner

I pushed 7331339 which should fix this.

@jrmuizel jrmuizel closed this Feb 15, 2024
@prscoelho
Copy link
Contributor Author

It does indeed. Thank you!

@prscoelho prscoelho deleted the fix/default_width branch February 15, 2024 02:31
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