boazsegev
commented
7 years ago @oskarpearson ,
Thank you very much for opening this issue! šš»
I noticed it was a very small difference in recursion protection. I think I was super stingy when I set the value to 3... 8 or 12 should guard comfortably against a stack overflow issues.
Your suggestion of creating a modifiable variable in api.rb sounds great!šš»
I'm a bit busy this week, but I'll try to get to it during this weekend. If you would like to submit a PR, that would be fantastic!
Again, thank you very much for noticing and opening this issue šš» Bo.
P.S.
The Xerox story had me ššš
Hi
Firstly, thanks for the great Gem - and thanks for any time you might spend on this issue.
We've encountered a variant of https://github.com/boazsegev/combine_pdf/issues/109 - which is "fixed" if we increase the number of layers that equal_layers recursively examines.
Our change is at https://github.com/BloomAndWild/combine_pdf/commit/05b2ca141fe69040a962a871fa6eb8f159bfaba7
I'm happy to submit a PR - but I wanted to check if you thought it was the best solution?
Overall this still seems non-optimal, since I expect that there will always be edge cases for the comparison. It reminds me a bit of http://www.dkriesel.com/en/blog/2013/0802_xerox-workcentres_are_switching_written_numbers_when_scanning
If this variable is going to need to change a lot, it may make sense to make the number more easily modifiable. Currently it's a parameter of a private method, which makes it difficult to change without a private fork/PR.
Would you prefer to make the variable adaptable/changeable via a modification to api.rb Or perhaps an environment variable? I'd prefer the former, if anything.