Skip to content
This repository has been archived by the owner on Sep 6, 2018. It is now read-only.

Fix rounding error for color hex value #19

Merged
merged 5 commits into from
May 22, 2017
Merged

Fix rounding error for color hex value #19

merged 5 commits into from
May 22, 2017

Conversation

yusuke024
Copy link

Without passing the colorFloat * 0xff to the round() function first, SwiftGen will incorrectly parse 0xf8f8f8 color into 0xf7f7f7.

Copy link
Member

@djbe djbe left a comment

Choose a reason for hiding this comment

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

Hi @Yusuke-Onishi,

Thank you for this PR. Funny that we never noticed this.

Please add a changelog entry mentioning this change, PR, and crediting yourself. Please base yourself on the format of other changelog entries, and take care to add two spaces after the . at the end of the description.

It might be best to add an extra set of test cases for our color hex string parser, to test your specific case + some other ones. Could you write it, or do you need some help with it? (best to put it in a separate test file such as ColorParserTests.swift)

@yusuke024
Copy link
Author

Hi @djbe , sorry for the delay.
I've updated the log file but I'm not sure how to test this one.
Since NSColor.hexValue is marked as fileprivate and @testable doesn't seem to work, I think I can only test it via ColorsCLRFileParser which basically can be done by adding more entries to the Resources/Fixtures/Colors/colors.clr file.
If you have any suggestions please let me know.

Also, when I tried to run rake it gave the below error (I have Xcode 8.3.2). I had to modify the script to make it works.

== Compile using Xcode ==
rake aborted!

[!!!] You need to have Xcode 8.x to compile SwiftGen.

/Users/st20966/Developer/SwiftGenKit/rakelib/utils.rake:73:in `version_select'
/Users/st20966/Developer/SwiftGenKit/rakelib/utils.rake:13:in `block in run'
/Users/st20966/Developer/SwiftGenKit/rakelib/utils.rake:12:in `map'
/Users/st20966/Developer/SwiftGenKit/rakelib/utils.rake:12:in `run'
/Users/st20966/Developer/SwiftGenKit/rakelib/xcode.rake:10:in `block (2 levels) in <top (required)>'
/Users/st20966/.brew/lib/ruby/gems/2.4.0/gems/rake-12.0.0/exe/rake:27:in `<top (required)>'
Tasks: TOP => default => xcode:test => xcode:build
(See full trace by running task with --trace)```

@djbe
Copy link
Member

djbe commented May 2, 2017

Really quick comment (will look at the testing question later), you can fix that error by doing this:
SwiftGen/StencilSwiftKit#32 (comment)

@djbe
Copy link
Member

djbe commented May 2, 2017

You can mark the method as internal. We just need tests for a couple of values, for example #000000, #FFFFFF, your color and maybe some more.

@djbe djbe added this to the 2.0.0 milestone May 8, 2017
@djbe djbe merged commit b8ce637 into SwiftGen:master May 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants