Make --files-from work like in gettext
#75
Conversation
|
Thanks a lot for your contribution! Can you add 1 entry to each of the 3 test files (so you have 2 lines in each)? And test for at least 1 string from the second file? Just to be sure it actually does the splitting we're after. |
| @@ -195,6 +195,14 @@ function xgettext(input, options, cb) { | |||
| if (options['files-from']) { | |||
| input = fs.readFileSync(options['files-from'], options['from-code']) | |||
| .split('\n') | |||
There was a problem hiding this comment.
The combination of this line and the lines you added feels a bit like a mix of 2 solutions.
I'd choose either of these:
- If you want to follow GNU gettext as close as possible, use readline to do the line splitting. After that you can use your code to do the trimming (\t \r \n and space). Ideally a future commit/PR can take care of ignoring lines that start with # (comments).
- Or use
.split(/\r\n|\r|\n/)to split immediately and forget about \t and space.
To get the core functionality you were after, I'm perfectly fine with the second option. Of course getting close to GNU gettext is the goal, but this would already be a step in that direction.
| return c + acc; | ||
| } | ||
| return acc; | ||
| }, ''); |
There was a problem hiding this comment.
Also, having to loop through all characters just to remove the last ones feels quite heavy.
Can't this be done with a regex? Something like line.replace(/[\t\r ]+$/, '')?
| @@ -195,6 +195,14 @@ function xgettext(input, options, cb) { | |||
| if (options['files-from']) { | |||
| input = fs.readFileSync(options['files-from'], options['from-code']) | |||
| .split('\n') | |||
| .map(function (line) { | |||
| return line.split('').reduceRight(function(acc, c) { | |||
There was a problem hiding this comment.
split('') has some issues. You can use Array.from(line) since we only support node 6 and up.
Fixes #74