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

[Work In Progress] Implement generic handling of method invocation #295

Conversation

david-garcia-garcia
Copy link
Contributor

No description provided.

@StefH
Copy link
Collaborator

StefH commented Aug 31, 2019

There are some merge conflicts. Can you fix these.?

@@ -5,7 +5,7 @@ namespace System.Reflection
/// <summary>
/// https://github.com/castleproject/Core/blob/netcore/src/Castle.Core/Compatibility/IntrospectionExtensions.cs
/// </summary>
internal static class CustomIntrospectionExtensions
public static class CustomIntrospectionExtensions
Copy link
Collaborator

Choose a reason for hiding this comment

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

why public?

using System;
using System.Linq.Expressions;

namespace ETG.SABENTISpro.Utils.DynamicLinkCore.Compatibility
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you merge master into your branch, you get this code. So this class can be removed.

@@ -10,6 +10,8 @@ namespace System.Linq.Dynamic.Core.CustomTypeProviders
/// </summary>
public abstract class AbstractDynamicLinqCustomTypeProvider
{

Copy link
Collaborator

Choose a reason for hiding this comment

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

remove empty lines

@@ -23,6 +23,17 @@ public string GetDisplayName(bool a, bool b, bool c)

class C : AbstractDynamicLinqCustomTypeProvider, IDynamicLinkCustomTypeProvider
{
/// <inheritdoc cref="IDynamicLinkCustomTypeProvider"/>
public List<Type> GetMethodIntrospectionTypes()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not define these hard-coded in the code and let the user add more if needed?

like public List<Type> AdditionalMethodIntrospectionTypes() ?


if (ce != null)
// TODO: Recode to handle N possible arguments
Copy link
Collaborator

Choose a reason for hiding this comment

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

For a quick solution, just add a private helper method which handles up to 10 arguments.

@@ -84,13 +123,20 @@ public virtual Expression Promote(Expression expr, Type type, bool exact, bool c
}
}
}

// Try to autopromote string to guid, because in Json and other serializations guid
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are several places where some exceptions are made for Guid's.

Maybe remove this code here, and make a new PR to fix/update the handling of guids?

@@ -253,14 +335,14 @@ public static bool IsNumericType(Type type)
return GetNumericTypeKind(type) != 0;
}

public static bool IsNullableType(Type type)
public static bool IsNullableType(this Type type)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please keep all methods in this class as normal methods? (Not extension methods.)

@StefH
Copy link
Collaborator

StefH commented Sep 1, 2019

@david-garcia-garcia Other question, maybe if you have time, can you also quickly look at this #286 ?

@StefH
Copy link
Collaborator

StefH commented Feb 12, 2021

Closing this PR for now since it's been some time since this was active.

@StefH StefH closed this Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants