Add support for textbox -> txbxContent #203

jlward · 2016-05-11T19:56:20Z

      <w:r>
            <w:pict>
                <v:textbox>
                  <w:txbxContent>
                    <w:p>
                      <w:r>
                        <w:t>Foo bar baz</w:t>
                      </w:r>
                    </w:p>
                  </w:txbxContent>
                </v:textbox>
              </v:rect>
            </w:pict>
      </w:r>

…This will prevent quite a few circular imports.

…need to figure that out.

jlward · 2016-05-11T19:57:32Z

pydocx/openxml/markup_compatibility/alternate_content.py

+
+class AlternateContent(XmlModel):
+    XML_TAG = 'AlternateContent'
+    children = XmlCollection(Fallback)


I might make a pass and update all the XmlCollection stuff that I touched to use strings instead of classes.

jlward · 2016-05-11T20:32:42Z

pydocx/openxml/wordprocessing/textbox.py

+    )
+
+
+class Textbox(XmlModel):


This guy should really live in vml instead of wordprocessing :( I'll handle this after the code review.

kylegibson · 2016-05-12T14:47:23Z

pydocx/models.py

+    def types(self):
+        return set(self._set_types(*self._types))
+
+    def _set_types(self, *types):


Do we want to consider caching this result? Would be expensive to have to run it more than once, if stuff is needing to be imported.

I had considered it. Do you recall if pydocx has a built in caching tool? Or should I do it the old fashioned way?

A thought. This gets called for each instance. I might need to make types be a class method and cache it there.

I don't think a class method would actually fix this.

https://github.com/CenterForOpenScience/pydocx/blob/master/pydocx/util/memoize.py

My research leads me to believe that no caching is needed (it won't actually change anything).

Could you clarify what you researched, Jason?

I printed the types in this method and could only get that printing the first time it was used in a test. The second test did not show any of the printing.

jlward · 2016-05-12T16:36:01Z

tests/export/html/test_markup_compatibility.py

+        '''
+        self.assert_document_generates_html(document, expected_html)
+
+    def test_textbox_with_content_outside_of_textbox(self):


Continuation of https://github.com/CenterForOpenScience/pydocx/pull/203/files#r63035777 I have the tests written. However I don't have any idea what I need to do to break this up into multiple paragraphs. From what I can tell, the outside opening p tag has already happened. Which means I'll need to close it, do my thing, then re-open it. If this is the case, I have no idea how it can be done inline. I could probably build a post-processor, but it looks like we don't have those anymore.

We're doing something like this elsewhere in the code. I'm not certain where.

I would almost rather push this live as is, and deal with it later. But that's mostly because I don't have a good solution in mind.

For ComplexFields, we had to implement a two-pass approach: https://github.com/CenterForOpenScience/pydocx/blob/master/pydocx/export/base.py#L115

What happens in the editor if you get nested paragraph tags like this?

CKEditor actually fixes it for us.

AAABBB --> AAABBB

The legacy editor actually also fixes this auto-magically.

Having an editor that fixes it for us is good, but other users of pydocx might not have that luxury. Our goal is to output clean HTML, right? I'm +0 on fixing this in a separate ticket, before #204 or as part of #204, if you think that would be a better way to structure the work.

I don't have a good way of fixing it. Honestly, I was going to punt on it, get #204 done, then not worry about it until someone finds a good way to fix it.

kylegibson · 2016-05-12T18:58:23Z

pydocx/openxml/markup_compatibility/fallback.py

+
+class Fallback(XmlModel):
+    XML_TAG = 'Fallback'
+    children = XmlCollection('wordprocessing.Picture')


Do you know what other child types Fallback supports?

I will be dealing with that in #204 as I mentioned during stand up :)

I don't plan to push a new release until both this and 203 are done.

Can you add this comment?
#TODO in #204- actually include all of the children defined by the spec

kylegibson · 2016-05-12T19:01:42Z

Do you have any test cases for when textbox appears without the markup_compatibility? This would happen if you had a word document created in an older version of word that didn't support the markup compatibility syntax.

jlward · 2016-05-12T19:02:57Z

Do you have any test cases for when textbox appears without the markup_compatibility? This would happen if you had a word document created in an older version of word that didn't support the markup compatibility syntax.

I do not. I only have it with markup_compatibility because that was the only way I could get it to happen using libreoffice.

kylegibson · 2016-05-12T19:03:19Z

I only have it with markup_compatibility because that was the only way I could get it to happen using libreoffice.

What version of libreoffice?

jlward · 2016-05-12T19:06:07Z

What version of libreoffice?

5.1. And I don't have access to 4.x

kylegibson · 2016-05-12T19:10:20Z

5.1. And I don't have access to 4.x

Our vagrant boxes ship with 3.5. Although, ideally we would create the documents using an older version of Word. I wish we had access to Word VMs like we do with IE VMs.

jlward · 2016-05-12T19:40:52Z

Our vagrant boxes ship with 3.5. Although, ideally we would create the documents using an older version of Word. I wish we had access to Word VMs like we do with IE VMs.

Using 3.5 and saving as a docx actually removes the contents of the docx. So there's that.

winhamwr · 2016-05-13T16:28:56Z

pydocx/export/base.py

@@ -15,7 +15,7 @@
    NumberingSpan,
    NumberingSpanBuilder,
 )
-from pydocx.openxml import wordprocessing, vml
+from pydocx.openxml import wordprocessing, vml, markup_compatibility


alphabetize?

This reverts commit fed6498.

winhamwr · 2016-05-13T18:06:34Z

pydocx/export/base.py

+    def export_textbox_content(self, textbox_content):
+        return self.yield_nested(textbox_content.children, self.export_node)
+
+    # Markup Compatibility exporters


Seems like we no longer need this comment.

winhamwr · 2016-05-13T18:58:27Z

Everything looks good now except for the paragraph nesting thing. It would be good to include Kyle in that decision, since I feel like I don't have a great understanding of what it would take to implement a fix.

winhamwr · 2016-05-13T21:15:41Z

I'll wait to review the description of the ticket to fix the nested paragraph before giving this one the ol' 👍

jlward · 2016-05-16T16:59:23Z

I'll wait to review the description of the ticket to fix the nested paragraph before giving this one the ol' 👍

#213 has been created to deal with nested paragraphs.

caffodian · 2016-05-16T18:45:40Z

@kylegibson has reviewed #213 so this is good

Jason Ward added 7 commits May 11, 2016 15:02

refs #203: Made it possible to pass along dotted string path to tag. …

340f67e

…This will prevent quite a few circular imports.

refs #203: Added new test files.

d0b2cb6

refs #203: Alphabetize all the things.

368e957

refs #203: Added the new test to the runner.

6015680

refs #203: Added the new tags needed to deal with textboxes

47b0a2e

refs #203: "Fixed" the test. I am getting nested paragraphs, so I'll …

d2b373c

…need to figure that out.

refs #203: Get the rest of the tests to pass.

2937c06

jlward reviewed May 11, 2016
View reviewed changes

Jason Ward added 2 commits May 11, 2016 16:22

refs #203: Update to the new hotness on the touched files.

fed6498

refs #203: Update note.

563354c

jlward reviewed May 11, 2016
View reviewed changes

Jason Ward added 3 commits May 11, 2016 16:50

refs #203: Moved the definition of the textbox.

b80b088

refs #203: Update the references to Textbox

afb093d

refs #203: Renamed the textbox content file.

f735783

jlward mentioned this pull request May 12, 2016

Add support for colorized text #211

Merged

kylegibson reviewed May 12, 2016
View reviewed changes

Jason Ward added 2 commits May 12, 2016 12:32

refs #203: Removed a comma.

338a25b

refs #203: Added XML based tests.

7e5fc4b

jlward reviewed May 12, 2016
View reviewed changes

refs #203: Update test.

030b0d0

kylegibson reviewed May 12, 2016
View reviewed changes

Jason Ward added 2 commits May 12, 2016 15:45

refs #203: Added tests without markup compat.

1ce2887

Merge branch 'master' into issue_203

97438e5

winhamwr reviewed May 13, 2016
View reviewed changes

Jason Ward added 3 commits May 13, 2016 12:36

refs #203: name change.

6b738c1

Revert "refs #203: Update to the new hotness on the touched files."

5554636

This reverts commit fed6498.

refs #203: Added a comment.

4d12156

winhamwr reviewed May 13, 2016
View reviewed changes

refs #203: Removed a dead comment.

7e20b60

jlward mentioned this pull request May 16, 2016

Deal with nested paragraphs after #203 #213

Open

jlward merged commit e84d282 into master May 16, 2016

jlward mentioned this pull request May 16, 2016

Add support for the markup compatibility namespace #204

Merged

jlward deleted the issue_203 branch May 20, 2016 21:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for textbox -> txbxContent #203

Add support for textbox -> txbxContent #203

jlward commented May 11, 2016

jlward May 11, 2016

jlward May 11, 2016

kylegibson May 12, 2016

jlward May 12, 2016

jlward May 12, 2016

jlward May 12, 2016

kylegibson May 12, 2016

jlward May 12, 2016

winhamwr May 13, 2016

jlward May 13, 2016

jlward May 12, 2016

kylegibson May 12, 2016

jlward May 12, 2016

kylegibson May 12, 2016

kylegibson May 12, 2016

jlward May 12, 2016 •

edited

Loading

jlward May 12, 2016

winhamwr May 13, 2016

jlward May 13, 2016

kylegibson May 12, 2016

jlward May 12, 2016

winhamwr May 13, 2016

kylegibson commented May 12, 2016

jlward commented May 12, 2016

kylegibson commented May 12, 2016

jlward commented May 12, 2016

kylegibson commented May 12, 2016

jlward commented May 12, 2016

winhamwr May 13, 2016

winhamwr May 13, 2016

winhamwr commented May 13, 2016

winhamwr commented May 13, 2016

jlward commented May 16, 2016

caffodian commented May 16, 2016

Add support for textbox -> txbxContent #203

Add support for textbox -> txbxContent #203

Conversation

jlward commented May 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlward May 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kylegibson commented May 12, 2016

jlward commented May 12, 2016

kylegibson commented May 12, 2016

jlward commented May 12, 2016

kylegibson commented May 12, 2016

jlward commented May 12, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winhamwr commented May 13, 2016

winhamwr commented May 13, 2016

jlward commented May 16, 2016

caffodian commented May 16, 2016

jlward May 12, 2016 •

edited

Loading