Merge pull request #416 from compressed/compare_commits

Add commit compare functionality
2014-09-01 20:02:34 -07:00 · 2014-09-01 20:02:34 -07:00 · 85f375433d
parent d55c5b9e28 af0741da07
commit 85f375433d
11 changed files with 178 additions and 66 deletions
--- a/cmd/web.go
+++ b/cmd/web.go
@ -342,6 +342,7 @@ func runWeb(*cli.Context) {
 		r.Get("/commit/:branchname/*", repo.Diff)
 		r.Get("/releases", repo.Releases)
 		r.Get("/archive/*.*", repo.Download)
+		r.Get("/compare/:before([a-z0-9]+)...:after([a-z0-9]+)", repo.CompareDiff)
 	}, ignSignIn, middleware.RepoAssignment(true, true))

 	m.Group("/:username", func(r *macaron.Router) {
--- a/models/action.go
+++ b/models/action.go
@ -172,7 +172,7 @@ func updateIssuesCommit(userId, repoId int64, repoUserName, repoName string, com

 // CommitRepoAction adds new action for committing repository.
 func CommitRepoAction(userId, repoUserId int64, userName, actEmail string,
-	repoId int64, repoUserName, repoName string, refFullName string, commit *base.PushCommits) error {
+	repoId int64, repoUserName, repoName string, refFullName string, commit *base.PushCommits, oldCommitId string, newCommitId string) error {

 	opType := COMMIT_REPO
 	// Check it's tag push or branch.
@ -226,6 +226,11 @@ func CommitRepoAction(userId, repoUserId int64, userName, actEmail string,
 	}

 	repoLink := fmt.Sprintf("%s%s/%s", setting.AppUrl, repoUserName, repoName)
+	compareUrl := ""
+	// if not the first commit, set the compareUrl
+	if !strings.HasPrefix(oldCommitId, "0000000") {
+		compareUrl = fmt.Sprintf("%s/compare/%s...%s", repoLink, oldCommitId, newCommitId)
+	}
 	commits := make([]*PayloadCommit, len(commit.Commits))
 	for i, cmt := range commit.Commits {
 		commits[i] = &PayloadCommit{
@ -258,6 +263,9 @@ func CommitRepoAction(userId, repoUserId int64, userName, actEmail string,
 			Name:  repo.Owner.LowerName,
 			Email: repo.Owner.Email,
 		},
+		Before:     oldCommitId,
+		After:      newCommitId,
+		CompareUrl: compareUrl,
 	}

 	for _, w := range ws {
--- a/models/git_diff.go
+++ b/models/git_diff.go
@ -175,25 +175,30 @@ func ParsePatch(pid int64, cmd *exec.Cmd, reader io.Reader) (*Diff, error) {
 	return diff, nil
 }

-func GetDiff(repoPath, commitid string) (*Diff, error) {
+func GetDiffRange(repoPath, beforeCommitId string, afterCommitId string) (*Diff, error) {
 	repo, err := git.OpenRepository(repoPath)
 	if err != nil {
 		return nil, err
 	}

-	commit, err := repo.GetCommit(commitid)
+	commit, err := repo.GetCommit(afterCommitId)
 	if err != nil {
 		return nil, err
 	}

 	rd, wr := io.Pipe()
 	var cmd *exec.Cmd
-	// First commit of repository.
-	if commit.ParentCount() == 0 {
-		cmd = exec.Command("git", "show", commitid)
+	// if "after" commit given
+	if beforeCommitId == "" {
+		// First commit of repository.
+		if commit.ParentCount() == 0 {
+			cmd = exec.Command("git", "show", afterCommitId)
+		} else {
+			c, _ := commit.Parent(0)
+			cmd = exec.Command("git", "diff", c.Id.String(), afterCommitId)
+		}
 	} else {
-		c, _ := commit.Parent(0)
-		cmd = exec.Command("git", "diff", c.Id.String(), commitid)
+		cmd = exec.Command("git", "diff", beforeCommitId, afterCommitId)
 	}
 	cmd.Dir = repoPath
 	cmd.Stdout = wr
@ -208,7 +213,7 @@ func GetDiff(repoPath, commitid string) (*Diff, error) {
 	}()
 	defer rd.Close()

-	desc := fmt.Sprintf("GetDiff(%s)", repoPath)
+	desc := fmt.Sprintf("GetDiffRange(%s)", repoPath)
 	pid := process.Add(desc, cmd)
 	go func() {
 		// In case process became zombie.
@ -226,3 +231,7 @@ func GetDiff(repoPath, commitid string) (*Diff, error) {

 	return ParsePatch(pid, cmd, rd)
 }
+
+func GetDiffCommit(repoPath, commitId string) (*Diff, error) {
+	return GetDiffRange(repoPath, "", commitId)
+}
--- a/models/slack.go
+++ b/models/slack.go
@ -70,19 +70,26 @@ func getSlackPushPayload(p *Payload, slack *Slack) (*SlackPayload, error) {
 	branchName := refSplit[len(refSplit)-1]
 	var commitString string

-	// TODO: add commit compare before/after link when gogs adds it
 	if len(p.Commits) == 1 {
 		commitString = "1 new commit"
+		if p.CompareUrl != "" {
+			commitString = SlackLinkFormatter(p.CompareUrl, commitString)
+		}
 	} else {
 		commitString = fmt.Sprintf("%d new commits", len(p.Commits))
+		if p.CompareUrl != "" {
+			commitString = SlackLinkFormatter(p.CompareUrl, commitString)
+		}
 	}

-	text := fmt.Sprintf("[%s:%s] %s pushed by %s", p.Repo.Name, branchName, commitString, p.Pusher.Name)
+	repoLink := SlackLinkFormatter(p.Repo.Url, p.Repo.Name)
+	branchLink := SlackLinkFormatter(p.Repo.Url+"/src/"+branchName, branchName)
+	text := fmt.Sprintf("[%s:%s] %s pushed by %s", repoLink, branchLink, commitString, p.Pusher.Name)
 	var attachmentText string

 	// for each commit, generate attachment text
 	for i, commit := range p.Commits {
-		attachmentText += fmt.Sprintf("<%s|%s>: %s - %s", commit.Url, commit.Id[:7], SlackFormatter(commit.Message), commit.Author.Name)
+		attachmentText += fmt.Sprintf("%s: %s - %s", SlackLinkFormatter(commit.Url, commit.Id[:7]), SlackTextFormatter(commit.Message), SlackTextFormatter(commit.Author.Name))
 		// add linebreak to each commit but the last
 		if i < len(p.Commits)-1 {
 			attachmentText += "\n"
@ -103,7 +110,7 @@ func getSlackPushPayload(p *Payload, slack *Slack) (*SlackPayload, error) {
 }

 // see: https://api.slack.com/docs/formatting
-func SlackFormatter(s string) string {
+func SlackTextFormatter(s string) string {
 	// take only first line of commit
 	first := strings.Split(s, "\n")[0]
 	// replace & < >
@ -112,3 +119,7 @@ func SlackFormatter(s string) string {
 	first = strings.Replace(first, ">", "&gt;", -1)
 	return first
 }
+
+func SlackLinkFormatter(url string, text string) string {
+	return fmt.Sprintf("<%s|%s>", url, SlackTextFormatter(text))
+}
--- a/models/update.go
+++ b/models/update.go
@ -101,7 +101,7 @@ func Update(refName, oldCommitId, newCommitId, userName, repoUserName, repoName
 		commit := &base.PushCommits{}

 		if err = CommitRepoAction(userId, ru.Id, userName, actEmail,
-			repos.Id, repoUserName, repoName, refName, commit); err != nil {
+			repos.Id, repoUserName, repoName, refName, commit, oldCommitId, newCommitId); err != nil {
 			log.GitLogger.Fatal(4, "runUpdate.models.CommitRepoAction: %s/%s:%v", repoUserName, repoName, err)
 		}
 		return err
@ -152,7 +152,7 @@ func Update(refName, oldCommitId, newCommitId, userName, repoUserName, repoName

 	//commits = append(commits, []string{lastCommit.Id().String(), lastCommit.Message()})
 	if err = CommitRepoAction(userId, ru.Id, userName, actEmail,
-		repos.Id, repoUserName, repoName, refName, &base.PushCommits{l.Len(), commits}); err != nil {
+		repos.Id, repoUserName, repoName, refName, &base.PushCommits{l.Len(), commits}, oldCommitId, newCommitId); err != nil {
 		return fmt.Errorf("runUpdate.models.CommitRepoAction: %s/%s:%v", repoUserName, repoName, err)
 	}
 	return nil
--- a/models/webhook.go
+++ b/models/webhook.go
@ -169,11 +169,14 @@ type BasePayload interface {

 // Payload represents a payload information of hook.
 type Payload struct {
-	Secret  string           `json:"secret"`
-	Ref     string           `json:"ref"`
-	Commits []*PayloadCommit `json:"commits"`
-	Repo    *PayloadRepo     `json:"repository"`
-	Pusher  *PayloadAuthor   `json:"pusher"`
+	Secret     string           `json:"secret"`
+	Ref        string           `json:"ref"`
+	Commits    []*PayloadCommit `json:"commits"`
+	Repo       *PayloadRepo     `json:"repository"`
+	Pusher     *PayloadAuthor   `json:"pusher"`
+	Before     string           `json:"before"`
+	After      string           `json:"after"`
+	CompareUrl string           `json:"compare_url"`
 }

 func (p Payload) GetJSONPayload() ([]byte, error) {
--- a/public/css/gogs.css
+++ b/public/css/gogs.css
@ -968,6 +968,13 @@ body {
 .guide-box .zclip {
    left: auto !important;
 }
+div.compare div#commits {
+    margin-top: 5px;
+}
+div.compare div#commits h4 {
+  margin: 10px 0;
+  line-height: 1.1;
+}
 .diff-head-box h4 {
    margin-top: 0;
    margin-bottom: 0;
--- a/routers/repo/commit.go
+++ b/routers/repo/commit.go
@ -114,9 +114,9 @@ func Diff(ctx *middleware.Context) {

 	commit := ctx.Repo.Commit

-	diff, err := models.GetDiff(models.RepoPath(userName, repoName), commitId)
+	diff, err := models.GetDiffCommit(models.RepoPath(userName, repoName), commitId)
 	if err != nil {
-		ctx.Handle(404, "GetDiff", err)
+		ctx.Handle(404, "GetDiffCommit", err)
 		return
 	}

@ -162,6 +162,67 @@ func Diff(ctx *middleware.Context) {
 	ctx.HTML(200, DIFF)
 }

+func CompareDiff(ctx *middleware.Context) {
+	ctx.Data["IsRepoToolbarCommits"] = true
+	ctx.Data["IsDiffCompare"] = true
+	userName := ctx.Repo.Owner.Name
+	repoName := ctx.Repo.Repository.Name
+	beforeCommitId := ctx.Params(":before")
+	afterCommitId := ctx.Params(":after")
+
+	commit, err := ctx.Repo.GitRepo.GetCommit(afterCommitId)
+	if err != nil {
+		ctx.Handle(404, "GetCommit", err)
+		return
+	}
+
+	diff, err := models.GetDiffRange(models.RepoPath(userName, repoName), beforeCommitId, afterCommitId)
+	if err != nil {
+		ctx.Handle(404, "GetDiffRange", err)
+		return
+	}
+
+	isImageFile := func(name string) bool {
+		blob, err := commit.GetBlobByPath(name)
+		if err != nil {
+			return false
+		}
+
+		dataRc, err := blob.Data()
+		if err != nil {
+			return false
+		}
+		buf := make([]byte, 1024)
+		n, _ := dataRc.Read(buf)
+		if n > 0 {
+			buf = buf[:n]
+		}
+		_, isImage := base.IsImageFile(buf)
+		return isImage
+	}
+
+	commits, err := commit.CommitsBeforeUntil(beforeCommitId)
+	if err != nil {
+		ctx.Handle(500, "CommitsBeforeUntil", err)
+		return
+	}
+
+	ctx.Data["Commits"] = commits
+	ctx.Data["CommitCount"] = commits.Len()
+	ctx.Data["BeforeCommitId"] = beforeCommitId
+	ctx.Data["AfterCommitId"] = afterCommitId
+	ctx.Data["Username"] = userName
+	ctx.Data["Reponame"] = repoName
+	ctx.Data["IsImageFile"] = isImageFile
+	ctx.Data["Title"] = "Comparing " + base.ShortSha(beforeCommitId) + "..." + base.ShortSha(afterCommitId) + " · " + userName + "/" + repoName
+	ctx.Data["Commit"] = commit
+	ctx.Data["Diff"] = diff
+	ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0
+	ctx.Data["SourcePath"] = "/" + path.Join(userName, repoName, "src", afterCommitId)
+	ctx.Data["RawPath"] = "/" + path.Join(userName, repoName, "raw", afterCommitId)
+	ctx.HTML(200, DIFF)
+}
+
 func FileHistory(ctx *middleware.Context) {
 	ctx.Data["IsRepoToolbarCommits"] = true

--- a/templates/repo/commits.tmpl
+++ b/templates/repo/commits.tmpl
@ -3,47 +3,6 @@
 {{template "repo/nav" .}}
 {{template "repo/toolbar" .}}
 <div id="body" class="container">
-    <div id="commits">
-        <div class="panel panel-default commit-box info-box">
-            <div class="panel-heading info-head">
-                <form class="search pull-right col-md-3" action="{{.RepoLink}}/commits/{{.BranchName}}/search" method="get" id="commits-search-form">
-                    <div class="input-group">
-                        <input class="form-control search" type="search" placeholder="search commit" name="q" value="{{.Keyword}}" />
-                        <div class="input-group-btn">
-                            <button type="submit" class="btn btn-default">Find</button>
-                        </div>
-                    </div>
-                </form>
-                <h4>{{.CommitCount}} Commits</h4>
-            </div>
-            <table class="panel-footer table commit-list table table-striped">
-                <thead>
-                    <tr>
-                        <th class="author">Author</th>
-                        <th class="sha">SHA1</th>
-                        <th class="message">Message</th>
-                        <th class="date">Date</th>
-                    </tr>
-                </thead>
-                <tbody>
-                {{ $username := .Username}}
-                {{ $reponame := .Reponame}}
-                {{$r := List .Commits}}
-                {{range $r}}
-                <tr>
-                    <td class="author"><img class="avatar" src="{{AvatarLink .Author.Email}}" alt=""/><a href="/user/email2user?email={{.Author.Email}}">{{.Author.Name}}</a></td>
-                    <td class="sha"><a rel="nofollow" class="label label-success" href="/{{$username}}/{{$reponame}}/commit/{{.Id}} ">{{SubStr .Id.String 0 10}} </a></td>
-                    <td class="message">{{.Summary}} </td>
-                    <td class="date">{{TimeSince .Author.When $.Lang}}</td>
-                </tr>
-                {{end}}
-                </tbody>
-            </table>
-        </div>
-        {{if not .IsSearchPage}}<ul class="pagination" id="commits-pager">
-            {{if .LastPageNum}}<li><a href="{{.RepoLink}}/commits/{{.BranchName}}{{if .FileName}}/{{.FileName}}{{end}}?p={{.LastPageNum}}" rel="nofollow">&laquo; Newer</a></li>{{end}}
-            {{if .NextPageNum}}<li><a href="{{.RepoLink}}/commits/{{.BranchName}}{{if .FileName}}/{{.FileName}}{{end}}?p={{.NextPageNum}}" rel="nofollow">&raquo; Older</a></li>{{end}}
-        </ul>{{end}}
-    </div>
+  {{template "repo/commits_table" .}}
 </div>
 {{template "base/footer" .}}
--- a/templates/repo/commits_table.tmpl
+++ b/templates/repo/commits_table.tmpl
@ -0,0 +1,42 @@
+<div id="commits">
+    <div class="panel panel-default commit-box info-box">
+        <div class="panel-heading info-head">
+            <form class="search pull-right col-md-3" action="{{.RepoLink}}/commits/{{.BranchName}}/search" method="get" id="commits-search-form">
+                <div class="input-group">
+                    <input class="form-control search" type="search" placeholder="search commit" name="q" value="{{.Keyword}}" />
+                    <div class="input-group-btn">
+                        <button type="submit" class="btn btn-default">Find</button>
+                    </div>
+                </div>
+            </form>
+            <h4>{{.CommitCount}} Commits</h4>
+        </div>
+        <table class="panel-footer table commit-list table table-striped">
+            <thead>
+                <tr>
+                    <th class="author">Author</th>
+                    <th class="sha">SHA1</th>
+                    <th class="message">Message</th>
+                    <th class="date">Date</th>
+                </tr>
+            </thead>
+            <tbody>
+            {{ $username := .Username}}
+            {{ $reponame := .Reponame}}
+            {{$r := List .Commits}}
+            {{range $r}}
+            <tr>
+                <td class="author"><img class="avatar" src="{{AvatarLink .Author.Email}}" alt=""/><a href="/user/email2user?email={{.Author.Email}}">{{.Author.Name}}</a></td>
+                <td class="sha"><a rel="nofollow" class="label label-success" href="/{{$username}}/{{$reponame}}/commit/{{.Id}} ">{{SubStr .Id.String 0 10}} </a></td>
+                <td class="message">{{.Summary}} </td>
+                <td class="date">{{TimeSince .Author.When $.Lang}}</td>
+            </tr>
+            {{end}}
+            </tbody>
+        </table>
+    </div>
+    {{if not .IsSearchPage}}<ul class="pagination" id="commits-pager">
+        {{if .LastPageNum}}<li><a href="{{.RepoLink}}/commits/{{.BranchName}}{{if .FileName}}/{{.FileName}}{{end}}?p={{.LastPageNum}}" rel="nofollow">&laquo; Newer</a></li>{{end}}
+        {{if .NextPageNum}}<li><a href="{{.RepoLink}}/commits/{{.BranchName}}{{if .FileName}}/{{.FileName}}{{end}}?p={{.NextPageNum}}" rel="nofollow">&raquo; Older</a></li>{{end}}
+    </ul>{{end}}
+</div>
--- a/templates/repo/diff.tmpl
+++ b/templates/repo/diff.tmpl
@ -3,7 +3,18 @@
 {{template "repo/nav" .}}
 <div id="body" class="container" data-page="repo">
    <div id="source">
+      {{if .IsDiffCompare }}
        <div class="panel panel-info diff-box diff-head-box">
+            <div class="panel-heading">
+                <a class="pull-right btn btn-primary btn-sm" rel="nofollow" href="{{.SourcePath}}">Browse Source</a>
+                <h4><a href="{{$.RepoLink}}/commit/{{.BeforeCommitId}}" class="label label-success">{{ShortSha .BeforeCommitId}}</a> ... <a href="{{$.RepoLink}}/commit/{{.AfterCommitId}}" class="label label-success">{{ShortSha .AfterCommitId}}</a></h4>
+            </div>
+            <div class="panel-body compare">
+              {{template "repo/commits_table" .}}
+            </div>
+        </div>
+      {{else}}
+          <div class="panel panel-info diff-box diff-head-box">
            <div class="panel-heading">
                <a class="pull-right btn btn-primary btn-sm" rel="nofollow" href="{{.SourcePath}}">Browse Source</a>
                <h4>{{.Commit.Message}}</h4>
@ -22,9 +33,9 @@
                    <a class="name" href="/user/email2user?email={{.Commit.Author.Email}}"><strong>{{.Commit.Author.Name}}</strong></a>
                    <span class="time">{{TimeSince .Commit.Author.When $.Lang}}</span>
                </p>
-            </div>
+          </div>
        </div>
-
+      {{end}}
        {{if .DiffNotAvailable}}
        <h4>Diff Data Not Available.</h4>
        {{else}}