"Sort by key" causes the value type to change

[yang-chengye]

Hi,

When I use the "Sort by key" function, after sorting, it converts the number type to a string type.
See "age" in the example below.

Before sorting
{ "name": "Ycy", "age": 27, "is_student": false }

After sorting
{ "age": "27", "is_student": false, "name": "Ycy" }

My plugin version is 2.1.1.0 and npp version is 8.8.5
Looking forward to your reply, thx.

[AntonDevil]

I investigated the issue a little bit.

Here is the test for it (can be added to JsonHandlerTest.cpp):

    TEST_F(JsonHandlerTest, TestSortJsonByKey_NumberValue_Success)
    {
        std::string inputJson = R"({"b": "valueB", "a": 30})";
        auto        result    = m_jsonHandler.SortJsonByKey(inputJson, {}, {}, ' ', 4);

        ASSERT_TRUE(result.success);
        ASSERT_EQ(result.response, "{\n    \"a\": 30,\n    \"b\": \"valueB\"\n}");
    }

Indeed, it won't pass, because it will output {"a": "30", "b": "valueB"}

The root cause is the line JsonHandler.h:29
constexpr auto flgBaseReader = rj::kParseEscapedApostropheFlag | rj::kParseNanAndInfFlag | rj::kParseNumbersAsStringsFlag;
Note rj::kParseNumbersAsStringsFlag
When JsonHandler.cpp:108 SortJsonText read json it treats numbers as string (as an attempt to preserve whatever number there was even if it's too long to be parsed precisely) and outputs it as string.

I'm not a maintainer of this repo (just joined) and I didn't figure out a good and simple solution yet.

[AntonDevil]

There was an attempt to support this:
RawNumber() inconsistent about quotes
But there were some problem about it Write RawNumber without quotes

Since RapidJSON dependency come from a fork by @SinghRajenM (main maintainer)
I can suggest to fix this in this fork and keep a list of patches applied to keep track of this for future syncs with the main RapidJSON repo

[SinghRajenM]

@AntonDevil can you raise PR for this?
RapidJson is forked and part of this org now: https://github.com/NPP-JSONViewer/rapidjson

[AntonDevil]

Hi @SinghRajenM

First, I need to merge this:
NPP-JSONViewer/rapidjson#1

[SinghRajenM]

One QQ: does it retain the precision?

[AntonDevil]

Inherently it does.

The numbers are still read as strings (not interpreted, thus keeping the original text), but I also set a flag to say it was a raw number, so on output we can output it as RawNumber (instead of string).

Do you have any test to try it?

[SinghRajenM]

Currently, precision and format are not retained in RapidJson. There are few tests, FormatJson_numbers is one of them.

I've merged the NPP-JSONViewer/rapidjson#1

[AntonDevil]

I added the test for precision to the PR for rapidjson.

But on the plugin side, there is another issue with precision.
When it writes the json back (after sorting), it uses rj::kParseFullPrecisionFlag so it lose precision

[AntonDevil]

@SinghRajenM if you like, you can merge followup with more tests:
NPP-JSONViewer/rapidjson#2

[SinghRajenM]

Rather I will add these to Plugin instead of RapidJason

[AntonDevil]

@SinghRajenM

1. Tests

My plan was to add to both (why not? more test the better =)),
These are two separate projects,
but it's your call.

2. Parse flags

BTW, why flgBaseWriter uses rj::kParseFullPrecisionFlag
instead of rj::kParseNumbersAsStringsFlag as in flgBaseReader?

JSON-Viewer/src/NppJsonViewer/JsonHandler.h

Lines 29 to 30 in 095bf27

	constexpr auto flgBaseReader = rj::kParseEscapedApostropheFlag | rj::kParseNanAndInfFlag | rj::kParseNumbersAsStringsFlag;
	constexpr auto flgBaseWriter = rj::kParseEscapedApostropheFlag | rj::kParseNanAndInfFlag | rj::kParseFullPrecisionFlag;

I guess this is exactly because of this issue of writing RawNumbers that I just fixed?
It used to convert raw numbers to strings, but now it won't

If so, I can unify both and there will be no precision loss.

[AntonDevil]

@SinghRajenM

#240 fixes the subject issue

#241 fixes the rest like FormatJson_numbers you mentioned (built on top of the first PR)

we taught rapidJSON to work with raw numbers properly =)

It also fixes a number of other issues related to precision accumulated through the years

[SinghRajenM]

Fixed in #240