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

add basic style definition #138

Merged
merged 4 commits into from
Oct 15, 2023
Merged

add basic style definition #138

merged 4 commits into from
Oct 15, 2023

Conversation

miko53
Copy link

@miko53 miko53 commented Aug 2, 2023

Hello,

I see that style is not given for paragraph, If you want I propose a minimalist implementation of style.
With it you can have the name of the style by paragraph.

The main API is style(), called for a paragraph it returns it style name. As the style are stored in document, I have added an API to access to document in paragraph class.

Can you review please ? Any suggestions are welcome !

Regards,

Copy link
Member

@satoryu satoryu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for opening this pull request.

I put one request change as a comment here.

Comment on lines 213 to 215
p = Elements::Containers::Paragraph.new(p_node, document_properties)
p.document = self
p
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#document= looks much open: anybody can replace another Document instance with this accessor of the paragraph returned.
how about giving Document object to Paragraph's constructor like Element::Containers::Paragraph.new(p_node, document_properties, self) ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it's ok, my initial idea was to avoid modifying previous API.
I modify branch to take into account.


describe 'reading style' do
before do
@doc = Docx::Document.open(@fixtures_path + '/test_with_style.docx')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add one indent

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done also

@miko53 miko53 requested a review from satoryu October 14, 2023 13:45
@satoryu satoryu merged commit b928484 into ruby-docx:master Oct 15, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants