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

Use escape character over double quoting in generated CSV #972

Closed
wants to merge 3 commits into from

Conversation

ebeahan
Copy link
Member

@ebeahan ebeahan commented Sep 21, 2020

Summary

Adjust the params in the csv.writer() object in the CSV generator to use escape \ as an escape characters for double quotes within an already quoted field. Default behavior generates field values with double double-quotes.

Behavior was noted while working on #966.

Example

Current:

2.0.0-dev,true,base,labels,object,core,,"{""application"": ""foo-bar"", ""env"": ""production""}",Custom key/value pairs.

PR:

2.0.0-dev,true,base,labels,object,core,,"{\"application\": \"foo-bar\", \"env\": \"production\"}",Custom key/value pairs.

Side Effect

One small, side-effect change is example values themselves containing \ as part of their string values will now be quoted:

Current:

2.0.0-dev,true,dll,dll.path,keyword,extended,,C:\Windows\System32\kernel32.dll,Full file path of the library.

PR:

2.0.0-dev,true,dll,dll.path,keyword,extended,,"C:\Windows\System32\kernel32.dll",Full file path of the library.

@ebeahan ebeahan self-assigned this Sep 21, 2020
@ebeahan ebeahan added the ready Issues we'd like to address in the future. label Sep 22, 2020
@ebeahan
Copy link
Member Author

ebeahan commented Sep 22, 2020

Adding backslash escaping into CSV breaks default importing into a spreadsheet app, so I won't move forward pursuing this change for now.

@ebeahan ebeahan closed this Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Issues we'd like to address in the future.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant