Add PDFCharIsLineEnding helper

Add PDFCharIsLineEnding helper

This CL adds a helper to check if a given character is a \n or \r.

R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/4b38229b004b7b6493cb4c4507df450adc6085f6

[dsinclair, 2015-10-28 15:20:30]

dsinclair@chromium.org changed reviewers:
+ thestig@chromium.org, tsepez@chromium.org

[dsinclair, 2015-10-28 15:20:31]

PTAL.

[Tom Sepez, 2015-10-28 17:17:21]

https://codereview.chromium.org/1418073006/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp (right):

https://codereview.chromium.org/1418073006/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp:83: if
(PDFCharIsLineEnding(ch) || ch == ' ' || ch == '\t')
How does this differ from the standard IsWhitespace test?

https://codereview.chromium.org/1418073006/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp:134: if
(PDFCharIsLineEnding(ch) || ch == ' ' || ch == '\t')
ditto

[dsinclair, 2015-10-28 18:05:07]

https://codereview.chromium.org/1418073006/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp (right):

https://codereview.chromium.org/1418073006/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp:83: if
(PDFCharIsLineEnding(ch) || ch == ' ' || ch == '\t')
On 2015/10/28 at 17:17:21, Tom Sepez wrote:
> How does this differ from the standard IsWhitespace test?

std::isspace is more restrictive in that it also checks for \v and \f. I'm not
sure if that's a valid addition to this logic or not.

[dsinclair, 2015-10-28 18:32:06]

https://codereview.chromium.org/1418073006/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp (right):

https://codereview.chromium.org/1418073006/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp:83: if
(PDFCharIsLineEnding(ch) || ch == ' ' || ch == '\t')
On 2015/10/28 at 18:05:07, dsinclair wrote:
> On 2015/10/28 at 17:17:21, Tom Sepez wrote:
> > How does this differ from the standard IsWhitespace test?
> 
> std::isspace is more restrictive in that it also checks for \v and \f. I'm not
sure if that's a valid addition to this logic or not.


For this one, it isn't spec compliant. The PDF spec says:

The ASCII85Decode filter shall ignore
all white-space characters (see 7.2, "Lexical Conventions"). 

Which 7.2 define as:

null 0x00
horizontal tab 0x09
line feed 0x0a
form feed 0x0c
carriage return 0x0d
space    0x20   


The std::isspace includes vertical tab 0x0b which isn't in the list of
whitespace chars in the PDF spec.

https://codereview.chromium.org/1418073006/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp:134: if
(PDFCharIsLineEnding(ch) || ch == ' ' || ch == '\t')
On 2015/10/28 at 17:17:21, Tom Sepez wrote:
> ditto


I think this is similar in that PDF doesn't list vertical tab as whitespace in
their spec.

[Tom Sepez, 2015-10-28 21:08:33]

lgtm

[dsinclair, 2015-10-28 21:14:46]

Committed patchset #2 (id:20001) manually as
4b38229b004b7b6493cb4c4507df450adc6085f6 (presubmit successful).