gitparse: Use an object for currentDiff #1573
Conversation
This reverts commit c5f0708.
mcastorina
changed the title
| @@ -309,8 +309,8 @@ func (c *Parser) FromReader(ctx context.Context, stdOut io.Reader, commitChan ch | |||
| if currentCommit == nil { | |||
| currentCommit = &Commit{} | |||
| } | |||
| if currentDiff != nil && currentDiff.Content.Len() > 0 { | |||
| currentCommit.Diffs = append(currentCommit.Diffs, *currentDiff) | |||
| if currentDiff.Content.Len() > 0 { | |||
There was a problem hiding this comment.
why does making it an object allow us to avoid nil checking? is it just going to default to an empty struct?
There was a problem hiding this comment.
Yep. When its initialized it is an empty struct instead of a pointer to nil.
There was a problem hiding this comment.
In this case @bill-rich would it be a pointer to nil or a nil pointer? I think the latter?
This is my understanding:
var i *int = nil // i is a nil pointer
var p **int = &i // p is a pointer to a nil pointer || closest thing to pointer to nil lol
There was a problem hiding this comment.
You're right. I worded that poorly (wrong). Should have been "pointing to nil".
There was a problem hiding this comment.
this lgtm and i'm assuming it doesn't run afoul of @bill-rich's commentary re: pointers on #1570 (because he's commenting here without objecting) but maybe check with him if you haven't already
yep, chatted out-of-band and we were already making a copy when we dereferenced it to put in the channel. |
This is safer than relying on state to ensure we don't get a nil pointer dereference.