search

thomasloven / hass-lovelace_gen

🔹 Improve the lovelace yaml parser for Home Assistant
MIT License
218 stars 22 forks source link

add tojson to note #10

Closed akasma74 closed 4 years ago

akasma74 commented 4 years ago

I thought it makes sense to note that both fromjson and tojson are not Jinja2 filters and belong to this integration.

Actually, initially I mixed them up with from_json and to_json, which are HA filters. Maybe it would make sense to make your names the same as those in HA because they both are used in templates and do the same thing so having different names is ok-ish but might confuse some. Just an idea from templates' user.

thomasloven commented 4 years ago

tojson is a default jinja filter, though: https://jinja.palletsprojects.com/en/2.11.x/templates/#tojson

akasma74 commented 4 years ago

you're right, thanks for enlightening!

I had such an impression maybe because I read HA docs on templating first and when I noticed from/to_json there I didn't look for the same functionality in the Jinja docs 😄 Well, it's even more weird then:

It's up to you but aligning with HA looks more consistent to me (as both HA and lovelace_gen implement to and from)

akasma74 commented 4 years ago

but the PR has no meaning anymore

thomasloven commented 4 years ago

Yeah, you're right. When I started working on fromjson, the to_json and from_json didn't actually exist yet.

I'll do some testing, and then I'll probably remove it from lovelace_gen if the hass ones work the same way.

akasma74 commented 4 years ago

Yeah, you're right. When I started working on fromjson, the to_json and from_json didn't actually exist yet.

they appeared maybe 6 months ago or so, pretty new stuff

I'll do some testing, and then I'll probably remove it from lovelace_gen if the hass ones work the same way.

they are just wrappers for appropriate functions from json module