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

LitJSON serialize properties .ToLower() #2

Closed
RLittlesII opened this Issue Dec 2, 2015 · 7 comments

Comments

Projects
None yet
2 participants
@RLittlesII
Copy link
Contributor

RLittlesII commented Dec 2, 2015

Can the LitJSON serialize C# properties ToLower() ? I have a feature I'd like to submit, but can't get LitJSON to convert my C# object to the JSON array without making all my C# properties lower case.

If not are you okay with the potential switch to Newtonsoft? I have proved that I can get it working with that library.

Thanks!
--Rodney

@devlead

This comment has been minimized.

Copy link
Member

devlead commented Dec 2, 2015

Hi @RLittlesII,

I'm not entirely sure exactly what you're asking for here, but here goes 😄

You have described requirements on how you want to implement a feature, but not what that feature is, which makes it hard for me to give a definitive answer. I'm below assuming it's a feature you want to add to Cake.Slack

I've checked the code and doesn't look like LitJSON currently supports lower casing property names, one could probably add a option to the JsonWriter.WritePropertyName and ad an JsonMapperconstructor that allows you to supply your custom JsonWriter.

If changing from an internal serializer to an external 1.3MB nuget is a valid option totally depends on the feature you wan't do add. Currently Cake.Slack.dll debug release is 68KB including JSON.Net would increase that footprint, which can be perfectly valid if the value of the feature justifies it.
(an issue with using JSON.Net could be assembly versioning as it's a very commonly used assembly, we could work around by ilmerging and internalizing, this could potentially add complexity though)

If changing litjson core behavior is an good option also depends on the feature you like to add, could be perfectly viable.

So in short could you provide some insights on what you are trying to add to the Cake.Slack feature set? If so I'll happily provide any feedback I can about implementation details.

Regards,
/Mattias

@RLittlesII

This comment has been minimized.

Copy link
Contributor Author

RLittlesII commented Dec 2, 2015

Apologies. I am new to this.

I am implementing sending message attachments as a new parameter on Post. I am currently sending a SlackChatMessageAttachment[] in from the build.cake. It follows the Slack Api Documentation JSON.

The problem I am facing is when the internal PostToIncomingWebHook does the JsonMapper.ToJson() unless all my properties in the SlackChatMessageAttachment object are lowercase in my class library the JsonMapper call doesn't convert the property names to lower case. I believe it is because I am passing an entire object to a single property in the ToJson as opposed to building the property inline.

I don't want to increase the footprint. I was wondering if I missed something reading through LitJson. I used JSON.net just to prove that it could be done. I wanted to leverage more of the functionality of the Slack API to have more robust messages for my build output.

@devlead

This comment has been minimized.

Copy link
Member

devlead commented Dec 2, 2015

@RLittlesII first, sounds like an excellent feature to add to Cake.Slack.

I think I have an good idea on how to achieve this without major refactoring, I'll get back to you shortly,

@devlead

This comment has been minimized.

Copy link
Member

devlead commented Dec 2, 2015

Hi @RLittlesII,

I've now with #3 added possibility to optionally lower case property names while serializing to JSON, this should be available if you pull down the latest bits,

Example usage:

private static string ToJson(object obj)
{
    var jsonWriter = new JsonWriter {LowerCaseProperties = true};
    JsonMapper.ToJson(obj, jsonWriter);
    return jsonWriter.ToString();
}

Please take a look and test if this is enough to start working on your feature.

@RLittlesII

This comment has been minimized.

Copy link
Contributor Author

RLittlesII commented Dec 2, 2015

@devlead
I'll take a look when I get home and let you know what I find.

Thanks!

@devlead

This comment has been minimized.

Copy link
Member

devlead commented Dec 2, 2015

@RLittlesII superb 👍

@RLittlesII

This comment has been minimized.

Copy link
Contributor Author

RLittlesII commented Dec 3, 2015

That seems to have resolved my issues. Going to clean up the code and submit a PR tonight.

Thanks!!!

@RLittlesII RLittlesII closed this Dec 3, 2015

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