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

use IsClean to detect if changes need committing #446

Closed
wants to merge 1 commit into from
Closed

use IsClean to detect if changes need committing #446

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions git/gogit/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,15 +292,13 @@ func (g *Client) Commit(info git.Commit, commitOpts ...repository.CommitOption)
return "", err
}

var changed bool
for path, content := range options.Files {
if err := g.writeFile(path, content); err != nil {
return "", err
}
if _, err = wt.Add(path); err != nil {
return "", fmt.Errorf("cannot stage file %s: %w", path, err)
}
changed = true
}

status, err := wt.Status()
Expand All @@ -310,10 +308,14 @@ func (g *Client) Commit(info git.Commit, commitOpts ...repository.CommitOption)

for file := range status {
_, _ = wt.Add(file)
Copy link
Author

@jack-evans jack-evans Jan 6, 2023 •
edited

Choose a reason for hiding this comment

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

not sure I understand why we do this here as well as after calling g.writeFile in the other for loop but changed as minimal as I could

Copy link
Member

Choose a reason for hiding this comment

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

My understanding is that it stages files that were changed at the fs, whether or not they were passed on via options.Files.

Please note that line 309 does the same loop as status.IsClean():

func (s Status) IsClean() bool {
	for _, status := range s {
		if status.Worktree != Unmodified || status.Staging != Unmodified {
			return false
		}
	}

	return true
}

So to avoid going through twice over the statuses, we could just add the if status.Worktree != Unmodified || status.Staging != Unmodified in 310 here. However, as I mentioned on the other comment, this does not seem to be the problem, as by default go-git should not allow "empty commits".

changed = true
}

clean, err := status.IsClean()
if err != nil {
return "", err
}

if !changed {
if clean {
head, err := g.repository.Head()
if err != nil {
return "", err
Expand Down