Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update embedded jsonnet code. #35

Open
Akhalaka opened this issue Oct 23, 2022 · 5 comments · May be fixed by #54
Open

Update embedded jsonnet code. #35

Akhalaka opened this issue Oct 23, 2022 · 5 comments · May be fixed by #54

Comments

@Akhalaka
Copy link

Testing using the docker master image, but I believe the pkger.go code is using old jsonnet files that don't have the "filename" and "version" parameters in the d.pkg() function.

Given that the most recent changes appear to allow rendering the docs directly using jsonnet, is the recommended path forward to directly use: jsonnet -S -c -m docs/ docs.jsonnet instead of the docsonnet binary?

@Duologic
Copy link
Member

This is a very good question.

I've expanded doc-util with these parameters to get a bit nicer output with the jsonnet renderer but made them optional to not break the golang version. Are you facing any problems because of this?

Do you think the jsonnet renderer works well enough? I've done my best to get at least feature parity. I'm not aware of many people using it so any feedback on this would be very useful.

Personally I prefer to use the jsonnet renderer as it reduces the number of binary dependencies I need to write and maintain libraries (jsonnet and jb), but it requires a bit more bootstrap code to get the docs. Perhaps the bootstrapping is a blocker for some.

@malcolmholmes
Copy link
Collaborator

What do you mean by "bootstapping" @Duologic ?

@Duologic
Copy link
Member

With the golang binary, it comes with batteries included, you only need to run docsonnet and it will generate the docs. With jsonnet, you'll need to have a place where you call d.render() with jsonnet -S -c -m docs/ docs.jsonnet.

Also mentioned in the docs: https://github.com/jsonnet-libs/docsonnet/tree/master/doc-util#fn-render

@Akhalaka
Copy link
Author

In experimenting with this, I do like the jsonnet renderer. It is relatively simple to use and it generate comparable markdown files. They are certainly not identical, but adding things like the install/usage statement I think is an overall improvement. It also gives the end user the ability to alter the format of the markdown by overriding the relevant functions/templates in render.libsonnet which is very convenient (if you know what you are doing, as I have definitely broken myself attempting this for a bit).

That said, the binary is then currently out of sync with what is on master. If I were to create the following "package" (living in /tmp/test)

# test.jsonnet
{
  local d = import 'doc-util/main.libsonnet',
  '#':: d.pkg(name='test', url='https://example.com', help='help', filename='main.libsonnet', version='1.2.3'),
}

And then run docker run -it --rm -v /tmp/test:/vol jsonnetlibs/docsonnet:master /vol/test.jsonnet produces the following error:

Extracting from Jsonnet
Extracting: RUNTIME ERROR: function has no parameter filename
    /vol/test.jsonnet:3:9-111    object <anonymous>
    ...

This appears to be due to the fact that the content in pkger.go is out of date and has not been updated with your updated doc-util/main.libsonnet file that allows these extra 2 parameters to the pkg function.

@Duologic
Copy link
Member

I'd like to find good reasons to keep the golang version. One potential problem with jsonnet might be performance on a big library, I'll probably test that some time soon.

Regardless it might be useful to fix above errors and support the additional arguments. We're open to PRs if you want to give it a go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants