If a patch is passed through a system that converts line endings to
'\r\n', mode lines end up with trailing whitespace that confuses
strconv.ParseInt. In Git, this is avoided by using strtoul() for parsing,
which stops at the first non-digit character.
Changing line endings in patch content itself will cause other problems
so it is best to avoid this transform, but if it does happen, it
shouldn't cause a parse error.
Git does not quote file names containing spaces when generating header
lines. However, I misread the Git implementation and thought it used
spaces as a name terminator by default, implying that names with spaces
would be quoted.
Rewrite the the name parsing code to better match Git. Specifically,
when both names in a header are not quoted, test all splits to see if
any of them produce two equal names. Also account for spaces in file
names when parsing file metadata.
Finally, allow trailing spaces on file names, which seem to be allowed
by Git (although this sounds like a terrible idea to me.)
This should almost always be handled specially, so don't let tests that
expect errors hide the fact that they returned an io.EOF. This could be
revisited if the tests are ever updated to check for specific errors.
To make output better, also add some (temporary?) String() functions and
fix an assumption about the smallest fragment header I discovered was
wrong while looking at sample patches.
After considering fragment parsing, it made sense to change the
invariant estabilished in the previous commit. Specifically, parser
functions now assume they are call on the first line of their object and
return with the parser on the first line after their object. This means
code can call parse function immediately after each other without
advancing the parser in between.
It's possible this will change back later on... there seem to be
annoying edge cases with either choice, but I think making the functions
consistent is important.
After parsing a valid header, the current line of the parser should be
the last line of the header. If the header is invalid (the parse
function returns an error), the state of the parser is undefined.
Also add a test since this is easy to mess up.
Instead of checking that a line has a certain prefix before calling a
header parsing function, the functions now check this and return nil
objects when called on the wrong line type. If the line passes this
basic check but is still invalid, and error is returned as before.
At the moment, we need only to read three lines, but now the value is
easy to adjust as needed. I've only seen the Git implementation read two
lines ahead so far, but it has the whole input in memory and may read
more in other places.
Call Next() to advance the parser state until it returns a non-nil
error, then check if the error is io.EOF. This makes EOF handling easier
and also means that Line() and PeekLine() can be called multiple times
without changing state.
The next step is to update parse functions to return an internal marker
error if they are called on an invalid line. This should improve
correctness and remove the duplication of testing a condition and then
calling a parse function, which checks the same condition.
This avoids including the newline character as part of the name when it
is the last name on a line. Also split out functions for quoted and
unquoted names to clarify the logic.
The parsed OIDs are usually prefixes, not full hashes, so rename the
fields appropriately. Also note that values that are too long to be
valid OIDs are still accepted by the library.
Billy Keyes