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

Multiple improvements across multiple forks #49

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

Multiple improvements across multiple forks #49

wants to merge 28 commits into from

Conversation

Hessesian
Copy link

I have tracked several improvements that went into multiple forks and added some of my own, including:

  • added newline in the in memory buffer
  • fixed migration issues across new versions of libraries
  • added new sources of input to read from

I understand it's a lot of changes that are not fully coherent, and can't vouch for all of them, but I'm open to cherry picks or suggestions

badicsalex and others added 28 commits August 20, 2022 13:16
@badicsalex
Cargo.toml: customize crate metadata
f652dbf
@badicsalex
Cargo.toml: upgrade to 2021 edition
2175aaf
@badicsalex
Use log::{warn, info} instead of printlns
eb6c393
@fisherdarling @badicsalex
Project: run cargo fmt
071dd6b
@fisherdarling @badicsalex
do not panic on unknown char_decode
3d0d448
@badicsalex
lib.rs: Add allow(dead_code) for some currently unused struct fields
1d05148 
Instaed of deleting them, because they may become useful in the future
@badicsalex
lib.rs: Fixed clippy issues
433f31a 
Mostly trivial changes, but unfortunately there's no way to know what I
broke, because there are no tests.
@badicsalex
Cargo.toml: use num_parser with lopdf
053587f
@badicsalex
glyphnames: use the phf library instead of binary search
400ed4b
@badicsalex
lib.rs: republish lopdf::Document
6f65918 
This way a depending project does not need to directly depend on lopdf.
@badicsalex
lib.rs: Publish PdfFont, and give it to OutputDev
3e415a6
@badicsalex
PdfFont: don't panic! when get_width is called on an unknown code
7f260b3
@badicsalex
PdfFont: export get_basefont
11ec1b4
@badicsalex
PdfSimpleFont.new: mark unicode mismatch as dlog
da89f72 
It seems like a silly workaround with no real consequences anyway.
@badicsalex
make_colorspace: patch over the Separation-type colorspace handling
c2785e5 
See jrmuizel#38
@badicsalex
make_colorspace: don't panic on unknown color spaces
3c4509d
@badicsalex
examples: fix crate name in extract example
a0f934c
@badicsalex
Fix colorspace in SC command handling.
eda8dce 
See jrmuizel#39
@badicsalex
project: remove github CI integration
8e1eeae
@badicsalex
Pass CID to OutputDev.show_text implementers
24043ab 
See jrmuizel#41
@badicsalex
lib: don't panic on unknown smask entries
5d40745
@QPixel
fix(lib): apply patch for smask
00ea73f
@QPixel
Merge https://github.com/badicsalex/pdf-extract-fhl
2dd7ecf
@QPixel
chore: update package name and version
cb9723b
@QPixel
fix: bump lopdf version
88f51d5
@Hessesian
fix build errros
8634a7b
@Hessesian
add newlines to plain text
16e2a0e
@Hessesian
add loading file from other sources (mem, read)
1077a88
jrmuizel
jrmuizel reviewed Feb 12, 2023
View reviewed changes
src/lib.rs
}
}

fn as_num(o: &Object) -> f64 {
match *o {
Object::Integer(i) => i as f64,
Object::Real(f) => f,
Object::Real(f) => f.into(),
Copy link
Owner

Choose a reason for hiding this comment

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

What's the build error being fixed here?

Copy link
Author

Choose a reason for hiding this comment

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

f is of type f32, but signature wants f64, so this will convert into it

@joepio
Copy link
Contributor

joepio commented Feb 12, 2023 •
edited
Loading

@Hessesian since we both opened a PR in the last 24 hours with a lot of changes, and we're both keen on working on this project, maybe we should collaborate and try to merge our changes.

Stuff we both do:

  • We both seem to have run rustfmt and clippy, so many changes will overlap
  • Re-export lopdf
  • Fix some of the panics

Stuff only your PR changes:

Stuff only my PR changes:

  • Error handling (this is the biggest change by far). I have made many changes to lib.rs, including a lot of function signatures. That was a time-consuming task. As far as I can see, it looks like most changes from your PR to lib.rs are linting related. Is that right? If that's the case, I think it makes sense to take my lib.rs as a staring point.
  • Tests
  • Readme

@Hessesian
Copy link
Author

Sure, seems to be wise to create separate pull request first for the formatting changes and common issues resolved, make sure it works and then we can do separate PR's for individual features that can be tweaked.

Can you maybe go ahead with your changes that don't clash ?

@joepio
Copy link
Contributor

joepio commented Feb 13, 2023

@Hessesian Is possible, but I'd prefer it if my PR was merged and we than cherry pick commits from yours. I'd gladly do that, but I want to know if the changes are OK to merge. @jrmuizel what is your opinion on this?

Might be useful to have a short video meet about all of this if you want. Reach me at [email protected]

@joepio joepio mentioned this pull request Feb 25, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrmuizel jrmuizel jrmuizel left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Hessesian @joepio @jrmuizel @badicsalex @fisherdarling @QPixel