Skip to content
This repository was archived by the owner on Apr 14, 2023. It is now read-only.

Ensure strings are always wrapped in quotes#441

Merged
7 commits merged into
masterfrom
csv-ensure-strings-are-quoted
Jan 21, 2019
Merged

Ensure strings are always wrapped in quotes#441
7 commits merged into
masterfrom
csv-ensure-strings-are-quoted

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jan 18, 2019

This change ensures:

  • String values being unquoted and potentially being understood as numerical or other values
  • Empty strings being synonomous with null (now they're represented as "")

Tests performed:

  • null is emitted without quotes
  • strings are emitted with quotes
  • strings containing quotes are emitted with encoded quotes
  • temporal values are emitted without quotes
  • numeric values are emitted without quotes
  • formatted values are emitted with quotes

This change ensures:
- String values being unquoted and potentially being understood as numerical or other values
- Empty strings being synonomous with null (now they're represented as "")
@ghost ghost added enhancement Issues related to improving the codebase, the documentation or process within the project generator labels Jan 18, 2019
@ghost ghost added this to the MVP:1 (Generator) milestone Jan 18, 2019
@ghost ghost removed the high-priority label Jan 18, 2019
@ghost ghost merged commit 13d98c5 into master Jan 21, 2019
@ghost ghost deleted the csv-ensure-strings-are-quoted branch January 21, 2019 14:41
@cakehurstryan
Copy link
Copy Markdown
Contributor

Do we support escaped characters? If so is this something we should test for?

@ghost
Copy link
Copy Markdown
Author

ghost commented Jan 22, 2019

Do we support escaped characters? If so is this something we should test for?

@cakehurstryan I tested the content hello "world" which was correctly 'encoded' as "hello ""world""". This encoding is handed off to the CSV encoder so it should cover encoding correctly. I didn't test other types of encoding, but it should be relatively easy to test.

@harryBedfordSL is introducing tests for the CSV writer on ticket #432 so we can add additional tests here, or after its complete, to prove the functionality.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement Issues related to improving the codebase, the documentation or process within the project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants