Don't write unnecessary auto render value to SVG output (PR #26620)
Blake-Madden
I'll get this fix in separately while I work on SVG unit tests.
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26620
Commit Summary
- 7921792 Don't write unnecessary auto render value to SVG output
File Changes
(1 file)
- M src/common/dcsvg.cpp (3)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26620.patch
- https://github.com/wxWidgets/wxWidgets/pull/26620.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26620)Note that this will still result in an extra space (due to "%s " being present in the format string). Not sure if it's worth fixing, but it would be tidier if we avoided it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 1 commit.
- 88418a3 Remove extra spaces from output SVG
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
Blake-Madden left a comment (wxWidgets/wxWidgets#26620)Note that this will still result in an extra space (due to
"%s "being present in the format string). Not sure if it's worth fixing, but it would be tidier if we avoided it.
Fixed
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
@vadz approved this pull request.
Thanks, this is difficult to review and I'm not sure if any changes here would be detected by our tests, but assuming you tested it and there are no missing spaces anywhere, it looks good and I'll merge it.
I do wonder if we could use some better/safer way of building strings with SVG attributes than wxString::Format("%s"), e.g. something like AttributeBuilder::Add("name", possibleEmptyValue).Add(...).ToString(). But we would definitely want to have tests detecting breakage first if we ever decide to change this code like this.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.