bump cupertino_icons to 1.08

[LongCatIsLooong]

And fix the incorrect codepoint for square_pencil_fill

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• I followed the breaking change policy and added Data Driven Fixes where supported.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[christopherfujino]

RSLGTM. @LongCatIsLooong do you have a reference link for why changing 0xf417 -> 0xf418 is the correct change?

[LongCatIsLooong]

codepoint change: https://github.com/xster/framework7-icons/pull/2/files#diff-8d861fabe46d688163d4afa524349547c56cea98546a41addd7c4f5c21eece72R55

[LongCatIsLooong]

There're new assertions in the script that makes sure that there are no codepoint conflicts so 0xf418 is not occupied by another symbol.

[LongCatIsLooong]

RSLGTM. @LongCatIsLooong do you have a reference link for why changing 0xf417 -> 0xf418 is the correct change?

Oh I should have added that, thanks! Added a github comment: #146806 (comment)

[LongCatIsLooong]

I don't have discord access right now. Can someone help me with contacting @test-exemption-reviewer? The codepoint change was in fact auto generated:.

[christopherfujino]

I don't have discord access right now. Can someone help me with contacting @test-exemption-reviewer? The codepoint change was in fact auto generated:.

I requested an exemption in hackers, but there was a question.

[andrewkolos]

Hello from PR triage. If my archaeological process was correct, this was was the conversation on discord. I will quote from it for those who do not access to Discord.

@christopherfujino: requestion (sic.) a test exemption for Mr. Long cat for #146806 (comment)

@Hixie: would it have caught this error?

@christopherfujino: it's unclear to me what the error was, cc @LongCatIsLooong and @xster

As far as I can tell, having a test for this would have theoretically caught this issue, but wouldn't such a test be no more than a copy-paste of a map of icon names to unicode code points? I hope my understanding is correct here. Assuming that understanding is correct, then—at the risk of asking a question so general that is pointless—should Flutter really have tests that directly verify correctness of its dependencies? All opinions welcome.

[Hixie]

I don't really have the context here to fully understand the issue that this fixes. Could you elaborate on what happened here?

For example, did we attempt to roll and then something failed so we knew we had to update the codepoint? Or did we roll, and in the dependency's documentation it warned that there was a codepoint change? Or did we roll and discover coincidentally that our use of the codepoint was wrong all along and we needed to change it? Or something else?

[LongCatIsLooong]

The issue is create_solid and create are currently mapped to the same codepoint when they are supposed to be different icons (https://main-api.flutter.dev/flutter/cupertino/CupertinoIcons/create-constant.html). The incorrect mapping was discovered because the script that generates the font was updated, so basically "discover coincidentally that our use of the codepoint was wrong all along and we needed to change it".

The .dart file that binds the codepoints to dart names is also generated by that script (https://github.com/xster/framework7-icons/pull/2/files#diff-1348eeb39757d4b57ff814747b5b00c2a1eca69059ae7592754c91691058b4b3), and with the updated script it would throw an exception if 2 different svgs get imported into the same glyph.

[Hixie]

Did we catch it when updating the script? Or did we not try to rerun the script after updating it, and then caught it later?

[LongCatIsLooong]

I did not realize there was a codepoint conflict until I ran the updated script to generate the ttf (and it threw an error), so the latter I think.

[Hixie]

In that case is it possible to write a test that runs the script and double-checks that the output matches what is in the tree?

[LongCatIsLooong]

In that case is it possible to write a test that runs the script and double-checks that the output matches what is in the tree?

Do you mean adding a test to flutter/flutter? The script lives in https://github.com/xster/framework7-icons/ so I fear that test won't be hermetic, and AFAIK there's no way of mapping the pub version of the package to the commit hash in the repo that contains the script, so I don't know which version of the script I should test against.

It does sound weird that the icon font is provided by the cupertino_icons package while the dart bindings are provided by the cupertino package. Is it possible to move the CupertinoIcons class from flutter/flutter to the cupertio_icons package? Then the codepoint mapping can be tested using FontLoader in golden tests.

[LongCatIsLooong]

I'll remove the codepoint change since that will break apps that pinned cupertino_icons to older versions (the icon isn't right but with this it would show the notdef tofu character). Filed #148075.

[christopherfujino]

LGTM