Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gitparse: Use an object for currentDiff #1573

Merged
merged 3 commits into from
Jul 31, 2023
Merged

gitparse: Use an object for currentDiff #1573

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8c033f7
gitparse: Use an object for currentDiff instead of a pointer
mcastorina Jul 28, 2023
c5f0708
gitparse: Use an object for currentCommit instead of a pointer
mcastorina Jul 28, 2023
31ab449
Revert "gitparse: Use an object for currentCommit instead of a pointer"
mcastorina Jul 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 12 additions & 15 deletions pkg/gitparse/gitparse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (c *Parser) FromReader(ctx context.Context, stdOut io.Reader, commitChan ch
outReader := bufio.NewReader(stdOut)
var (
currentCommit *Commit
currentDiff *Diff
currentDiff Diff

totalLogSize int
)
Expand All @@ -260,8 +260,8 @@ func (c *Parser) FromReader(ctx context.Context, stdOut io.Reader, commitChan ch
latestState = CommitLine

// If there is a currentDiff, add it to currentCommit.
if currentDiff != nil && currentDiff.Content.Len() > 0 {
currentCommit.Diffs = append(currentCommit.Diffs, *currentDiff)
if currentDiff.Content.Len() > 0 {
currentCommit.Diffs = append(currentCommit.Diffs, currentDiff)
currentCommit.Size += currentDiff.Content.Len()
}
// If there is a currentCommit, send it to the channel.
Expand All @@ -270,7 +270,7 @@ func (c *Parser) FromReader(ctx context.Context, stdOut io.Reader, commitChan ch
totalLogSize += currentCommit.Size
}
// Create a new currentDiff and currentCommit
currentDiff = &Diff{}
currentDiff = Diff{}
currentCommit = &Commit{
Message: strings.Builder{},
}
Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does making it an object allow us to avoid nil checking? is it just going to default to an empty struct?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. When its initialized it is an empty struct instead of a pointer to nil.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

@bill-rich bill-rich Jul 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I worded that poorly (wrong). Should have been "pointing to nil".

currentCommit.Diffs = append(currentCommit.Diffs, currentDiff)
// If the currentDiff is over 1GB, drop it into the channel so it isn't held in memory waiting for more commits.
totalSize := 0
for _, diff := range currentCommit.Diffs {
Expand All @@ -331,7 +331,7 @@ func (c *Parser) FromReader(ctx context.Context, stdOut io.Reader, commitChan ch
currentCommit.Message.WriteString(oldCommit.Message.String())
}
}
currentDiff = &Diff{}
currentDiff = Diff{}
case isModeLine(isStaged, latestState, line):
latestState = ModeLine
// NoOp
Expand All @@ -354,16 +354,13 @@ func (c *Parser) FromReader(ctx context.Context, stdOut io.Reader, commitChan ch
case isHunkLineNumberLine(isStaged, latestState, line):
latestState = HunkLineNumberLine

// TODO: Is it still necessary to check whether the currentDiff is nil?
if currentDiff != nil && currentDiff.Content.Len() > 0 {
currentCommit.Diffs = append(currentCommit.Diffs, *currentDiff)
if currentDiff.Content.Len() > 0 {
currentCommit.Diffs = append(currentCommit.Diffs, currentDiff)
}
newDiff := &Diff{
currentDiff = Diff{
PathB: currentDiff.PathB,
}

currentDiff = newDiff

words := bytes.Split(line, []byte(" "))
if len(words) >= 3 {
startSlice := bytes.Split(words[2], []byte(","))
Expand Down Expand Up @@ -420,14 +417,14 @@ func (c *Parser) FromReader(ctx context.Context, stdOut io.Reader, commitChan ch
latestState = ParseFailure
}

if currentDiff != nil && currentDiff.Content.Len() > c.maxDiffSize {
if currentDiff.Content.Len() > c.maxDiffSize {
ctx.Logger().V(2).Info(fmt.Sprintf(
"Diff for %s exceeded MaxDiffSize(%d)", currentDiff.PathB, c.maxDiffSize,
))
break
}
}
cleanupParse(currentCommit, currentDiff, commitChan, &totalLogSize)
cleanupParse(currentCommit, &currentDiff, commitChan, &totalLogSize)

ctx.Logger().V(2).Info("finished parsing git log.", "total_log_size", totalLogSize)
}
Expand Down
Loading