Allow symbol value via ENV

[manjunath724]

This PR converts string starting with : to symbol. Allows symbol value via ENV.

Config::Options when overriding settings via ENV variables is enabled and parsing ENV variable values is enabled should recognize strings starting with : as symbols.

But strings starting with :: will be left intact.

Ex:

# Given the following environment:
SETTINGS__SECTION__SERVER_TYPE=:remote
SETTINGS__SECTION__SERVER_ACCESS=::ENABLED

# The following settings will be available:
Settings.section.server_type # => :remote
Settings.section.server_access # => '::ENABLED'

String#strip is used for removing whitespace characters from string inputs, such as: ' :remote ', ' : ', ' : remote '

.gsub is used to replace characters with the help of regex pattern match

.tr to covert to snakecase

.downcase to transform uppercase letters to lowercase

Ref: #286

[manjunath724]

The tests have passed, can the PR be reviewed?

[cjlarose]

This new behavior is backwards-incompatible because it's possible that someone has an existing configuration where a value begins with : and their expectation is that they'll receive a String.

I think we have to let users opt into the new behavior explicitly. Let's add a new Config.env_parse_symbols whose default is false.

[manjunath724]

Agreed! I will add a new configuration

[cjlarose]

I prefer the explicit cases for true and false. eval is a needlessly powerful and expensive tool to use here and reduces the readability of the code.

[manjunath724]

I will revert this change

[cjlarose]

None of the transformations are expected. The least surprising behavior from a user's perspective is that their input is interpreted exactly as they specified it. What's the rationale for modifying the input?

[manjunath724]

Transformations are needed for Symbol representation because a String may contain whitespaces, may start with or contain invalid characters, etc.
Example,

1. "1"
2. "$"
3. " hello. this is a string ", etc

Invalid Symbol representation for above stings are:

• :1
• :$
• :hello. this is a string

Whereas, .to_sym would prepend : to a passed string:

• :"1"
• :"$"
• :" hello. this is a string "

After transformation Symbol representations would look like:

• :"1"
• :""
• :hello_this_is_a_string

Can you please suggest the preferred Symbol representation for Config.env_parse_symbols?

[cjlarose]

Ah, I see why these transformation were added then. The behavior, I think, is just unexpected from a user's point-of-view. Right now the behavior is roughly,

If Config.env_parse_symbols is truthy, then an ENV var value that starts with a : (but not with ::) with be returned as a Symbol. If the value starts with : (but not with ::) and contains characters that are incompatible with Ruby's definition of a Symbol literal, then the string is modified to conform to that definition.

This is unexpected for users because they likely have code that depends on the precise characters in the Symbol they specified in configuration. In other words, if they have code the depends on Settings.foo == :"hi, hello!", they would likely expect that ENV['Settings.foo'] = ":\"hi, hello!\"" would initialize Settings.foo to be exactly the Symbol specified. In other words, I'd expect this spec to pass:

        it 'should recognize quoted symbols as symbols' do
          ENV['Settings.new_var'] = ':"hi, hello!"'

          expect(config.new_var).to eq(:"hi, hello!")
        end

Instead, it fails because the returned Symbol is :hi_hello.

Instead of the current behavior, I'd expect the parsing logic to be

If Config.env_parse_symbols is truthy, and if the ENV var value is a valid Ruby Symbol literal, then return a Symbol. Otherwise, just leave it alone and return a String. In particular, if the ENV var value was literally ":hello. this is a string", then I'd expect the user to receive a String containing exactly those same characters.

In code, I'd expect this spec to pass:

        it 'should return invalid symbols as strings' do
          ENV['Settings.new_var'] = ':hello. this is a string'

          expect(config.new_var).to eq(':hello. this is a string')
        end

[manjunath724]

':"bam"' and ":bam" both looks valid to me, as latter was the format requested in this issue #286.

If Config.env_parse_symbols is set to true and the string starts with : then the code will parse the string as symbol.

str.tr(%q{:"'}, '').to_sym # Replace :, ", ' before converting into symbol

A simple logic that doesn't have to invalidate string based on its format.

Example:

':foo'.tr(%q{:"'}, '').to_sym             #   :foo
':"foo"'.tr(%q{:"'}, '').to_sym           #   :foo
':foo bar'.tr(%q{:"'}, '').to_sym         #   :"foo bar"  
':"foo bar"'.tr(%q{:"'}, '').to_sym       #   :"foo bar"
':foo_bar'.tr(%q{:"'}, '').to_sym         #   :foo_bar
':"foo_bar"'.tr(%q{:"'}, '').to_sym       #   :foo_bar

# String with Special characters and Numbers. Also, starts with and without `:`

":$foo".tr(%q{:"'}, '').to_sym            #   :$foo
':"$foo"'.tr(%q{:"'}, '').to_sym          #   :$foo
"4foo-bar".tr(%q{:"'}, '').to_sym         #   :"4foo-bar"
':"4foo-bar"'.tr(%q{:"'}, '').to_sym      #   :"4foo-bar"
"$4foo".tr(%q{:"'}, '').to_sym            #   :"$4foo"
':"$4foo"'.tr(%q{:"'}, '').to_sym         #   :"$4foo"
":*".tr(%q{:"'}, '').to_sym               #   :*
':"*"'.tr(%q{:"'}, '').to_sym             #   :*

From the above examples, we can notice that the string w/o : and/or "(quotes) can be represented as symbols - that should give users a flexibility to define strings as per their convience and set them to be parsed as symbols.

Please correct me if I am wrong with the above conclusion.

[cjlarose]

Ok yes I understand the confusion now after re-reading the comments in #286

I'll re-iterate that I don't find any value in supporting the conversion of strings like :hello this is a string or :4foo-bar into symbols because they are not valid Ruby Symbol literals. Converting these things into Symbols as a "convenience," I think, would be more of a maintenance burden and source of confusion than is necessary.

For now, Config.env_parse_values only parses values that are valid in the Ruby grammar, and I think that's the least surprising convention.

[cjlarose]

Closing because this is stale. I'm open to the idea, but the particular implementation here needs some work.