Skip to content

Commit 4e8b7b5

Browse files
committed
Fix no newline handling in fragment parser
Correctly process "\ No newline..." marker lines when parsing fragments, triming the new line from the last read line and advancing the parser. Also fix the text chunk parser to follow the invariant of not advancing past the end of the object in the event of an error. The error in question now has a correct line number as well.
1 parent 2ca8340 commit 4e8b7b5

File tree

1 file changed

+34
-9
lines changed

1 file changed

+34
-9
lines changed

gitdiff/parser.go

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,14 @@ func (p *parser) ParseTextChunk(frag *Fragment) error {
236236
return p.Errorf(0, "no content following fragment header")
237237
}
238238

239+
isNoNewlineLine := func(s string) bool {
240+
// test for "\ No newline at end of file" by prefix because the text
241+
// changes by locale (git claims all versions are at least 12 chars)
242+
return len(s) >= 12 && s[:2] == "\\ "
243+
}
244+
239245
oldLines, newLines := frag.OldLines, frag.NewLines
240-
for oldLines > 0 || newLines > 0 {
246+
for {
241247
line := p.Line(0)
242248
switch line[0] {
243249
case '\n':
@@ -252,21 +258,28 @@ func (p *parser) ParseTextChunk(frag *Fragment) error {
252258
}
253259
frag.Lines = append(frag.Lines, FragmentLine{OpContext, line[1:]})
254260
case '-':
255-
frag.LinesDeleted++
256261
oldLines--
262+
frag.LinesDeleted++
263+
frag.TrailingContext = 0
257264
frag.Lines = append(frag.Lines, FragmentLine{OpDelete, line[1:]})
258265
case '+':
259-
frag.LinesAdded++
260266
newLines--
267+
frag.LinesAdded++
268+
frag.TrailingContext = 0
261269
frag.Lines = append(frag.Lines, FragmentLine{OpAdd, line[1:]})
262270
default:
263-
// this could be "\ No newline at end of file", which we allow
264-
// only check the prefix because the text changes by locale
265-
// git also asserts that any translation is at least 12 characters
266-
if len(line) >= 12 && strings.HasPrefix("\\ ", line) {
271+
// this may appear in middle of fragment if it's for a deleted line
272+
if isNoNewlineLine(line) {
273+
last := len(frag.Lines) - 1
274+
frag.Lines[last].Line = strings.TrimSuffix(frag.Lines[last].Line, "\n")
267275
break
268276
}
269-
return p.Errorf(0, "invalid fragment line")
277+
return p.Errorf(0, "invalid line operation: %q", line[0])
278+
}
279+
280+
next := p.Line(1)
281+
if oldLines <= 0 && newLines <= 0 && !isNoNewlineLine(next) {
282+
break
270283
}
271284

272285
if err := p.Next(); err != nil {
@@ -278,7 +291,12 @@ func (p *parser) ParseTextChunk(frag *Fragment) error {
278291
}
279292

280293
if oldLines != 0 || newLines != 0 {
281-
return p.Errorf(0, "invalid fragment: remaining lines: %d old, %d new", oldLines, newLines)
294+
hdr := max(frag.OldLines-oldLines, frag.NewLines-newLines) + 1
295+
return p.Errorf(-hdr, "fragment header miscounts lines: %+d old, %+d new", -oldLines, -newLines)
296+
}
297+
298+
if err := p.Next(); err != nil && err != io.EOF {
299+
return err
282300
}
283301
return nil
284302
}
@@ -306,3 +324,10 @@ func parseRange(s string) (start int64, end int64, err error) {
306324

307325
return
308326
}
327+
328+
func max(a, b int64) int64 {
329+
if a > b {
330+
return a
331+
}
332+
return b
333+
}

0 commit comments

Comments
 (0)