search

gma / nesta

File Based CMS and Static Site Generator
http://nestacms.com
MIT License
902 stars 120 forks source link

Cannot use env-specific var when not defined at top level #200

Closed pelargir closed 5 months ago

pelargir commented 5 months ago

The default config file comes with 2 google_analytics_code key/value pairs. There is a top-level copy (commented out) and a copy underneath the production key. The comment for the top-level copy suggests "not setting a default value" so I left it commented out. This caused a lot of problems as the copy underneath the production key was not getting picked up. It was always returning a blank value.

Turns out, the Nesta::Config#fetch method will only return an env-specific value if that variable is defined both in that environment and at the top-level in the config file. (config.fetch(setting) raises an exception even if the key exists in env_config). This behavior is quite unexpected and was very difficult to track down.

I'd be happy to submit a PR fixing this, but wanted to ask first since this might be intended behavior. Is it?

gma commented 5 months ago

Oh balls, sorry. That the production-scoped key didn't get picked up is definitely a bug.

When I read this I immediately thought "I bet I know when this got introduced". Nesta used to read the config file, and fall back to reading environment variables. I got rid of the support for that in March 2023, and I think I'll have created this problem then.

Patches welcome. Thanks for reporting it…

pelargir commented 5 months ago

I've created https://github.com/gma/nesta/pull/202

gma commented 5 months ago

I've merged it. I'll give the new GA code a whirl (I've still not ported my sites over, so I'll be interested to see this), get that merged, then release 0.16.0.

People can install from the main branch in the meantime if they need this fix sooner than I get that done.