cpb- / yocto-cooker

Meta buildtool for Yocto Project based Linux embedded systems
GNU General Public License v2.0
43 stars 22 forks source link

Allow comments in menu / JSON5 #138

Closed AdrienMatricon closed 1 year ago

AdrienMatricon commented 1 year ago

It would be nice to be able to add comments to the menu file.

Comments are not directly part of the JSON standard, which is intended for machine-to-machine communication, but they would make menu files much easier to maintain.

JSON config files seem to be the reason why the JSON5 superset exists, and as far as I can tell enabling that superset would be as simple as replacing json.load() by json5.load() (and same for other calls) in cooker.py, and adding a dependency to pyjson5 in setup.py.

cpb- commented 1 year ago

You can use the pseudo-sections "notes" to add comment. See sample-menus/pi3-sample-menu.json for an example.

AdrienMatricon commented 1 year ago

That doesn't really match how one uses comments though.

For example, in sample-menus/pi3-sample-menu.json I'd want to be able to comment every line in

"local.conf": [
    "MACHINE = 'raspberrypi3' ",
    "ENABLE_UART = '1' ",
    "CMDLINE_append += 'quiet' "
]
cpb- commented 1 year ago

Here's the trick I use:

"local.conf": [
        "# We build an image for RPI 3",
    "MACHINE = 'raspberrypi3' ",
        "# Activate serial port output",
    "ENABLE_UART = '1' ",
        "# Avoid kernel messages on the console",
    "CMDLINE_append += 'quiet' "
]
AdrienMatricon commented 1 year ago

@cpb- That's definitely a nice trick, and I guess that if those lines are copy/pasted as is in local.conf (I assume they are?) you can argue that you're commenting the generated file, but it does feel like a trick rather than a clean solution.

Is there any reason not to use JSON5 ? I don't know much about it but it would seem like a good fit for cooker's use of JSON. I'm willing to try and make the corresponding PR if you're open to the idea

cpb- commented 1 year ago

Hello Adrien, Being able to accept JSON5 files seems interesting. Could you propose a PR (with some tests too for the new features)

AdrienMatricon commented 1 year ago

I've created a PR here, but I'm unsure what tests to put in.

I can imagine creating a test/json5 directory with a menu in it copy/pasted from another test and with comments added to it to check that they are indeed supported now, but that seems both not enough (it's a single test) and too much (it's basically testing pyjson5 rather than cooker). Did you have anything specific in mind?

cpb- commented 1 year ago

Closed by #143