Skip to content

Commit 2bab049

Browse files
a1012112796a1012112796
committed
enhancement of compare feature
splited from #25322 - split branch/tags dropdownas two parts: repository dropdown and branch/tags dropdown. that's same with github. - user can search any repository in repository search box. - show a single error log instead of a 404 page when the head repo or user, the head or base ref is not exist. then user still can choose other option easyly. Signed-off-by: a1012112796 <[email protected]>
1 parent 85c59d6 commit 2bab049

File tree

7 files changed

+300
-195
lines changed

7 files changed

+300
-195
lines changed

‎options/locale/locale_en-US.ini

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,6 +1192,7 @@ branch = Branch
11921192
tree = Tree
11931193
clear_ref = `Clear current reference`
11941194
filter_branch_and_tag = Filter branch or tag
1195+
filter_repo = Search repository
11951196
find_tag = Find tag
11961197
branches = Branches
11971198
tags = Tags
@@ -1737,6 +1738,9 @@ issues.reference_link = Reference: %s
17371738

17381739
compare.compare_base = base
17391740
compare.compare_head = compare
1741+
compare.titile = Comparing %s
1742+
compare.refs_not_exist = Head or base ref is not exist.
1743+
compare.head_info_not_exist = Head user or repository is not exist.
17401744

17411745
pulls.desc = Enable pull requests and code reviews.
17421746
pulls.new = New Pull Request

‎routers/web/repo/compare.go

Lines changed: 156 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,10 @@ type CompareInfo struct {
194194
BaseBranch string
195195
HeadBranch string
196196
DirectComparison bool
197+
RefsNotExist bool
198+
HeadInfoNotExist bool
199+
HeadRef string
200+
BaseRef string
197201
}
198202

199203
// ParseCompareInfo parse compare info between two commit for preparing comparing references
@@ -251,6 +255,9 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
251255
}
252256
}
253257

258+
ci.HeadRef = infos[1]
259+
ci.BaseRef = infos[0]
260+
254261
ctx.Data["BaseName"] = baseRepo.OwnerName
255262
ci.BaseBranch = infos[0]
256263
ctx.Data["BaseBranch"] = ci.BaseBranch
@@ -261,44 +268,56 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
261268
isSameRepo = true
262269
ci.HeadUser = ctx.Repo.Owner
263270
ci.HeadBranch = headInfos[0]
271+
ctx.Data["HeadUserName"] = ctx.Repo.Owner.Name
264272
} else if len(headInfos) == 2 {
265273
headInfosSplit := strings.Split(headInfos[0], "/")
266274
if len(headInfosSplit) == 1 {
275+
ctx.Data["HeadUserName"] = headInfos[0]
276+
ctx.Data["HeadRepoName"] = ""
277+
ci.HeadBranch = headInfos[1]
278+
267279
ci.HeadUser, err = user_model.GetUserByName(ctx, headInfos[0])
268280
if err != nil {
269281
if user_model.IsErrUserNotExist(err) {
270-
ctx.NotFound("GetUserByName", nil)
282+
ci.HeadInfoNotExist = true
283+
ci.RefsNotExist = true
271284
} else {
272285
ctx.ServerError("GetUserByName", err)
286+
return nil
287+
}
288+
} else {
289+
ci.HeadBranch = headInfos[1]
290+
isSameRepo = ci.HeadUser.ID == ctx.Repo.Owner.ID
291+
if isSameRepo {
292+
ci.HeadRepo = baseRepo
273293
}
274-
return nil
275-
}
276-
ci.HeadBranch = headInfos[1]
277-
isSameRepo = ci.HeadUser.ID == ctx.Repo.Owner.ID
278-
if isSameRepo {
279-
ci.HeadRepo = baseRepo
280294
}
281295
} else {
296+
ctx.Data["HeadUserName"] = headInfosSplit[0]
297+
ctx.Data["HeadRepoName"] = headInfosSplit[1]
298+
ci.HeadBranch = headInfos[1]
299+
282300
ci.HeadRepo, err = repo_model.GetRepositoryByOwnerAndName(ctx, headInfosSplit[0], headInfosSplit[1])
283301
if err != nil {
284302
if repo_model.IsErrRepoNotExist(err) {
285-
ctx.NotFound("GetRepositoryByOwnerAndName", nil)
303+
ci.HeadInfoNotExist = true
304+
ci.RefsNotExist = true
286305
} else {
287306
ctx.ServerError("GetRepositoryByOwnerAndName", err)
307+
return nil
288308
}
289-
return nil
290-
}
291-
if err := ci.HeadRepo.LoadOwner(ctx); err != nil {
292-
if user_model.IsErrUserNotExist(err) {
293-
ctx.NotFound("GetUserByName", nil)
294-
} else {
295-
ctx.ServerError("GetUserByName", err)
309+
} else {
310+
if err := ci.HeadRepo.LoadOwner(ctx); err != nil {
311+
if user_model.IsErrUserNotExist(err) {
312+
ctx.NotFound("GetUserByName", nil)
313+
} else {
314+
ctx.ServerError("GetUserByName", err)
315+
}
316+
return nil
296317
}
297-
return nil
318+
ci.HeadUser = ci.HeadRepo.Owner
319+
isSameRepo = ci.HeadRepo.ID == ctx.Repo.Repository.ID
298320
}
299-
ci.HeadBranch = headInfos[1]
300-
ci.HeadUser = ci.HeadRepo.Owner
301-
isSameRepo = ci.HeadRepo.ID == ctx.Repo.Repository.ID
302321
}
303322
} else {
304323
ctx.NotFound("CompareAndPullRequest", nil)
@@ -327,8 +346,7 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
327346
}
328347
return nil
329348
} else {
330-
ctx.NotFound("IsRefExist", nil)
331-
return nil
349+
ci.RefsNotExist = true
332350
}
333351
}
334352
ctx.Data["BaseIsCommit"] = baseIsCommit
@@ -373,6 +391,65 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
373391
}
374392
}
375393

394+
loadForkReps := func() *CompareInfo {
395+
// list all fork repos
396+
var (
397+
forks []*repo_model.Repository
398+
err error
399+
)
400+
401+
if rootRepo == nil {
402+
forks, err = repo_model.GetForks(ctx, baseRepo, db.ListOptions{
403+
Page: 0,
404+
PageSize: 20,
405+
})
406+
} else {
407+
forks, err = repo_model.GetForks(ctx, rootRepo, db.ListOptions{
408+
Page: 0,
409+
PageSize: 20,
410+
})
411+
}
412+
413+
if err != nil {
414+
ctx.ServerError("GetForks", err)
415+
return nil
416+
}
417+
418+
forkmap := make(map[int64]*repo_model.Repository)
419+
for _, fork := range forks {
420+
forkmap[fork.ID] = fork
421+
}
422+
423+
if _, ok := forkmap[baseRepo.ID]; !ok {
424+
forkmap[baseRepo.ID] = baseRepo
425+
}
426+
427+
if rootRepo != nil {
428+
if _, ok := forkmap[rootRepo.ID]; !ok {
429+
forkmap[rootRepo.ID] = rootRepo
430+
}
431+
}
432+
433+
if ownForkRepo != nil {
434+
if _, ok := forkmap[ownForkRepo.ID]; !ok {
435+
forkmap[ownForkRepo.ID] = ownForkRepo
436+
}
437+
}
438+
439+
forks = make([]*repo_model.Repository, 0, len(forkmap))
440+
for _, fork := range forkmap {
441+
forks = append(forks, fork)
442+
}
443+
444+
ctx.Data["CompareRepos"] = forks
445+
446+
return ci
447+
}
448+
449+
if ci.HeadInfoNotExist {
450+
return loadForkReps()
451+
}
452+
376453
has := ci.HeadRepo != nil
377454
// 3. If the base is a forked from "RootRepo" and the owner of
378455
// the "RootRepo" is the :headUser - set headRepo to that
@@ -423,6 +500,11 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
423500

424501
ctx.Data["HeadRepo"] = ci.HeadRepo
425502
ctx.Data["BaseCompareRepo"] = ctx.Repo.Repository
503+
ctx.Data["HeadRepoName"] = ci.HeadRepo.Name
504+
505+
if loadForkReps() == nil {
506+
return nil
507+
}
426508

427509
// Now we need to assert that the ctx.Doer has permission to read
428510
// the baseRepo's code and pulls
@@ -458,57 +540,15 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
458540
ci.HeadRepo,
459541
permHead)
460542
}
461-
ctx.NotFound("ParseCompareInfo", nil)
462-
return nil
543+
ci.HeadInfoNotExist = true
544+
ci.RefsNotExist = true
545+
return ci
463546
}
464547
ctx.Data["CanWriteToHeadRepo"] = permHead.CanWrite(unit.TypeCode)
465548
}
466549

467-
// If we have a rootRepo and it's different from:
468-
// 1. the computed base
469-
// 2. the computed head
470-
// then get the branches of it
471-
if rootRepo != nil &&
472-
rootRepo.ID != ci.HeadRepo.ID &&
473-
rootRepo.ID != baseRepo.ID {
474-
canRead := access_model.CheckRepoUnitUser(ctx, rootRepo, ctx.Doer, unit.TypeCode)
475-
if canRead {
476-
ctx.Data["RootRepo"] = rootRepo
477-
if !fileOnly {
478-
branches, tags, err := getBranchesAndTagsForRepo(ctx, rootRepo)
479-
if err != nil {
480-
ctx.ServerError("GetBranchesForRepo", err)
481-
return nil
482-
}
483-
484-
ctx.Data["RootRepoBranches"] = branches
485-
ctx.Data["RootRepoTags"] = tags
486-
}
487-
}
488-
}
489-
490-
// If we have a ownForkRepo and it's different from:
491-
// 1. The computed base
492-
// 2. The computed head
493-
// 3. The rootRepo (if we have one)
494-
// then get the branches from it.
495-
if ownForkRepo != nil &&
496-
ownForkRepo.ID != ci.HeadRepo.ID &&
497-
ownForkRepo.ID != baseRepo.ID &&
498-
(rootRepo == nil || ownForkRepo.ID != rootRepo.ID) {
499-
canRead := access_model.CheckRepoUnitUser(ctx, ownForkRepo, ctx.Doer, unit.TypeCode)
500-
if canRead {
501-
ctx.Data["OwnForkRepo"] = ownForkRepo
502-
if !fileOnly {
503-
branches, tags, err := getBranchesAndTagsForRepo(ctx, ownForkRepo)
504-
if err != nil {
505-
ctx.ServerError("GetBranchesForRepo", err)
506-
return nil
507-
}
508-
ctx.Data["OwnForkRepoBranches"] = branches
509-
ctx.Data["OwnForkRepoTags"] = tags
510-
}
511-
}
550+
if ci.RefsNotExist {
551+
return ci
512552
}
513553

514554
// Check if head branch is valid.
@@ -522,10 +562,11 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
522562
ctx.Data["HeadBranch"] = ci.HeadBranch
523563
headIsCommit = true
524564
} else {
525-
ctx.NotFound("IsRefExist", nil)
526-
return nil
565+
ci.RefsNotExist = true
566+
return ci
527567
}
528568
}
569+
529570
ctx.Data["HeadIsCommit"] = headIsCommit
530571
ctx.Data["HeadIsBranch"] = headIsBranch
531572
ctx.Data["HeadIsTag"] = headIsTag
@@ -713,6 +754,34 @@ func getBranchesAndTagsForRepo(ctx gocontext.Context, repo *repo_model.Repositor
713754
return branches, tags, nil
714755
}
715756

757+
func prepareHeadBranchAndTags(ctx *context.Context, headRepoID int64) {
758+
headBranches, err := git_model.FindBranchNames(ctx, git_model.FindBranchOptions{
759+
RepoID: headRepoID,
760+
ListOptions: db.ListOptions{
761+
ListAll: true,
762+
},
763+
IsDeletedBranch: optional.Some(false),
764+
})
765+
if err != nil {
766+
ctx.ServerError("GetBranches", err)
767+
return
768+
}
769+
ctx.Data["HeadBranches"] = headBranches
770+
771+
// For compare repo branches
772+
PrepareBranchList(ctx)
773+
if ctx.Written() {
774+
return
775+
}
776+
777+
headTags, err := repo_model.GetTagNamesByRepoID(ctx, headRepoID)
778+
if err != nil {
779+
ctx.ServerError("GetTagNamesByRepoID", err)
780+
return
781+
}
782+
ctx.Data["HeadTags"] = headTags
783+
}
784+
716785
// CompareDiff show different from one commit to another commit
717786
func CompareDiff(ctx *context.Context) {
718787
ci := ParseCompareInfo(ctx)
@@ -733,13 +802,19 @@ func CompareDiff(ctx *context.Context) {
733802
ctx.Data["CompareSeparator"] = ".."
734803
ctx.Data["OtherCompareSeparator"] = "..."
735804
}
805+
ctx.Data["CompareRefsNotFound"] = ci.RefsNotExist
806+
ctx.Data["HeadInfoNotExist"] = ci.HeadInfoNotExist
736807

737-
nothingToCompare := PrepareCompareDiff(ctx, ci,
738-
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
739-
if ctx.Written() {
740-
return
808+
var nothingToCompare bool
809+
if ci.RefsNotExist {
810+
nothingToCompare = true
811+
} else {
812+
nothingToCompare = PrepareCompareDiff(ctx, ci,
813+
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
814+
if ctx.Written() {
815+
return
816+
}
741817
}
742-
743818
baseTags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID)
744819
if err != nil {
745820
ctx.ServerError("GetTagNamesByRepoID", err)
@@ -753,31 +828,12 @@ func CompareDiff(ctx *context.Context) {
753828
return
754829
}
755830

756-
headBranches, err := git_model.FindBranchNames(ctx, git_model.FindBranchOptions{
757-
RepoID: ci.HeadRepo.ID,
758-
ListOptions: db.ListOptions{
759-
ListAll: true,
760-
},
761-
IsDeletedBranch: optional.Some(false),
762-
})
763-
if err != nil {
764-
ctx.ServerError("GetBranches", err)
765-
return
766-
}
767-
ctx.Data["HeadBranches"] = headBranches
768-
769-
// For compare repo branches
770-
PrepareBranchList(ctx)
771-
if ctx.Written() {
772-
return
773-
}
774-
775-
headTags, err := repo_model.GetTagNamesByRepoID(ctx, ci.HeadRepo.ID)
776-
if err != nil {
777-
ctx.ServerError("GetTagNamesByRepoID", err)
778-
return
831+
if !ci.HeadInfoNotExist {
832+
prepareHeadBranchAndTags(ctx, ci.HeadRepo.ID)
833+
if ctx.Written() {
834+
return
835+
}
779836
}
780-
ctx.Data["HeadTags"] = headTags
781837

782838
if ctx.Data["PageIsComparePull"] == true {
783839
pr, err := issues_model.GetUnmergedPullRequest(ctx, ci.HeadRepo.ID, ctx.Repo.Repository.ID, ci.HeadBranch, ci.BaseBranch, issues_model.PullRequestFlowGithub)
@@ -805,14 +861,12 @@ func CompareDiff(ctx *context.Context) {
805861
}
806862
}
807863
}
808-
beforeCommitID := ctx.Data["BeforeCommitID"].(string)
809-
afterCommitID := ctx.Data["AfterCommitID"].(string)
810864

811865
separator := "..."
812866
if ci.DirectComparison {
813867
separator = ".."
814868
}
815-
ctx.Data["Title"] = "Comparing " + base.ShortSha(beforeCommitID) + separator + base.ShortSha(afterCommitID)
869+
ctx.Data["Title"] = ctx.Tr("repo.compare.titile", ci.BaseRef+separator+ci.HeadRef)
816870

817871
ctx.Data["IsRepoToolbarCommits"] = true
818872
ctx.Data["IsDiffCompare"] = true

0 commit comments

Comments
 (0)