[XamlC] compile ListStringTypeConverter #660

Merged
merged 2 commits into from Jan 18, 2017

Conversation

Projects
None yet
3 participants
@StephaneDelcroix
Member

StephaneDelcroix commented Dec 22, 2016

Description of Change

compile the ListStringTypeConverter, used in Style and the new OnPlatform

Behavioral Changes

slightly longer IL, but no reflection calls, and less method calls

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
+ yield return Instruction.Create(OpCodes.Ldnull);
+ yield break;
+ }
+ var parts = value.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries).Select(s => s.Trim()).ToList();

This comment has been minimized.

@samhouts

samhouts Dec 28, 2016

Member

I wonder if there is a scenario where users would want a whitespace entry to occupy space in a list of strings. Perhaps providing labels for a chart where they want to skip every other item, for example?

@samhouts

samhouts Dec 28, 2016

Member

I wonder if there is a scenario where users would want a whitespace entry to occupy space in a list of strings. Perhaps providing labels for a chart where they want to skip every other item, for example?

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Jan 9, 2017

Member

All scenarios are possible... the ListStringTypeConverter (the new compiled version, and the old one) are only used for VisualElement.StyleClass where it is ok to RemoveEmptyEntries, but it's not required. And it is also aligned with the non-compiled converter

@StephaneDelcroix

StephaneDelcroix Jan 9, 2017

Member

All scenarios are possible... the ListStringTypeConverter (the new compiled version, and the old one) are only used for VisualElement.StyleClass where it is ok to RemoveEmptyEntries, but it's not required. And it is also aligned with the non-compiled converter

@@ -4,7 +4,8 @@
x:Class="Xamarin.Forms.Xaml.UnitTests.CompiledTypeConverter"
RectangleP="0,1,2,4"
RectangleBP="4,8,16,32"
- BackgroundColor="Pink">
+ BackgroundColor="Pink"
+ List="Foo, Bar">

This comment has been minimized.

@samhouts

samhouts Dec 28, 2016

Member

If we're explicitly removing whitespace entries, should we test for that?

@samhouts

samhouts Dec 28, 2016

Member

If we're explicitly removing whitespace entries, should we test for that?

StephaneDelcroix added some commits Dec 22, 2016

@StephaneDelcroix StephaneDelcroix merged commit 3701720 into master Jan 18, 2017

6 checks passed

Android-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests passed: 351, i…
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: Windows Debug : Tests passed: 3697, ignored: 10
Details
iOS10-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests passed: 34…
Details
iOS8-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8 : Tests passed: 345…
Details
iOS9-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests passed: 347…
Details

@StephaneDelcroix StephaneDelcroix deleted the xamlc_compiledListConverter branch Jan 31, 2017

@samhouts samhouts added D-15.4 and removed cla-not-required labels Oct 10, 2017

@samhouts samhouts modified the milestones: 2.3.0, 2.3.5 Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment