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

Roundtrip to/from a property file breaks #1415

Closed
Kopfbremse opened this issue Nov 7, 2022 · 7 comments
Closed

Roundtrip to/from a property file breaks #1415

Kopfbremse opened this issue Nov 7, 2022 · 7 comments
Labels

Comments

@Kopfbremse
Copy link

Kopfbremse commented Nov 7, 2022

Version of yq: 4.29.2
Operating system: linux
Installed via: binary release

Describe the bug
If a YAML contains keys that are numbers, the roundtrip YAML --> PROP and back PROP --> YAML fails.
This is because numbers are interpreted as array indexes and not as key names. See my feature proposal below.

Input Yaml

error:
  count:
    404: 1
    500: 3
  msg:
    404: unavailable
    500: internal server error

Command 1
Step 1: Convert YAML to PROPS
yq --output-format props '.' "$FILE_YAML"

error.count.404 = 1
error.count.500 = 3
error.msg.404 = unavailable
error.msg.500 = internal server error

Command 2
Step 2: Convert PROPS back to YAML
yq --input-format props $FILE_PROPS

error:
  count:
    - null
    - (...)
    - "1"
    - null
    - (...)
    - "3"
  msg:
    - null
    - (...)
    - unavailable
    - null
    - (...)
    - internal server error

Expected behavior
Command 2 should generate the input of step 1 to achieve a working roundtrip

Feature proposal
It would be nice if there were new options to control the conversion from PROP to YAML more detailed,
e.g. --props-number-handling = [arrayIndex, keyName].
I love the yq support of property files. However, since it is perfectly valid to use numbers in property files, I consider it an incomplete feature.

@mikefarah
Copy link
Owner

Yeah I see. I'll have to think about this - issue with a flag is that it wouldn't work if you had a mix of arrays and numeric keys.

@mikefarah
Copy link
Owner

As a workaround, would it be ok to cast the numbered keys to strings (e.g. "400") before converting to props, and then parse them as numbers again after reading from props?

@Kopfbremse
Copy link
Author

Kopfbremse commented Nov 7, 2022

You are right ... a general option for the whole decoding process is not sufficient.
What do you think about:

Leave the default behaviour as it is (treat numbers in key paths as array index).
Add an option to define exceptions to the default behaviour for a list of paths:

--props-numbers-as-keys = 'error.count.*, error.msg.*'

or equivalent in the upper use case:

--props-numbers-as-keys = 'error.*'

Think of all those properties files out there in the wild. Wouldn't it be nice to process them with yq?
A typical roundtrip would then be the other way around PROPS -> YAML -> PROPS to process a property file.
Since the source in this case is not YAML, your idea of encoding numbers in YAML as strings in the prop file does not help here.

mikefarah added a commit that referenced this issue Nov 8, 2022
@mikefarah
Copy link
Owner

I'm always thinking of property files out in the wild. I see them in my dreams :)

I think the right way to do it is via expressions. I'll add a array_to_map operator which will do the conversion, then you'll need to do:

yq -p=props '(.error.count, error.msg) |= array_to_map' file.props

Will add in the next release

@Kopfbremse
Copy link
Author

That's a good idea!
Thanks for the extension in the next release.
I am very pleased how quickly you respond.

@Kopfbremse
Copy link
Author

They have already implemented and tested it in less than a day since I raised this issue?
That's mind-boggling to me!

@mikefarah
Copy link
Owner

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

No branches or pull requests

2 participants