search

vandeseer / easytable

Small table drawing library built upon Apache PDFBox
MIT License
246 stars 94 forks source link

Better wordbreaking #79

Closed AndreKoepke closed 4 years ago

AndreKoepke commented 4 years ago

Hi vandeseer, I have again a little goodie for your project. :)

AndreKoepke commented 4 years ago

I hope this is the correct version.

vandeseer commented 4 years ago

Thanks for your pull request :+1:

I didn't yet look very closely (the new code seems more compact which is always nice) but there are two tests failing:

The second one looks like a float comparison issue (the test could be faulty as well!), but we need to check that before I can merge.

Also there are regressions in the following tests:

Would you mind having a look?

Best, Stefan

AndreKoepke commented 4 years ago

Damn. I will check that tomorrow. :+1:

AndreKoepke commented 4 years ago

So, Iam done. There was a issue in my code and the other one was (as you said) a float-error in the test-case. Both fixed now. :sunglasses:
The other ones seem good. See here: https://next.akop.online/s/qqBsTcpbRifPRd5

Only one question: If the cell is to small for a single character, then it will be overwritten. It's not better to throw an exception instead?

vandeseer commented 4 years ago

I realized there is still one question I didn't say anything about: Isn't it better to throw an exception in case a cell is too small for a single character?

Yes, that would make sense. We can alter that still in a separate change.

Also I want to add functionality to compare the output PDFs in the integration tests with some pre-rendered images. As of now one has to check them visually one by one which is obviously error prone.

vandeseer commented 4 years ago

Hi @AndreKoepke,

there's still an issue here. Didn't see that the first time unfortunately. Please have a look again at the "excelLike.pdf" file: In the last line on the cell on the right, the line should not be justified and what is of more importance: the second to last cell in the middle has a strange line breaking – just compare this with the image in the README file.

I am currently working on an automatic comparison of the generated PDF files so that such differences will be detected early on and not only by having a look manually ... As soon as I'm done I will put all the generated files from the latest version before this change under version control and we will hopefully see that there's a change in the rendered PDF documents.

Best, Stefan

vandeseer commented 4 years ago

As of now I won't revert the pull request, because I hope we'll find the bug soon ... :wink: