v.info: Add JSON output for history

[NishantBansal2003]

Fixes: #4217
Use parson to add json output format support to the v.info module for history flag.
The JSON output looks like as follows:

$ grass --exec v.info -h map=roadsmajor format=json
{
    "records": [
        {
            "command": "v.in.ogr dsn=\"majorroads.shp\" output=\"roads_major\" min_area=0.0001 snap=-1",
            "mapset_path": "\/bigdata\/grassdata05\/wakestpfeet\/PERMANENT",
            "user": "helena",
            "date": "Tue Nov  7 18:34:20 2006"
        },
        {
            "command": "v.proj input=\"wroads_major\" location=\"wakestpfeet\" output=\"roadsmajor_wake\"",
            "mapset_path": "\/bigdata\/grassdata05\/ncfromfile\/PERMANENT",
            "user": "helena",
            "date": "Wed Nov  8 00:18:50 2006"
        },
        {
            "command": "v.db.connect -o map=\"roadsmajor@PERMANENT\" driver=\"sqlite\" database=\"\/home\/neteler\/grassdata\/nc_spm_latest\/nc_spm_08\/PERMANENT\/sqlite\/sqlite.db\" table=\"roadsmajor\" key=\"cat\" layer=\"1\" separator=\"|\"",
            "mapset_path": "\/home\/neteler\/grassdata\/nc_spm_latest\/nc_spm_08\/PERMANENT",
            "user": "neteler",
            "date": "Mon Nov 26 16:55:38 2012"
        },
        {
            "command": "v.db.connect -o map=\"roadsmajor@PERMANENT\" driver=\"sqlite\" database=\"$GISDBASE\/$LOCATION_NAME\/$MAPSET\/sqlite\/sqlite.db\" table=\"roadsmajor\" key=\"cat\" layer=\"1\" separator=\"|\"",
            "mapset_path": "\/home\/neteler\/grassdata\/nc_spm_08_grass7\/PERMANENT",
            "user": "neteler",
            "date": "Fri Dec  7 23:25:12 2012"
        }
    ]
}

[NishantBansal2003]

Failing gunittest tests on the macOS build: FAILED ./scripts/g.extension/testsuite/test_addons_download.py (1 test failed). I am unsure why this test is failing.

Previously in the macOS CI, test_github_official_module_man_page_src_code_links_exists was failing.
Now, test_github_install is failing inside ./scripts/g.extension/testsuite/test_addons_download.py.

[neteler]

macOS: I have restarted this test.

And the OSGeo4W CI error is a bit unexpected:

checking whether to use PostgreSQL... "yes"
checking for location of PostgreSQL includes... /c/OSGeo4W/include
checking for libpq-fe.h... yes
checking for location of PostgreSQL library... /c/OSGeo4W/lib
checking for PQsetdbLogin in -lpq... no
checking for PQsetdbLogin in -lpq... no
checking for PQsetdbLogin in -lpq... no
checking for PQsetdbLogin in -lpq... no
configure: error: *** Unable to locate PostgreSQL library.

[nilason]

And the OSGeo4W CI error is a bit unexpected:

See #4990

[wenzeslaus]

This looks good overall and the generated JSON seems good too. I have just comments about code structure.

I'm still for the mapset_path but I would be happy to discuss alternatives if anyone feels like.

I completely agree that a function is needed. With how the code is written, two separate functions seem to be more fitting. One for actual parsing and another one for JSON. Let's say we would want to write to a different format than JSON. However, my reason for wanting two functions is mostly from code reading perspective.

[NishantBansal2003]

Hey @wenzeslaus, thanks for the detailed review. I've tried to improve the code structure based on your feedback and will keep it in mind for next time. Could you please take a look and check if everything is covered? Thanks.

[nilason]

After a quick reading, I have some minor suggestions.

[cwhite911]

Nice job @NishantBansal2003

[nilason]

I wonder, with the traditional output the history is ordered, this is not evident with current json output?!

Update: e.g. add a "history_no", or "event_no" or similarly named int to json output.

[NishantBansal2003]

I wonder, with the traditional output the history is ordered, this is not evident with current json output?!

The order of elements within a JSON object created by Parson may vary (i.e., they are not necessarily in the order in which we inserted them). However, the order of elements in a JSON list/array created by Parson remains the same (i.e., exactly in the order we inserted them) unless you remove elements. See: kgabis/parson#218

[nilason]

I wonder, with the traditional output the history is ordered, this is not evident with current json output?!

The order of elements within a JSON object created by Parson may vary (i.e., they are not necessarily in the order in which we inserted them). However, the order of elements in a JSON list/array created by Parson remains the same (i.e., exactly in the order we inserted them) unless you remove elements. See: kgabis/parson#218

That is what I mean. The order of history is important, but this information is lost working with a json file with an array of "records" (no predictable way to know the order there, is there). It is like a database table without a pkey.

Something like:

{
    "records": [
        {
            "history_no_or_whatever_name": "1",
            "command": "v.in.ogr dsn=\"majorroads.shp\" output=\"roads_major\" min_area=0.0001 snap=-1",
            "mapset_path": "\/bigdata\/grassdata05\/wakestpfeet\/PERMANENT",
            "user": "helena",
            "date": "Tue Nov  7 18:34:20 2006"
        },
...

This is, I imagine, easy to implement with a counter in the read line loop.

[wenzeslaus]

Given the strings are used to signal the state, I think it is better to clear them in the caller code instead of the function. It will be more clear what is happening.

[nilason]

Given the strings are used to signal the state, I think it is better to clear them in the caller code instead of the function. It will be more clear what is happening.

I'm not sure what you mean.

[nilason]

Looks good to me.
@NishantBansal2003 Thanks!

[NishantBansal2003]

Given the strings are used to signal the state, I think it is better to clear them in the caller code instead of the function. It will be more clear what is happening.

Sorry, I'm not entirely clear on your suggestion. Could you please explain in more detail where you think the strings should be cleared?

[petrasovaa]

Given the strings are used to signal the state, I think it is better to clear them in the caller code instead of the function. It will be more clear what is happening.

I'm not sure what you mean.

I think he refers to parse_history_line

[nilason]

Given the strings are used to signal the state, I think it is better to clear them in the caller code instead of the function. It will be more clear what is happening.

I'm not sure what you mean.

I think he refers to parse_history_line

That would be my guess too, but that’s where it is known when to clear.

[nilason]

That would be my guess too, but that’s where it is known when to clear.

Or possibly after call to add_record_to_json(), would work as well.

[wenzeslaus]

That would be my guess too, but that’s where it is known when to clear.

Or possibly after call to add_record_to_json(), would work as well.

Yes, that's what I meant. Keeping the state out of the function. Sorry for not being clear.

[echoix]

So is this PR ok, or your requested changes still apply?

[wenzeslaus]

Great. Thank you!