Feature/tsv support #394
Feature/tsv support #394
Conversation
WalkthroughThe changes introduce a new export type, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Exporter
Client->>Server: Request export with type 'tsv'
Server->>Exporter: Process request for 'tsv'
Exporter->>Server: Generate TSV data
Server->>Client: Respond with TSV data
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
src/exportFromJSON.ts (1)
Line range hint
34-79: Approved: UpdatedexportFromJSONfunction to support TSV exports.The function now correctly handles TSV exports by setting the appropriate delimiter and processing the data accordingly. The implementation of the new case for 'tsv' is consistent with the existing structure for 'csv'.
Suggestion for Improvement:
Consider adding a comment explaining the default delimiter logic for better code clarity, especially for new contributors or when revisiting this code in the future.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- README.md (2 hunks)
- src/converters.ts (2 hunks)
- src/exportFromJSON.ts (3 hunks)
- src/processors.ts (1 hunks)
- src/types.ts (2 hunks)
Additional context used
LanguageTool
README.md
[misspelling] ~131-~131: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...o supports mapper field name by passing an name mapper, e.g. { 'bar': 'baz' }, def...(EN_A_VS_AN)
[uncategorized] ~134-~134: The correct spelling defined by the Internet Assigned Numbers Authority (IANA) is “UTF-8” or “utf-8”.
Context: ...BOM is expected byExcelwhen reading UTF8 CSV file. It is default tofalse. ...(UTF_8_HYPHEN)
Additional comments not posted (7)
src/types.ts (2)
1-1: Approved: Addition of 'tsv' to ExportType.The inclusion of 'tsv' in the
ExportTypetype definition is correct and aligns with the PR's objectives to support TSV exports.
Line range hint
1-11: Approved: Update to exportTypes constant.The update to include 'tsv' in the
exportTypesconstant is correct and necessary for the new export type to function properly. It's also consistent with the changes to theExportTypetype definition.Please ensure that all parts of the system that utilize the
exportTypesconstant are updated to handle the 'tsv' type correctly. This might include functions that rely on this mapping to perform their operations.src/processors.ts (2)
33-35: Approve the changes for TSV and CSV handling.The modification to handle both 'csv' and 'tsv' in a unified manner is a good approach, as it reduces redundancy and enhances flexibility. The dynamic setting of
blobTypebased on thetypeparameter is a smart implementation choice.Please ensure to verify the behavior for both CSV and TSV exports to confirm that no regressions have been introduced.
Line range hint
47-63: Approve the unchanged functiondownloadFile.The function remains unchanged and continues to rely on
generateDataURI. It is important to verify that the integration with the modifiedgenerateDataURIfunctions correctly, especially with the new TSV support.Please verify the integration of
downloadFilewith the modifiedgenerateDataURIto ensure that the new TSV support functions as expected without issues.src/exportFromJSON.ts (1)
19-19: Approved: Expanded delimiter options inIOptioninterface.The addition of the tab character (
'\t') as a delimiter option is a necessary change to support TSV exports and is correctly implemented.src/converters.ts (1)
123-123: Review the updatedCreateCSVDataOptionsinterface for the new delimiter option.The interface now correctly includes the tab character (
'\t') as a valid option for thedelimiterproperty. This change aligns with the PR's objective to support TSV format natively in the library, enhancing flexibility for users.This update is straightforward and aligns well with the existing structure of the interface, ensuring that any implementation of this interface can now specify a tab as a delimiter when creating CSV data.
The changes to the
CreateCSVDataOptionsinterface are approved as they meet the requirements for supporting new delimiter options effectively.README.md (1)
104-106: Correct implementation for TSV export type.The addition of the 'tsv' case in the switch statement is correctly implemented, setting the appropriate
Content-Typefor TSV files.
| function encloser (value: string, delimiter: ',' | ';' | '\t') { | ||
| const enclosingTester = new RegExp(`${delimiter === "\t" ? "\\t" : delimiter}|"|\n`) |
There was a problem hiding this comment.
Review the updated encloser function for handling the tab delimiter.
The function now correctly handles the tab delimiter ('\t') by adjusting the regular expression used to determine if a value needs to be enclosed in double quotes. This change is crucial for supporting TSV formats and ensures that values containing tabs are properly quoted.
However, the implementation could be further optimized by pre-compiling the regular expression outside of the function to avoid re-compilation on every function call, which can improve performance especially when processing large datasets.
Consider pre-compiling the regular expression to improve performance:
+ const enclosingTesterMap = {
+ ',': new RegExp(",|\"|\n"),
+ ';': new RegExp(";|\"|\n"),
+ '\t': new RegExp("\\t|\"|\n")
+ }
function encloser(value: string, delimiter: ',' | ';' | '\t') {
- const enclosingTester = new RegExp(`${delimiter === "\t" ? "\\t" : delimiter}|"|\n`)
+ const enclosingTester = enclosingTesterMap[delimiter]
const enclosingCharacter = enclosingTester.test(value) ? '"' : ''
const escaped = value.replace(/"/g, '""')
return `${enclosingCharacter}${escaped}${enclosingCharacter}`
}Committable suggestion was skipped due to low confidence.
| | extension | false | string | filename extension, by default it takes the exportType | ||
| | fileNameFormatter | false | `(name: string) => string` | filename formatter, by default the file name will be formatted to snake case | ||
| | fields | false | `string[]` or field name mapper type `Record<string, string>` | fields filter, also supports mapper field name by passing an name mapper, e.g. { 'bar': 'baz' }, default to `undefined` | ||
| | exportType | false | Enum ExportType | 'txt'(default), 'css', 'html', 'json', 'csv', 'xls', 'xml', 'tsv' |
There was a problem hiding this comment.
Documentation updates for TSV support are accurate.
The updates to the exportType and delimiter options in the documentation correctly reflect the new support for TSV files. This enhances user understanding and usability of the library.
Static Analysis Fixes:
- Line 131: Correct the article usage from "an name mapper" to "a name mapper".
- Line 134: Update the spelling of "UTF8" to "UTF-8" as per IANA standards.
Apply these corrections:
- an name mapper
+ a name mapper
- UTF8 CSV file
+ UTF-8 CSV fileAlso applies to: 136-136
I noticed that this library does not have support for tsv format, however, the code that exists should be able to handle it already simply by using a different delimiter.
This pull request makes the necessary changes to allow using tsv as the export type.
Currently, when using typescript, you can get tsv exports working by casting the exportFromJSON function to a custom type that allows for "\t" as a delimiter. Adding support directly to the library would be very convenient and prevent the need for ugly type casting.
Note: my IDE added some spacing to the README, and I could not figure out how to prevent it.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation