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

#561 Added optional getText() argument to return limited number of document pages if set #562

Merged
merged 7 commits into from
Dec 16, 2022
Merged

#561 Added optional getText() argument to return limited number of document pages if set #562

merged 7 commits into from
Dec 16, 2022

Conversation

alesrebec
Copy link
Contributor

Solving #561
Tested locally and working fine.

k00ni
k00ni requested changes Nov 25, 2022
View reviewed changes
Copy link
Collaborator

@k00ni k00ni left a comment

Choose a reason for hiding this comment

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

@alesrebec thank you for this nice addition.

Please provide at least one test which covers your code.

src/Smalot/PdfParser/Document.php Outdated Show resolved Hide resolved
src/Smalot/PdfParser/Document.php Outdated Show resolved Hide resolved
@k00ni k00ni linked an issue Nov 28, 2022 that may be closed by this pull request
Only get limited number of pages from PDF #561
Closed
@k00ni k00ni self-assigned this Dec 13, 2022
@k00ni
Copy link
Collaborator

k00ni commented Dec 13, 2022

@alesrebec I took the liberty and finalized the PR. Any remarks?

I will merge it in a couple of days if there are no objections.

@alesrebec
Copy link
Contributor Author

@alesrebec I took the liberty and finalized the PR. Any remarks?

I will merge it in a couple of days if there are no objections.

Thanks for finalizing the PR.
I had some issues running the tests locally when I was about to write the tests and it just slipped from my todo list.
All good with the merge.

@k00ni
Copy link
Collaborator

k00ni commented Dec 13, 2022

I had some issues running the tests locally when I was about to write the tests

Which issues did you have? Maybe we can improve our tooling.

@alesrebec
Copy link
Contributor Author

It was on my end but I'll give it another go.

@k00ni k00ni merged commit d430fe6 into smalot:master Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@k00ni k00ni k00ni approved these changes

Assignees

@k00ni k00ni

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Only get limited number of pages from PDF
2 participants
@alesrebec @k00ni