-
Notifications
You must be signed in to change notification settings - Fork 14
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
Basic table support (v2) #77
Conversation
html2docx/html2docx.py
Outdated
table = self.tables.pop()[0] | ||
section = self.doc.sections[0] | ||
page_width = section.page_width - section.left_margin - section.right_margin | ||
page_width = int(page_width * (0.5 ** len(self.tables))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please explain this line? I don't follow why the page width is multiplied by 0.5 ^ len(self.tables). Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't. I did not wrote this line, this is from a previous pull request (#55), I just took the existing code and worked on the tests which was the main feedback.
If I had to guess: this is an estimation of the width of the table which is smaller the more nested is the table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately the original reviewer is no longer with our organization. I can't approve this PR until I fully understand what this line does. If you like you can fork the repo and merge it in on your fork.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I understand. This line is originally to handle nested table which I think is fairly rare. If you agree, I will just remove the "nested" feature so that we can start with this simple case and then we can add more edge cases in the a future pull request?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that's a good idea
@kbattersby Do we need something else or we can merge this? 🙂 |
It's been approved and all checks passed, so you can merge it. |
@clement-escolano OK I did it |
Thank you |
@kbattersby Is it possible to publish a new version please? Right now, I must install the package from GitHub instead of pypi to use this feature. Thank you |
@kbattersby Sorry to insist, is it possible to publish a new version in the near future? Thank you |
Ok we will do that. Most likely I'll be able to release it within the next week or 2. |
@kbattersby Thank you, I see you upgraded the version in the repository but I don't see it in pypi. There is probably an extra step missing. |
Yes I still need to release it. I've had some other tasks on my plate but I plan to get back to this quite soon. |
@clement-escolano It should be released now. Let me know if you have any problems. Thanks! |
@kbattersby Thank you 🙏 |
Hello
Thank you for this library. I am also interested in table support and saw there was already an open pull request.
I took the first commit unchanged and changed the tests as you asked and also took into account some feedback.
What do you think?
Thank you