-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Feature: Paragraph.add_hyperlink() (wip) #278
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
Feature: Paragraph.add_hyperlink() (wip) #278
Conversation
Hi again @scanny Could you please me some feekback on this PR ? I wait for your GO to attack the implementation part. :) |
5804492
to
9b71461
Compare
Hello @scanny , Thank you very much for the explanation in detail, it really helps :). I understand more now. I am totally agree with you that we should escape special chars in For <w:history>, I think you are right. In this document of Microsoft, it says
So the value can be 0/1/True/False. As you have mentioned, <w:anchor> is a bookmark name with '#' prepended, either for the document itself (no <r:id>) or for an external uri (with <r:id>). If neither <r:id> and the bookmark is present, the <w:anchor> is just a harmless bookmark name, e.g., as a link '#anchor_value'. |
@scanny Thank you again. |
@tanyunshi Thanks for letting me know about the commits, no worries, just let me know when you're ready :) On the If Word doesn't mind I expect we wouldn't either :) But if it fails to load such a document we'll need to do due diligence to avoid allowing the user to create such a document. Does it make sense? |
@scanny @tanyunshi I was wondering if this is getting merged with the main branch and if so when would we be looking at seeing it? Thanks for all the awesome work! |
Hey @rushton3179 I am always on the feature. I am a bit busy untils mid-may. I will catch up the dev after mi-may. :") |
Hey @tanyunshi, no worries if you would ever like a hand implementing it let me know. I wrote a function that inserts hyperlinks and put it in the original feature request thread if its of any help too :) |
@tanyunshi Great work! Will you be able to finalize this PR? Where should we start if you want help? |
@spohner - I think you can pick this up more or less from the beginning of implementation. The analysis document (in the first commit above) is fairly well developed, so should provide the background needed. @tanyunshi also provided a "spiked" implementation, so I'm sure that's useful background as well, but to proceed with a merge we'll need tests, and it will need to be in test-driven commits. The first step would be developing the acceptance test(s) in ( These two are pretty good examples: Merges maintain a strictly test-driven discipline; it's the only way to keep the library robust while keeping maintenance/committer effort within doable levels :) The basic idea is:
Let me know with any questions once you've had a chance to look through :) |
@scanny I am currently up to implementing a hyperlink style and adding bookmarks in for anchors. I am having a little trouble as when I zip and unzip word files on my mac, word does not like them and says they are unreadable. In styles.xml I am attempting to add
Do you have any tips for ensuring the modified default document is correct? I believe this should work as when using the hack I initially wrote when I manually set the rStyle value to 'Hyperlink' it would work as intended when modifying a document that didn't come from the default template. Also I cannot seem to find any current implementation of inserting bookmarks into runs or paragraphs. The method of generating functions automatically has been a little confusing though so I may be missing something and thought I'd ask. Other than that the basic feature tests I have written are working and I am just trying to fine tune it. Once I have those done I plan to do some more extensive testing. Edit: I have gotten the bookmarks working properly thanks to @rev112, combing over his code made me realise I had a small mistake. Thanks! |
I think I'll need a little more to go on to fully understand what you're trying to do, but in general you shouldn't have to work with zip files at all. That's all abstracted out by the A Word file as a whole is known as a package. It can be saved as a zip archive, which is what a .docx file is, but can also be stored in other ways, such as in a database and so on. Each individual "file" within the zip archive is known as a part. This is the language you'll see reflected in the The The styles part is a subclass of Pretty much every kind of operation on the XML has been done somewhere in the library already, and can serve as an example. If you explain more specifically what you're trying to accomplish I can probably guide you to those examples. If all you want to do is add a few styles to the document, there's already API for that. Check out this page in the documentation: http://python-docx.readthedocs.io/en/latest/api/style.html#styles-objects |
@scanny Awesome, that should get me through what I need and I will hopefully have a PR ready in the next couple of days. When it comes to testing, so far I have come up with most scenarios I can think of in the acceptance testing and I have noticed you comment before that unit testing is used in python-docx when acceptance test cases fail. If this is the case is there anything I should write unit test for just to ensure everything is as needed? Cheers! |
The general flow is you write an acceptance test before any code. It should fail, of course, because the code that implements it isn't there. You mark the acceptance test with the @wip decorator so the test suite can skip these and therefore still pass. This acceptance test gets committed as a separate commit with message starting with 'acpt:...'. Where the acceptance test fails determines what the first unit test and code is written. Each commit is atomic, generally meaning one unit test and just the code it takes to make it pass. After a few commits, the acceptance test passes, the @wip decorator is removed from it, and that's one cycle. This pattern is clearly visible in the commit history, which you should study a bit to get the flow. The one for If you don't get this right you end up rewriting stuff or putting me to unnecessary effort (time) when it comes to merging. |
Gotchya, while I have actually done this all in one commit on my branch (but using a similar workflow) I can set up a new branch and step through it like this so you get something that is easy to review and has all the necessary tests. It might be a week or so until I get it done but it shouldn't be too far off as it is all currently working. |
Hi @tanyunshi, just wanted to let you know that your analysis doc made it into release v1.0.0 going out in the next few days. Right now that's in this commit: 4935c30 But the SHA may change across rebases and force-pushes as it wends its way to release probably later this week. In any case, wanted to thank you for that work and let you know it had been incorporated :) There's no |
The PR answers to the feature #74
paragraph.add_hyperlink
-> implementation wip
paragraph.iter_run_level_items()