Skip to content

Array variables PoC #748

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Array variables PoC #748

wants to merge 3 commits into from

Conversation

boguszj
Copy link

@boguszj boguszj commented Feb 26, 2025

@ndeloof I realize you mentioned that this is not a good first issue, but since I already had a local PoC that works and seems to break no tests here or in main repo (locally), I thought it would be a shame not to make sure. If this is not the right direction, then I'm sorry for the spam. If it is tho, I could continue working on this and come back with complete change set across all the repos if you think it's worth it (I realize this is far from everything that needs to be done).

Signed-off-by: boguszj <[email protected]>
@ndeloof
Copy link
Collaborator

ndeloof commented Feb 27, 2025

That's an interesting first step, but many questions remain with this approach
Using bash array syntax was a suggestion, not sure this is the best option to be adopted for this feature. AFAIK we can't retrieve such variables from go code using os.Env (they are not actual environment variables, but a bash-feature) which would make it impossible to append/override values from command line.
Feel free to move forward anyway, at least this could demonstrate an opportunity and give us more inputs to decide which direction to adopt

@boguszj
Copy link
Author

boguszj commented Mar 1, 2025

@ndeloof I've leaned into bash-like syntax and implemented a following "spec":

Indexed declarations (for env-file):

ARR[0]="0"
ARR[1]="1"

Inlined declarations (for both env-file & system env)

ARR="(foo bar)"
ARR='("foo bar")'
ARR="('foo bar')"
ARR="(foo\ bar)"

Referenced like this:

$ARR[*]
${ARR[*]}

Referencing the array without [*] suffix results in:

  • string literal of the array declarations for inlined
  • empty string for indexed

Inlined declarations take precedence before indexed ones, which matches current precedence for regular variables - it's more likely someone would want to override an env file with an env variable rather than the other way around. I've initially implemented overriding specific indexes of inlined declarations with indexed ones, but ended up rolling that one back, it seemed confusing.

I've also added the tests that came to mind. Apart from testing the new module, I added / extended interpolation & loader tests. If I missed anything, I'll be happy to add it.

Let me know what do you think about such approach & whether or not you'd like to move forward with it - if you would, I'd be happy to open a PR in a spec repo.

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

Successfully merging this pull request may close these issues.

2 participants