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

map documentation could use some more information #221

Closed
brucejo75 opened this issue Mar 17, 2022 · 7 comments
Closed

map documentation could use some more information #221

brucejo75 opened this issue Mar 17, 2022 · 7 comments

Comments

@brucejo75
Copy link
Contributor

brucejo75 commented Mar 17, 2022

Moved from https://github.com/caddyserver/caddy/issues/4642


Did you know that the map directive cannot assign integers to placeholders? Unclear from the documentation. There is a lot of integer assignment in the example but if you assume the placeholder is an integer you would be wrong.

Luckily I ran across this forum entry.

Request: please update the documentation to explain that the map directive cannot assign integers.

Thanks!

@brucejo75
Copy link
Contributor Author

BTW: let me know if you would like me to submit a PR. Happy to help!

@mholt
Copy link
Member

mholt commented Mar 17, 2022

The Caddyfile currently parses all values as strings. I suppose we could try to infer numbers and cast them before putting them into the map handler, but the map handler does in fact support numeric types if you use JSON.

I believe the only place the type of the map value would matter is in CEL expressions, not general Caddy usage.

@brucejo75
Copy link
Contributor Author

That is a rather complicated specification. Maybe worth some more documentation? I am happy to take a shot at it if you like.

mholt added a commit to caddyserver/caddy that referenced this issue Mar 17, 2022
@mholt
Copy link
Member

mholt commented Mar 18, 2022

Well, I haven't tested it yet, but I pushed a quick fix here: caddyserver/caddy@93c99f6

It should now store numbers as numeric types (and booleans as booleans).

If you want to build from tip and test it out, that'd be splendid 😃

@francislavoie
Copy link
Member

francislavoie commented Mar 18, 2022

I had some ideas to make this nicer, and work "as expected" in more situations: caddyserver/caddy#4643.

I liked Matt's idea of casting to scalar types, but it felt like a hack by doing it inside of the directive's parsing, instead of the token dispenser. And Matt's implementation didn't handle "true" and "0" as a strings, it would cast those to boolean/int respectively too.

@mholt
Copy link
Member

mholt commented Mar 18, 2022

Since that PR is merged, this should be much more intuitive now, which I think is more correct. Thanks!

@mholt mholt closed this as completed Mar 18, 2022
@brucejo75
Copy link
Contributor Author

Wow! OK

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

No branches or pull requests

3 participants