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

Allow to pass parser config object #30

Closed
wants to merge 1 commit into from

Conversation

ElGigi
Copy link

@ElGigi ElGigi commented Feb 19, 2024

Allow to pass SmalotPDF parser config object to the Reader service.

Regards.

@benito103e
Copy link
Member

Hello,
I like this, but shouldn't we be able to pass the \Smalot\PdfParser\Parser as a parameter to the Reader constructor?
This would allow the Parser configuration to be fully adjusted upstream.
Thanks in advance for your opinion.

@ElGigi
Copy link
Author

ElGigi commented Feb 22, 2024

Hello, I like this, but shouldn't we be able to pass the \Smalot\PdfParser\Parser as a parameter to the Reader constructor? This would allow the Parser configuration to be fully adjusted upstream. Thanks in advance for your opinion.

I wanted to take this approach, but the Parser object stores objects in memory without necessarily releasing them after parsing. This would therefore not have been optimal.

@benito103e
Copy link
Member

benito103e commented Feb 22, 2024

You're right !

What do you think about adding 2 properties with getter/setter :

protected array $smalotPdfParserCfg = [];
protected ?\Smalot\PdfParser\Config $smalotPdfParserConfig = null;

See #31

benito103e pushed a commit that referenced this pull request Feb 22, 2024
benito103e pushed a commit that referenced this pull request Feb 22, 2024
@ElGigi
Copy link
Author

ElGigi commented Feb 26, 2024

It's nice too 👍

@benito103e
Copy link
Member

Solved with #31

@benito103e benito103e closed this Feb 26, 2024
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