Skip to content

Conversation

rmja
Copy link

@rmja rmja commented Feb 15, 2022

This PR adds support for System.Text.Json.Nodes.JsonNode as model.
It is similar to the JsonElement implementation, but allows e.g. for case insensitive lookup as described in Handlebars-Net/Handlebars.Net#491 (comment)

@rmja
Copy link
Author

rmja commented Feb 16, 2022

Maybe someone capable can add the .NET 6 sdk to the github workflow?

@oformaniuk
Copy link
Member

oformaniuk commented Feb 17, 2022

Hello @rmja

Maybe someone capable can add the .NET 6 sdk to the github workflow?

Thanks for the PR! I will add .NET 6 to the workflow later this week.

@rmja
Copy link
Author

rmja commented Jun 14, 2022

Has anyone had a chance to look at this?

@rmja
Copy link
Author

rmja commented Sep 12, 2022

@zjklee Have you had a chance to go through this?

@rmja
Copy link
Author

rmja commented Jul 12, 2023

ping...

Copy link
Contributor

@thompson-tomo thompson-tomo left a comment

Choose a reason for hiding this comment

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

Should be reviewed after #10 has been merged as will likely need conflicts to be resolved.

Comment on lines 20 to 25
Handlebars.Create(new HandlebarsConfiguration().UseJson())
(Handlebars.Create(new HandlebarsConfiguration().UseJson()), json => JsonDocument.Parse(json)),
#if NET6_0_OR_GREATER
(Handlebars.Create(new HandlebarsConfiguration().UseJson()), json => System.Text.Json.Nodes.JsonNode.Parse(json)),
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this still be needed if we bumped system.text.json to v6?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bumping STJ package to v6 will eliminate the need for this conditional code.

@@ -0,0 +1,31 @@
#if NET6_0_OR_GREATER
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed if STJ is bumped to v6?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bumping STJ package to v6 will eliminate the need for this conditional code.

@@ -0,0 +1,172 @@
#if NET6_0_OR_GREATER
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed if STJ is bumped to v6?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bumping STJ package to v6 will eliminate the need for this conditional code.

@@ -0,0 +1,47 @@
#if NET6_0_OR_GREATER
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed if STJ is bumped to v6?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bumping STJ package to v6 will eliminate the need for this conditional code.

@@ -0,0 +1,44 @@
#if NET6_0_OR_GREATER
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed if STJ is bumped to v6?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bumping STJ package to v6 will eliminate the need for this conditional code.

@@ -0,0 +1,48 @@
#if NET6_0_OR_GREATER
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed if STJ is bumped to v6?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bumping STJ package to v6 will eliminate the need for this conditional code.

Copy link

sonarqubecloud bot commented Apr 7, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@thompson-tomo
Copy link
Contributor

Merge conflicts exist but should wait for #10 to be merged first.

@thompson-tomo
Copy link
Contributor

thompson-tomo commented Apr 8, 2024

@rmja please see rmja#1 as I have just submitted some proposed changes to improve maintainability & consistency.

@rmja
Copy link
Author

rmja commented Apr 9, 2024

@thompson-tomo merged:)

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.

3 participants