Skip to content
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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

LillianJensen
Copy link

No description provided.

@LillianJensen
Copy link
Author

LillianJensen commented Mar 18, 2022

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:

import pptx

ppt = pptx.Presentation()
ppt_slide = ppt.slides.add_slide(ppt.slide_layouts[6]) # blank Slide
t = ppt_slide.shapes.add_textbox(0, 0, 100, 100)
p = t.text_frame.paragraphs[0]

r = p.add_run()
r.text = "Test"

p = t.text_frame.add_paragraph()
f = p.add_field()
f.text = "1"
f.type = "slidenum"

with open("test.pptx", 'wb') as f:
    ppt.save(f)

The field properly updates when slides are added, or moved.

@LillianJensen LillianJensen force-pushed the fields branch 2 times, most recently from c200350 to b5630e3 Compare March 18, 2022 17:39
Copy link

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.

Copy link
Author

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)
Copy link

@ajkalb ajkalb Feb 26, 2024

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.

Copy link
Author

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)
Copy link

@ajkalb ajkalb Feb 26, 2024

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.

Copy link
Author

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

Copy link
Author

@LillianJensen LillianJensen left a 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)
Copy link
Author

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)
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants