aaemnnosttv / wp-cli-dotenv-command

Interact with a .env file from the command line.
https://aaemnnost.tv/wp-cli-commands/dotenv/
MIT License
173 stars 19 forks source link

Please quote all salts for consistency #5

Closed ssteinerx closed 8 years ago

ssteinerx commented 8 years ago

I see that you recently added single quotes to values containing spaces.

Is there any reason not to just quote them all for consistency?

I'd do a pull request, but the change is so simple it seems unnecessary.

Thanks for a very useful tool!

aaemnnosttv commented 8 years ago

Hey there, thanks for bringing this up!

I agree, salts should be quoted. The only reason they're not currently is just because the file parsing/processing does not differentiate between which values are salts, and which are not. That and I don't particularly think that every value should be quoted either, hense the issue.

I appreciate the offer for a pull request. It might not be worth it though since it is being rewritten anyways (see the 1.0-dev branch).

I'll plan on including this in the next release.

ssteinerx commented 8 years ago

Unfortunately, you can't source the .env file if it contains unescaped parenthesis, or various other characters as I just discovered.

Not sure what the solution is, but I'm using this with roots.io (not their Ansible setup, a Docker one I'm developing) and storing the environment in .env and I'm not able to use the bash source command to pull them in.

Something to think about for the 1.0-dev branch, I'd think.

I.E. The output should be able to be used with the bash source command with this snippet:

# Stolen from
# https://github.com/kennethreitz/autoenv/blob/master/activate.sh
# This just sets allexport while the environment is imported, then sets
# it back to what it was after.  Clever.
typeset allexport
allexport=$(set +o | \grep allexport)
set -a
source .env
eval "$allexport"
unset allexport
aaemnnosttv commented 8 years ago

Sounds good, thanks for the feedback. Are the salts the only problem for you?

ssteinerx commented 8 years ago

It is the only thing I've used, so, yes. ;-)

ssteinerx commented 8 years ago

One thing I've seen used in other programs that generate this sort of random key is the ability to limit the character set used in the key.

For example, in Ansible, when using the password lookup (which generates a random key), you can do something like:

    mysql_database: "{{ lookup('password', 'build/{{server_name}}/msql_database chars=ascii_lowercase length=20') }}"

Where the chars is an optional parameter using the normal Python string constants.

ssteinerx commented 8 years ago

NOTE: the Ansible syntax is very strange looking, for sure, but makes sense in the Ansible context.

aaemnnosttv commented 8 years ago

This should be all resolved now in 1.0 which was just released. Take a look and let me know if you're still having trouble.