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

[Core] A little refactoring of loadcanvas.cpp #1956

Merged
merged 1 commit into from
Feb 5, 2021

Conversation

ice0
Copy link
Collaborator

@ice0 ice0 commented Dec 27, 2020

Based on top of this PR (#1955)

@@ -2284,6 +2284,13 @@ CanvasParser::parse_static_list(xmlpp::Element *element,Canvas::Handle canvas)
return value_node;
}

bool read_bool_attribute(xmlpp::Element* element, const char* name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

  • prepend static

  • Maybe rename to is_bool_attribute_true() or evaluate_bool_attribute()
    When I read if (read_bool_attribute) I have the impression I'm checking if it could read the attribute, not exactly evaluating its value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have the impression I'm checking if it could read the attribute

This is the correct impression, because it tries to read the attribute and returns false if it cannot read it (for example, the attribute was not found).

Maybe rename to is_bool_attribute_true()

Agree. This is more correct.

@ice0 ice0 force-pushed the loadcanvs-cleanup branch 2 times, most recently from 5bb2376 to 4b97318 Compare February 5, 2021 05:02
@ice0 ice0 force-pushed the loadcanvs-cleanup branch from 4b97318 to ada51af Compare February 5, 2021 08:20
@ice0 ice0 merged commit 4789a5b into synfig:master Feb 5, 2021
@ice0 ice0 deleted the loadcanvs-cleanup branch February 5, 2021 16:57
rodolforg pushed a commit to rodolforg/synfig that referenced this pull request May 26, 2021
rodolforg pushed a commit to rodolforg/synfig that referenced this pull request May 26, 2021
rodolforg pushed a commit to rodolforg/synfig that referenced this pull request May 27, 2021
morevnaproject pushed a commit to morevnaproject/synfig that referenced this pull request Aug 9, 2021
morevnaproject pushed a commit to morevnaproject/synfig that referenced this pull request Aug 9, 2021
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