google / go-jsonnet

Apache License 2.0
1.61k stars 230 forks source link

std.URLEncode/Decode #427

Open ghostsquad opened 4 years ago

ghostsquad commented 4 years ago

This is a request to add URLEncode and URLDecode stdlib functions. I'm not sure it would be wise to attempt to produce this code in native jsonnet, when it is trivially done in golang.

Is there any problem with implementing stdlib functions in go-jsonnet that aren't in the c++ variant?

sbarzowski commented 4 years ago

Is there any problem with implementing stdlib functions in go-jsonnet that aren't in the c++ variant?

Yes. We want to keep them compatible.

I'm not sure it would be wise to attempt to produce this code in native jsonnet, when it is trivially done in golang.

As a rule of thumb we want to have a Jsonnet implementation, even if we expect to have a builtin for performance reasons. The cost of long-term maintenance + keeping backwards compatibility + documentation + tests + support is much bigger than the cost of the initial implementation. This is especially important when there is some ambiguity in what the function should do. I don't know if there is a precise standard available in this case.

sh0rez commented 4 years ago

See https://github.com/jsonnet-libs/xtd/blob/master/url.libsonnet for a jsonnet implementation

ghostsquad commented 4 years ago

So, @sh0rez would it be ok to pull that given code as the jsonnet implementation?

@sbarzowski would I need to submit a PR to google/jsonnet with the jsonnet stdlib function, then a followup PR in google/go-jsonnet to implement it in Go?

ghostsquad commented 4 years ago

What's the timeline for deprecating the CPP variant?

sbarzowski commented 4 years ago

@sbarzowski would I need to submit a PR to google/jsonnet with the jsonnet stdlib function, then a followup PR in google/go-jsonnet to implement it in Go?

It's enough to submit a PR to cpp-jsonnet to have it included. The stdlib in go-jsonnet is synced from time to time with cpp-jsonnet. If you want we can do this immediately after the addition (it's fairly trivial).

What's the timeline for deprecating the CPP variant?

There's no concrete timeline. And even when we mark it officially deprecated, we will still want to provide basic support for a while, so not much will change. The most important part of deprecation process from the developer standpoint has already happened – we no longer work on making C++ version faster, we may not add convenience command-line options there and we are going to base developer tools on go-jsonnet.

candlerb commented 3 years ago

This is especially important when there is some ambiguity in what the function should do. I don't know if there is a precise standard available in this case.

For OAuth client_id / client_secret, this is defined in RFC 6749 sec 2.3.1, where appendix B references "application/x-www-form-urlencoded" as defined in W3 REC-html401-19991224.