Adapt_framework: CVS delimiter detection is wrong on importing csv language files

Created on 30 Jul 2020  路  8Comments  路  Source: adaptlearning/adapt_framework

Subject of the issue/enhancement/features

The csv delimiter detection introduced in #2737 is not working correctly.
The introduced regex can't work correctly:

     const firstLineMatches = fileContent.match(/.+\/"{0,1}.{1}/);

It probably should look like:

    const firstLineMatches = fileContent.match(/.+\/"{0,1}[,;\t]{1}/);

Reference: https://github.com/adaptlearning/adapt_framework/blob/3f25a4d26eee849771478f8f0f1b3847b64f0885/grunt/helpers/Translate.js#L297

bug

All 8 comments

How is it not working correctly? What issue are you trying to solve?

I don't see how switching from . (any character) to [,;\t] (only comma, semicolon and tab) changes anything. It only limits the types of delimiter you could have. The code would function identically otherwise.

This line is trying to find the delimiter at the end of the first column. It says "match any character one or more times, a slash, followed by either 0 or 1 speechmarks, and then any other next character". It's the any other next character that would be the discovered delimiter. The only place I could see this not work would be if there were escaped speechmarks in the first column - which should never happen given that the first column is always a JSON property path and a _id.

Is it that the match doesn't work when the first column isn't wrapped with speechmarks?

Update: It was that there were slashes in the second column and the regex as defined is greedy

The regex is matching any character.

.+ -> min 1 char 
\"{0,1} -> must not match 
.{1} -> one char

So it can match on any place after the first char

In my case it detected "p" as delimiter. The regex is still not save to use but better. Ideally it will be removed or a library is used therefore.

Can you post the first line of your csv? I want to find something more fitting.

I can't post they complete line because it's a private project. But it's like that:

"articles/5e857dd5f99bb68c5cbf6515/body/","<p><strong>Le client est en ligne</strong></p>

Yea that makes sense.

Yea, your suggestion of limiting the delimiters seems like the only sensible option. This article suggests pipe and space should be included and we can probably drop the rest of the regex just matching the delimited list directly. /[,;\t| ]{1}/

These main use-cases would all work fine (no marks, marks, marks+html):

articles/5e857dd5f99bb68c5cbf6515/body/,Adapt
"articles/5e857dd5f99bb68c5cbf6515/body/","Adapt"
"articles/5e857dd5f99bb68c5cbf6515/body/","<p>Adapt</p>"

The best is to use this https://www.npmjs.com/package/detect-csv or this https://www.npmjs.com/package/csv-string#detectinput--string--string

With the regex their may still be a problem. Detection should also only be done if not defined via command line.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

brian-learningpool picture brian-learningpool  路  4Comments

sarveshwar-gavhane picture sarveshwar-gavhane  路  5Comments

guywillis picture guywillis  路  6Comments

chris-steele picture chris-steele  路  6Comments

chris-steele picture chris-steele  路  4Comments