-
Notifications
You must be signed in to change notification settings - Fork 52
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
Conversion of OOXML to CiceroMark and vice versa #251
Conversation
Signed-off-by: Aman Sharma <[email protected]>
58f46d9
to
328abad
Compare
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.
Why is this a different package from docx
?
Oh, I just thought it was supposed to be a different package. One reason I can think of is that |
I think it would make sense for all transforms to work on input data in memory and not depend on the file system? Maybe I'm missing something there. Also, it would help to figure out if there are any differences between ooxml or docx before creating a whole new package. @DianaLease @irmerk @dselman help here please? |
I think making this |
@dselman But we will only be able to transform OOXML from Word Documents to CiceroMark. Other OOXML (for eg. from PowerPoint, Excel, etc.) will fail as it makes no sense. |
Sounds like an argument for renaming the current package, not creating a new one? Also, maybe I'm wrong but isn't a docx containing several xml files? (some of which may or may not be ooxml?). |
@jeromesimeon OOXML file is just a zipped collection of all the XML files the Word contains as written here. |
After discussing on a call, I think it would likely make the most sense to keep the code in This will get rid of |
Signed-off-by: Aman Sharma <[email protected]>
@algomaster99 I think we wanted to keep it |
@irmerk cool, I will try to integrate it there. Once I will do that, I will close this PR. |
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.
One thought I had going through the code:
getId(elements) { | ||
const variableProperties = elements[0] | ||
for (const property of variableProperties.elements) { | ||
if (property.name === 'w:tag') { | ||
return property.attributes['w:val']; | ||
} | ||
} | ||
} |
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.
Is there a case where getId()
will not return anything, and you'd want to have a base case after the for loop?
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.
@irmerk I think every variable will have a corresponding ID so it ought to return something. And I am only calling this function when I parse w:sdt
which are node types for content controls.
Issue #247
Add package for converting
OOXML
<->CiceroMark
Changes