-
Notifications
You must be signed in to change notification settings - Fork 540
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
Added a:fld type to paragraphs for page numbers and datetimes #797
base: master
Are you sure you want to change the base?
Conversation
I probably need to clean up a bit before this is ready to merge into the original repository (any feedback would be appreciated, the code is very dense, and there is a lot of indirection happening). Currently I have tested it and it does work with the following code:
The field properly updates when slides are added, or moved. |
c200350
to
b5630e3
Compare
pptx/oxml/__init__.py
Outdated
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.
It looks like these are redundant additions.
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.
I removed the redundant lines
@@ -324,8 +326,10 @@ class CT_TextField(BaseOxmlElement): | |||
<a:fld> field element, for either a slide number or date field | |||
""" | |||
|
|||
id = RequiredAttribute("id", XsdString) |
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.
id is a python built-in function. Might use a different name.
Although maybe that makes functionality less obvious to user.
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.
I recognize that we are shadowing the id built-in, but since we are inside of a class, and are unlikely to need the id function here, I think the readability makes it worth it. There is also some precedent in the codebase for this: the CT_SlideId class in pptx/oxml/presentation.py, the CT_Connection class in pptx/oxml/shapes/connector.py, and CT_NonVisualDrawingProps in pptx/oxml/shapes/shared.py all have attributes named id.
rPr = ZeroOrOne("a:rPr", successors=("a:pPr", "a:t")) | ||
t = ZeroOrOne("a:t", successors=()) | ||
type = OptionalAttribute("type", ST_FieldType) |
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.
type is a python built-in function. Might use a different name.
Although maybe that makes functionality less obvious to user.
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.
See comment above re: id. There is a precedent for this as well in the CT_Placeholder class in pptx/oxml/shapes/shared.py
Remove redundant lines
Remove redundant lines
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.
I removed the duplicate lines, I'm not sure how I missed that. I think the use of id and type are appropriate for readability, and there are precedents in the code for both.
@@ -324,8 +326,10 @@ class CT_TextField(BaseOxmlElement): | |||
<a:fld> field element, for either a slide number or date field | |||
""" | |||
|
|||
id = RequiredAttribute("id", XsdString) |
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.
I recognize that we are shadowing the id built-in, but since we are inside of a class, and are unlikely to need the id function here, I think the readability makes it worth it. There is also some precedent in the codebase for this: the CT_SlideId class in pptx/oxml/presentation.py, the CT_Connection class in pptx/oxml/shapes/connector.py, and CT_NonVisualDrawingProps in pptx/oxml/shapes/shared.py all have attributes named id.
rPr = ZeroOrOne("a:rPr", successors=("a:pPr", "a:t")) | ||
t = ZeroOrOne("a:t", successors=()) | ||
type = OptionalAttribute("type", ST_FieldType) |
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.
See comment above re: id. There is a precedent for this as well in the CT_Placeholder class in pptx/oxml/shapes/shared.py
No description provided.