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

Replace comma-separated envvars with comma+whitespace separated envvars #2682

Open
mrnoname1000 opened this issue Feb 22, 2024 · 2 comments
Open

Comments

@mrnoname1000
Copy link

Enhancement Type

Improve an existing feature

Describe the enhancement

Seems like there are some envvars that split on commas, some on commas+newlines, some on commas+whitespace, and others on just whitespace. I propose replacing all of the first type with comma+whitespace splits unless they rely on newlines being retained (which I don't see being an issue). In places where commas are required (e.g. in mc-image-helper, and are they really required?), values can first be split on commas+whitespace, then re-joined on commas:

IFS=,$IFS                          # prepend comma to $IFS
plugins=($PLUGINS)                 # split on $IFS (comma+whitespace)
mc-image-helper mcopy ... "$*"     # join with comma
IFS=${IFS#?}                       # restore $IFS

The motivation behind this issue is this snippet from my docker-compose.yml:

SPIGET_RESOURCES:
  # spark,discordsrv,deathchest,pluginmanager,spark,veinminer #,vault,essentialsx
  57242,18494,101066,69061,57242,12038 #,34315,9089

Notice how I accidentally duplicated one of the plugins. Ignoring how cryptic YAML multiline indicators are, I would prefer to write this instead:

SPIGET_RESOURCES: >-
  57242   # spark
  18494   # discordsrv
  101066  # deathchest
  69061   # pluginmanager
  12038   # veinminer
  #34315   # vault
  #9089    # essentialsx

I can work on a PR for this if we're in agreement!

@itzg
Copy link
Owner

itzg commented Feb 22, 2024

Yes, those need to be brought into consistency of commas or newlines. Spaces might be significant so it is harder to declare that as the ubiquitous arg delimiter. There is a constant already for comma or newline

https://github.com/itzg/mc-image-helper/blob/388da11f847afd663ccd08f7544cb0720e1de000/src/main/java/me/itzg/helpers/McImageHelper.java#L95

A PR would be great, but change only one place first like SPIGET_RESOURCES so I can see what you have in mind.

@itzg
Copy link
Owner

itzg commented Feb 22, 2024

...actually SPIGET_RESOURCES is an unusual case since that's one of the next things I want to port over to mc-image-helper. At that point, it'll get the proper comma or whitespace/newline arg delimiting support. Also, support for inline comments is actually a feature that I was discussing with someone on Discord.

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

No branches or pull requests

2 participants