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

Do not generate TextAlignment extension if it's already generated by base type #205

Merged
merged 3 commits into from
Apr 20, 2023

Conversation

Youssef1313
Copy link
Contributor

@Youssef1313 Youssef1313 commented Apr 3, 2023

Another suggested fix: #206

(ONLY ONE of the PRs need to be merged)

Description of Change

Linked Issues

PR Checklist

Additional information

Copy link
Contributor Author

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

new Derived().M();

public class Base
{
}

public class Derived : Base
{
}

public static class Extensions1
{
    public static void M<T>(this T b) where T : Base { }
}

public static class Extensions2
{
    public static void M<T>(this T b) where T : Derived { }
}

@pictos Currently, we generate two generic extensions like these. The fact that they are generic is why the compiler actually complains.

If we generate it like this:

new Derived().M();

public class Base
{
}

public class Derived : Base
{
}

public static class Extensions1
{
    public static void M(this Base b) { }
}

public static class Extensions2
{
    public static void M(this Derived b) { }
}

It will compile just fine.

Is it possible that we switch to the non-generic approach?

@pictos
Copy link
Member

pictos commented Apr 3, 2023

@Youssef1313 here's some context around the usage of generics
#150

@Youssef1313 Youssef1313 closed this Apr 3, 2023
@Youssef1313 Youssef1313 deleted the text-alignment-fix branch April 3, 2023 19:51
@Youssef1313 Youssef1313 restored the text-alignment-fix branch April 3, 2023 19:51
@Youssef1313 Youssef1313 reopened this Apr 3, 2023
@Youssef1313
Copy link
Contributor Author

Ok, I think we can go with this approach.

Copy link
Collaborator

@brminnick brminnick left a comment

Choose a reason for hiding this comment

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

Thanks @Youssef1313!!

Apologies for not merging this sooner 🙌

@brminnick brminnick enabled auto-merge (squash) April 20, 2023 21:50
@brminnick brminnick merged commit c14dc32 into CommunityToolkit:main Apr 20, 2023
@Youssef1313 Youssef1313 deleted the text-alignment-fix branch May 26, 2023 08:01
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.

[BUG] TextAlignment Extension Throwing Compiler Error
3 participants