Deal with smartTag and links with multiple runs #20

Merged
merged 6 commits into from Mar 22, 2013

Projects

None yet

2 participants

Member
jlward commented Mar 21, 2013

No description provided.

@jlward jlward was assigned Mar 21, 2013
@winhamwr winhamwr commented on the diff Mar 22, 2013
docx2html/core.py
@@ -749,6 +755,8 @@ def get_relationship_info(tree, media, image_sizes):
continue
# Store the target in the result dict.
target = el.get('Target')
+ if any(target.lower().endswith(ext) for ext in ['emf', 'wmf', 'svg']):
winhamwr
winhamwr Mar 22, 2013 Owner

The update note should mention we're now ignoring these images and this should be a constant that's declared and commented as far as why they're ignored.

winhamwr
winhamwr Mar 22, 2013 Owner

Need a test that these are ignored.

@winhamwr winhamwr commented on the diff Mar 22, 2013
docx2html/core.py
@@ -848,7 +856,9 @@ def image_handler(image_id, relationship_dict):
image_sizes
)
styles_dict = get_style_dict(styles_xml)
- font_sizes_dict = get_font_sizes_dict(document_xml, styles_dict)
+ font_sizes_dict = defaultdict(int)
+ if DETECT_FONT_SIZE:
winhamwr
winhamwr Mar 22, 2013 Owner

Why did we start using this flag again?

jlward
jlward Mar 22, 2013 Member

What do you mean again, we have always been using this flag. I added this as a bit of an optimization since the font_size stuff is not used anywhere.

winhamwr
winhamwr Mar 22, 2013 Owner

Looking at the diff, it was added with this pull request. Do you know why?

jlward
jlward Mar 22, 2013 Member

We are using DETECT_FONT_SIZE in other places as well. I only added it here because we are not actually using the font_sizes_dict so why take the overhead of populating it.

winhamwr
winhamwr Mar 22, 2013 Owner

Gotcha. Just wanted to make sure there was a reason, since font size stuff wasn't mentioned anywhere in the spec or update note.

@winhamwr winhamwr commented on an outdated diff Mar 22, 2013
docx2html/core.py
@@ -1211,8 +1222,12 @@ def get_p_data(p, meta_data, is_td=False):
# Once we have the hyperlink_id then we need to replace the
# hyperlink tag with its child run tag.
winhamwr
winhamwr Mar 22, 2013 Owner

tag => tags

@winhamwr winhamwr commented on the diff Mar 22, 2013
docx2html/tests/document_builder.py
@@ -3,16 +3,18 @@
templates = {
'drawing': 'drawing.xml',
'hyperlink': 'hyperlink.xml',
+ 'insert': 'insert.xml',
winhamwr
winhamwr Mar 22, 2013 Owner

What is this?

jlward
jlward Mar 22, 2013 Member

This would be the ins tag which was never tested (acts the same as a smartTag

winhamwr
winhamwr Mar 22, 2013 Owner

Let's put that in the changelog entry, then.

jlward
jlward Mar 22, 2013 Member

There is no point, it been supported for quite awhile, I am only now testing it.

winhamwr
winhamwr Mar 22, 2013 Owner

Gotcha. That makes sense, thanks.

Member
jlward commented Mar 22, 2013

Dealt with @winhamwr review notes, merging this into master

@jlward jlward merged commit 907206b into master Mar 22, 2013

1 check passed

default The Travis build passed
Details
@winhamwr winhamwr commented on the diff Mar 22, 2013
docx2html/core.py
@@ -1210,9 +1221,13 @@ def get_p_data(p, meta_data, is_td=False):
hyperlink_id = el.get('%sid' % r_namespace)
# Once we have the hyperlink_id then we need to replace the
- # hyperlink tag with its child run tag.
- child_run_tag = el.find('%sr' % w_namespace)
- if child_run_tag is None:
+ # hyperlink tags with its child run tag.
winhamwr
winhamwr Mar 22, 2013 Owner

Read this again. We're getting child run tags (multiple)

@winhamwr winhamwr commented on the diff Mar 22, 2013
docx2html/tests/test_xml.py
@@ -307,6 +308,51 @@ def side_effect(*args, **kwargs):
''')
+class SkipImageTestCase(_TranslationTestCase):
+ relationship_dict = {
+ #'rId0': 'media/image1.svg',
winhamwr
winhamwr Mar 22, 2013 Owner

Why are these commented out?

@jlward jlward added a commit that referenced this pull request Mar 14, 2014
@jlward jlward refs #20: added tests for smart tag (and insert since it wasnt tested…
…) and links with multiple runs
8820e80
@jlward jlward added a commit that referenced this pull request Mar 14, 2014
@jlward jlward refs #20: correctly handled smart tags and hyperlinks with multiple r…
…un tags. In addition did some code cleanup.
4b8cdf1
@jlward jlward added a commit that referenced this pull request Mar 14, 2014
@jlward jlward refs #20: update note b46135b
@jlward jlward added a commit that referenced this pull request Mar 14, 2014
@jlward jlward refs #20: fixed a comment a529a87
@jlward jlward added a commit that referenced this pull request Mar 14, 2014
@jlward jlward refs #20: updated update note 6fa5205
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment