Introduce formatters #455
Introduce formatters #455
Conversation
|
This PR is ready for review and merge. Main features added:
Small changes:
|
package.json
Outdated
| @@ -34,7 +34,7 @@ | |||
| "lint": "eslint bin lib test", | |||
| "test-with-coverage": "nyc --reporter=text node test | tap-spec", | |||
| "coveralls": "nyc report --reporter=text-lcov | coveralls", | |||
| "deploy:docs": "docpress b && gh-pages -d _docpress", | |||
| "deploy:docs": "docpress s", | |||
There was a problem hiding this comment.
Looks like this needs to be reverted. Maybe add another script for serving docs?
|
|
||
| if (value === undefined) return ''; | ||
|
|
||
| if (value[0] === '"') value = value.replace(/^"(.+)"$/,'$1'); |
There was a problem hiding this comment.
Maybe a comment about what this is doing.
| const defaulStringFormatter = require('./string'); | ||
|
|
||
| function stringExcel(opts = { stringFormatter: defaulStringFormatter() }) { | ||
| return (value) => `"="${opts.stringFormatter(value)}""`; |
There was a problem hiding this comment.
A link or comment about why this is the way it is.
There was a problem hiding this comment.
This is documented in the readme. Do you think that it should also be documented here?
There was a problem hiding this comment.
Yeah, mainly if there is an issue or someone wants to submit a PR adding features here.
|
Is this ready to merge? |
|
sorry @knownasilya I've been in the middle of moving to a new country and pretty busy. One thing that I was thinking is that we should probably merge these v5 PRs to a v5 branch so we don't get people confused because the Readme doesn't match the npm version. |
|
I'm going to add a note to the top of the readme pointing to the last published readme and mentioning under construction. |
|
This closed #463 |
Alright! v5 seems to have some shortcomings so let's get right into v6.
This PR introduces the concept of formatters which allow the user to customize how different Javascript types are converted into text for CSV.
This would fix the 2 use cases explained in #420 and will provide a better solution for #444.
Also, I'm planning to add a second argument to formatters indicating if it's a header or not) which will fix the use case explained in #195.
This PR is to gather comments from the community if there are any.
I'm still working on fine-tuning the concept a fit further.