[Addition] Added IniRenderer and IniRenderer test cases #137

[Madmegsox1]

Added ini renderer and ini renderer test cases. Changes that still need to be added are as follows:

• a new in-language renderer
• a new standard library module, as mentioned in this issue.

Ill create another pr later with these changes.

In IniUtils, the list of charecters that need to be escaped within the .ini file are in accourdance with INI file Wiki.

The IniRenderer is very similar to PropertiesRenderer due to .properties file having a similar style to an .ini file

[ghyatzo]

great that this has already been made, any specific reason why lists\sets are not supported? something like this:

[section]
mylist=[a, list, of, things]
; or even
myotherlist=another,list,of,things

I have encountered them plenty of time in the wild. Also, sets could be rendered exactly the same, since the uniqueness of all items in a set should be checked in the pkl side.

[Madmegsox1]

One way of doing this for the time being is as follows:

output {
   renderer = new IniRenderer {
     converters {
       // render lists as comma-separated values
       [List] = (it) -> it.join(",")
    }
  }
}

I will add support for list/sets natively im just currently working on the ini std-lib.

[bioball]

Thanks for the PR! Please also add ini.pkl to this PR too.

The Java-side renderer is actually the less important renderer; those renderers are meant for Java users that want to take an already evaluated Java representation of a Pkl value. We only offer Java-side renderers for some formats.

The more important renderer is the in-language renderer, via ini.pkl, and I don't think we want the Java-side in without the in-language renderer also being there.

[Madmegsox1]

Ive been working on the in-language renderer over the past couple days. I have written the .pkl std lib. I have also added an IniModule class and the ini/RenderNodes class.

I just need to write the LanguageSnippetTest.

@bioball i have a question regarding the ini.pkl stb-lib. As mentioned in the comment above regarding using converters { [List] = (it) -> it.join(",") } could this be done to convert lists inside ini.pkl instead of doing it inside of ini/RenderNodes. There isnt any particular reason i just think it would be cool to do it inside of the stb-lib.

[bioball]

The converters mechanism is a way for users to override the intrinsic behavior of the renderer. The standard library should be defining the intrinsic behavior, so, it's not appropriate to specify converters within stdlib/ini.pkl. Also: for the standard library, we want to define everything in Java, because that will be faster than something implemented in Pkl.

For handling arrays: seems like there's no agreed upon format? Some implementations use commas, and others use square brackets, and others don't support them at all as far as I can tell.

Maybe one way we can handle this is to add a renderer option for some commonly used styles.

class Renderer {
  arrayStyle: ("comma"|"bracket")?
}

Where null means: throw if any List or Listing are found.

[Madmegsox1]

Thank you for your help. Test cases should be done today and i love the idea of adding the array style config to ini.pkl!

[holzensp]

All of what @bioball said and just to make the point explicit; the out-of-the-box-default behaviour of a renderer should always be restricted to target format standard representations. If there is no such standard, the default behaviour must be to error out. This preserves the principle of "least surprise." (This is the reason why, for example JsonRenderer doesn't know how to deal with Durations, even though there are perfectly reasonable default choices.)

[Madmegsox1]

Thank you for confirming. Very helpful!

[Madmegsox1]

Pushed changes for code review purposes. I know i still need to add some more in-lang renderer tests 👍

[Madmegsox1]

Having an issue with the tests that overwrite the render functuons...

So all the values get rendered out correctly but due to the way the ini renderer works using the end functuins to group up Maps, Dynamics etc, it renderes them still at the bottom of the file

res1 = gnirts
res2 = false
res3 = 43
res4 = 2.33
res5 = 3 s
res6 = 4 mb
res8 = [\n  43,\n  false,\n  \"gnirts\"\n]
res9 = [\n  \"gnirts\",\n  false,\n  43,\n  5\n]
res10 = {\n  \"eno\"\: false,\n  \"owt\"\: 2.33,\n  \"eerht\"\: \"3 s\"\n}
res11 = [\n  \"gnirts\",\n  false,\n  43,\n  5\n]
res12 = {\n  \"eman\"\: \"noegip\",\n  \"ega\"\: 31,\n  \"eerht\"\: \"3 s\"\n}
res13 = {\n  \"name\"\: \"noegip\",\n  \"age\"\: 31,\n  \"other\"\: \"rehto\"\n}
res14 = {\n  \"name\"\: \"noegip\",\n  \"age\"\: 41\n}
res15 = String

[res10]
eno = false
owt = 2.33

[res12]
eman = noegip
ega = 31

[res13]
name = noegip
age = 31

[res14]
name = noegip
age = String

Im wondering if there is a flag somewhere that could be accessed by the end functions to tell it that the main render function for that data type has been overwritten?

[Madmegsox1]

Have you had a chance to look at the issue i am having with the tests?

[ghyatzo]

It is annoying that ini files have such a loose format. With there being so many variations, to what extent can we then customize the renderer through converters? For example, some more variations that I have encountered in the wild:

• using : instead of = for the key-value separator
• no nesting, but instead everything is flattened into a single section

Is this something that can be done through converters or should it be a renderer setting like bioball suggested above (or straight up not supprted)?

My main concern is that exactly because the format is so loose, then there is a proliferation of dialects, but we should also be able to adapt this single renderer to any dialect (that doesn't take it too far) with ease. Luckily an ini file doesn't have that many moving parts to generalize.

[Madmegsox1]

Indeed the format that i am basing this renderer off (For the time being) can be found here. Currently im trying to fix some of the tests but need some of the maintainers help on a issue i am having the tests. Bioballs suggestion by letting the usere choose how to rendere the file will be implemented once i have fixed these tests

[Madmegsox1]

Having an issue with the tests that overwrite the render functuons...

So all the values get rendered out correctly but due to the way the ini renderer works using the end functuins to group up Maps, Dynamics etc, it renderes them still at the bottom of the file

res1 = gnirts
res2 = false
res3 = 43
res4 = 2.33
res5 = 3 s
res6 = 4 mb
res8 = [\n  43,\n  false,\n  \"gnirts\"\n]
res9 = [\n  \"gnirts\",\n  false,\n  43,\n  5\n]
res10 = {\n  \"eno\"\: false,\n  \"owt\"\: 2.33,\n  \"eerht\"\: \"3 s\"\n}
res11 = [\n  \"gnirts\",\n  false,\n  43,\n  5\n]
res12 = {\n  \"eman\"\: \"noegip\",\n  \"ega\"\: 31,\n  \"eerht\"\: \"3 s\"\n}
res13 = {\n  \"name\"\: \"noegip\",\n  \"age\"\: 31,\n  \"other\"\: \"rehto\"\n}
res14 = {\n  \"name\"\: \"noegip\",\n  \"age\"\: 41\n}
res15 = String

[res10]
eno = false
owt = 2.33

[res12]
eman = noegip
ega = 31

[res13]
name = noegip
age = 31

[res14]
name = noegip
age = String

Im wondering if there is a flag somewhere that could be accessed by the end functions to tell it that the main render function for that data type has been overwritten?

@holzensp is there functionality there for the Renderer to tell if parts of it has been overwritten by pkl?

[bioball]

Im wondering if there is a flag somewhere that could be accessed by the end functions to tell it that the main render function for that data type has been overwritten? @holzensp is there functionality there for the Renderer to tell if parts of it has been overwritten by pkl?

@Madmegsox1: I'm not totally understanding the question. What do you mean by "main render function for that data type has been overwritten"?

Can you share a snippet of the Pkl input, and your INI output, and your expected INI output?

[Madmegsox1]

Sorry for the confusion by datatypes i mean the pkl converters.

The reason this is happaning is because the ini renderer's end functions such as endMapping or endDynamic group VmTyped, VmMapping, VmDynamic, VmMap at the end of the type and then writes them to the builder and im guessing the reason they are still doing this when the converters are overwritten from pkl is because these end functions are still called which is why these types in the tests appear at the bottom of the output.

Just need to know if there is a flag somewhere could be passed to these endFunctions so they dont append the types?

Here are the snip-its:

Pkl Input

import "pkl:ini"

class Person {
  name: String
  age: Int?
}

res1 = "string"
res2 = true
res3 = 42
res4 = 1.23
res5 = 3.s
res6 = 4.mb
res8 = List("string", true, 42)
res9 = Set("string", true, 42)
res10 = Map("one", true, "two", 1.23)
res11 = new Listing { "string"; true; 42 }
res12 = new Mapping { ["name"] = "pigeon"; ["age"] = 30 }
res13 = new Dynamic { name = "pigeon"; age = 30 }
res14 = new Person { name = "pigeon" }
res15 = null

output {
  renderer = new ini.Renderer {
    omitNullProperties = false
    converters = (jsonValueRenderer.converters) {
      [List] = (it) -> jsonValueRenderer.renderValue(it.reverse())
      [Set] = (it) -> jsonValueRenderer.renderValue(it + List(4))
      [Map] = (it) -> jsonValueRenderer.renderValue(it + Map("three", 3.s))
      [Listing] = (it) -> jsonValueRenderer.renderValue((it) { 4 })
      [Mapping] = (it) -> jsonValueRenderer.renderValue((it) { ["three"] = 3.s })
      [Dynamic] = (it) -> jsonValueRenderer.renderValue((it) { other = "other" })
      [Person] = (it) -> jsonValueRenderer.renderValue((it) { age = 40 } /* fill in missing property */)
      [Pair] = (it) -> jsonValueRenderer.renderValue(List(it.second, it.first))
      [IntSeq] = (it) -> jsonValueRenderer.renderValue(List(it.start, it.end, it.step))
    }
  }
  local jsonValueRenderer = new JsonRenderer {
    converters {
      [String] = (it) -> it.reverse()
      [Boolean] = (it) -> !it
      [Int] = (it) -> it + 1
      [Float] = (it) -> it + 1.1
      [Duration] = (it) -> "\(it.value) \(it.unit)"
      [DataSize] = (it) -> "\(it.value) \(it.unit)"
      [Null] = (it) -> "String"
    }
  }
}

Expected

res1 = gnirts
res2 = false
res3 = 43
res4 = 2.33
res5 = 3 s
res6 = 4 mb
res8 = [\n  43,\n  false,\n  "gnirts"\n]
res9 = [\n  "gnirts",\n  false,\n  43,\n  5\n]
res10 = {\n  "eno"\: false,\n  "owt"\: 2.33,\n  "eerht"\: "3 s"\n}
res11 = [\n  "gnirts",\n  false,\n  43,\n  5\n]
res12 = {\n  "eman"\: "noegip",\n  "ega"\: 31,\n  "eerht"\: "3 s"\n}
res13 = {\n  "name"\: "noegip",\n  "age"\: 31,\n  "other"\: "rehto"\n}
res14 = {\n  "name"\: "noegip",\n  "age"\: 41\n}
res15 = String

Actual output

res1 = gnirts
res2 = false
res3 = 43
res4 = 2.33
res5 = 3 s
res6 = 4 mb
res8 = [\n  43,\n  false,\n  \"gnirts\"\n]
res9 = [\n  \"gnirts\",\n  false,\n  43,\n  5\n]
res10 = {\n  \"eno\"\: false,\n  \"owt\"\: 2.33,\n  \"eerht\"\: \"3 s\"\n}
res11 = [\n  \"gnirts\",\n  false,\n  43,\n  5\n]
res12 = {\n  \"eman\"\: \"noegip\",\n  \"ega\"\: 31,\n  \"eerht\"\: \"3 s\"\n}
res13 = {\n  \"name\"\: \"noegip\",\n  \"age\"\: 31,\n  \"other\"\: \"rehto\"\n}
res14 = {\n  \"name\"\: \"noegip\",\n  \"age\"\: 41\n}
res15 = String

[res10]
eno = false
owt = 2.33

[res12]
eman = noegip
ega = 31

[res13]
name = noegip
age = 31

[res14]
name = noegip
age = String

Thanks for the help @bioball

[Madmegsox1]

Hi has anyone been able to take a look at this?