Added the ability to clear the default empty paragraph in TableCell. #182
Conversation
Could not have conflicting variable name.
|
If you don't add an empty paragraph (any paragraph) to a cell - word won't open up. So adding empty paragraph when doing AddTable is the only way to make it functional. This also means you don't need to do AddParagraph() on given cell but simply directly work with it. Sure if you want multiple paragraphs in there via some loop, the logic gets a bit more complicated, but whether it makes sense to add optional parameter to remove existing things - I am not 100% sure. I am also not sure what you mean by PR is untested? We need tests, as otherwise things will be forgotten and sooner or later something will be broken. |
|
My application logic involves adding paragraph from a list into the word document, so I was trying to clear and immediately add. (Document/TabelCell).Paragraphs() Returns a list, but it's a new list, calling Clear on this list does nothing to the underlying paragraphs. I can for the head of the list apply everything to the first element and then add to the tail, but I assumed it would make more sense for this library to expose that feature. As far as the testing is concerned, I lack experience with dotnet and also have not even looked into whatever framework you're using for testing. I fully agree with you on tests being needed, this code however is simple, should not affect existing code, adding a single conditional statement (without an else to cover). Are you saying that I should close this PR and implement the seperate head/tail logic in the application? |
|
No, I'm saying I'm trying to understand and make some decisions. |
|
I think this should be rename the parameter to removeExistingParagraphs for it to be clear what will happen when using this parameter |
Updated the method of retrieving paragraphs to actually remove existing ones when specified in WordTableCell. Previously, the paragraphs collection might have been modified during enumeration, leading to unpredictable behavior. Also extended the AddParagraph method signature to include an optional parameter for conditional paragraph removal, enhancing the function's flexibility. This change ensures existing paragraphs can be cleared as expected before adding new text to a table cell.
|
I've fixed the logic, as it didn't account for more paragraphs being there, and I also added testing for this. Thankfully due to added testing I was able to find issue in the original code. Hopefully this gives you some tips & tricks on testing and next PR could include them. It's really beneficial to add it with testing even for those simple changes, as when i used some additional logic it failed right away when multiple paragraphs where in the list. |
|
Yeah thanks, I'm going to read over your commit with the testing now. |
Working with this library, I was having each cell being with an empty paragraph default styling. This pull request adds an optional parameter to the function for adding a paragraph, if left out or false the behavior will not change. When true the method will now remove every paragraph under the table cell. I had also considered making the parameter toggle deleting the prior paragraph only. This may be better, I decided against proposing that because a full reset is easier to think through in my mind.
The comment about mentioning in the PR ought to be removed, I have also documented the return value, parameter, and new parameter in the documentation comment.
This PR is untested.