Issue 5568 - Improve string handling by splitting read strings into only ASCII and non-ASCII strings

# Since one may not shuffle lines I think we should not over-complicate it and start with the proposed code. IMO any optimizations will be based on moving towards migration from JS to C++ and should be done as another commits.

[sergei, 2017-08-30 17:14:08]

https://codereview.adblockplus.org/29531696/diff/29531697/test/FileSystemJsOb...
File test/FileSystemJsObject.cpp (right):

https://codereview.adblockplus.org/29531696/diff/29531697/test/FileSystemJsOb...
test/FileSystemJsObject.cpp:326: readFromFile_Lines({"\n\r\n"}, {""});
The behavior has been changed here a bit. Previously it was producing an
additional empty line if there is either \r or \n at the beginning or at the end
of the line, correspondingly. Namely
\n       => ["", ""]      , now [""]
\nline   => ["", line]    , now [""]
line\n   => [line, ""]    , now [""]
\nline\n => ["", line, ""], now [""]
I think it makes sense because anyway the empty line is ignored in INIParser.

[hub, 2017-08-30 21:13:31]

just a few remarks

LGTM

https://codereview.adblockplus.org/29531696/diff/29531697/src/FileSystemJsObj...
File src/FileSystemJsObject.cpp (right):

https://codereview.adblockplus.org/29531696/diff/29531697/src/FileSystemJsObj...
src/FileSystemJsObject.cpp:67: inline bool CReturnOrLineFeed(char c)
nit': I would name this function "IsEndOfLine()"

https://codereview.adblockplus.org/29531696/diff/29531697/src/FileSystemJsObj...
src/FileSystemJsObject.cpp:72: inline StringBuffer::const_iterator
SkipCReturnAndLineFeed(StringBuffer::const_iterator ii,
StringBuffer::const_iterator end)
and this one SkipEndOfLine()

https://codereview.adblockplus.org/29531696/diff/29531697/src/FileSystemJsObj...
src/FileSystemJsObject.cpp:79: inline StringBuffer::const_iterator
AdvanceUntilCReturnOrLineFeed(StringBuffer::const_iterator ii,
StringBuffer::const_iterator end)
And this one AdvanceToEndOfLine()

But this is not really blocking. I find it clearer.

[sergei, 2017-08-31 10:45:26]

https://codereview.adblockplus.org/29531696/diff/29531697/src/FileSystemJsObj...
File src/FileSystemJsObject.cpp (right):

https://codereview.adblockplus.org/29531696/diff/29531697/src/FileSystemJsObj...
src/FileSystemJsObject.cpp:67: inline bool CReturnOrLineFeed(char c)
On 2017/08/30 21:13:31, hub wrote:
> nit': I would name this function "IsEndOfLine()"

I am actually also not glad with the name of this function. I will definitely
add "Is" prefix.
What about IsLineTerminator and SkipLineTerminators and AdvanceToLineTerminator?

[hub, 2017-08-31 12:59:44]

https://codereview.adblockplus.org/29531696/diff/29531697/src/FileSystemJsObj...
File src/FileSystemJsObject.cpp (right):

https://codereview.adblockplus.org/29531696/diff/29531697/src/FileSystemJsObj...
src/FileSystemJsObject.cpp:67: inline bool CReturnOrLineFeed(char c)
On 2017/08/31 10:45:26, sergei wrote:
> On 2017/08/30 21:13:31, hub wrote:
> > nit': I would name this function "IsEndOfLine()"
> 
> I am actually also not glad with the name of this function. I will definitely
> add "Is" prefix.
> What about IsLineTerminator and SkipLineTerminators and
AdvanceToLineTerminator?

"End of line" or EOL is often used through the UNIX man pages. "Line break" can
be used in the same context but I have seen it more for word processor. "Line
Terminator" I haven't seen that. Hence my choice of words.

[sergei, 2017-08-31 13:21:44]

https://codereview.adblockplus.org/29531696/diff/29531697/src/FileSystemJsObj...
File src/FileSystemJsObject.cpp (right):

https://codereview.adblockplus.org/29531696/diff/29531697/src/FileSystemJsObj...
src/FileSystemJsObject.cpp:67: inline bool CReturnOrLineFeed(char c)
On 2017/08/31 12:59:43, hub wrote:
> On 2017/08/31 10:45:26, sergei wrote:
> > On 2017/08/30 21:13:31, hub wrote:
> > > nit': I would name this function "IsEndOfLine()"
> > 
> > I am actually also not glad with the name of this function. I will
definitely
> > add "Is" prefix.
> > What about IsLineTerminator and SkipLineTerminators and
> AdvanceToLineTerminator?
> 
> "End of line" or EOL is often used through the UNIX man pages. "Line break"
can
> be used in the same context but I have seen it more for word processor. "Line
> Terminator" I haven't seen that. Hence my choice of words.

agree.

[hub, 2017-08-31 13:30:59]

LGTM