compare/pullrequest: introduce merge parameter

This is more correct and flexible than the old way of looking on same/different
repo.

rhodecode/controllers/compare.py

@@ -89,6 +89,15 b' class CompareController(BaseRepoControll'
         # other_ref will be evaluated in other_repo
         other_ref = (other_ref_type, other_ref)
         other_repo = request.GET.get('other_repo', org_repo)
+        # If merge is True:
+        #   Show what org would get if merged with other:
+        #   List changesets that are ancestors of other but not of org.
+        #   New changesets in org is thus ignored.
+        #   Diff will be from common ancestor, and merges of org to other will thus be ignored.
+        # If merge is False:
+        #   Make a raw diff from org to other, no matter if related or not.
+        #   Changesets in one and not in the other will be ignored
+        merge = bool(request.GET.get('merge'))
         # fulldiff disables cut_off_limit
         c.fulldiff = request.GET.get('fulldiff')
         # partial uses compare_cs.html template directly
@@ -100,7 +109,8 b' class CompareController(BaseRepoControll'
             repo_name=other_repo,
             org_ref_type=other_ref[0], org_ref=other_ref[1],
             other_repo=org_repo,
-            other_ref_type=org_ref[0], other_ref=org_ref[1])
+            other_ref_type=org_ref[0], other_ref=org_ref[1],
+            merge=merge or '')
 
         org_repo = Repository.get_by_repo_name(org_repo)
         other_repo = Repository.get_by_repo_name(other_repo)
@@ -130,22 +140,23 b' class CompareController(BaseRepoControll'
         c.org_ref_type = org_ref[0]
         c.other_ref_type = other_ref[0]
 
-        c.cs_ranges, ancestor = PullRequestModel().get_compare_data(
-            org_repo, org_ref, other_repo, other_ref)
+        c.cs_ranges, c.ancestor = PullRequestModel().get_compare_data(
+            org_repo, org_ref, other_repo, other_ref, merge)
 
         c.statuses = c.rhodecode_db_repo.statuses([x.raw_id for x in
                                                    c.cs_ranges])
         if partial:
+            assert c.ancestor
             return render('compare/compare_cs.html')
 
-        if ancestor and org_repo != other_repo:
+        if c.ancestor:
+            assert merge
             # case we want a simple diff without incoming changesets,
             # previewing what will be merged.
-            # Make the diff on the forked repo, with
-            # revision that is common ancestor
+            # Make the diff on the other repo (which is known to have other_ref)
             log.debug('Using ancestor %s as org_ref instead of %s'
-                      % (ancestor, org_ref))
-            org_ref = ('rev', ancestor)
+                      % (c.ancestor, org_ref))
+            org_ref = ('rev', c.ancestor)
             org_repo = other_repo
 
         diff_limit = self.cut_off_limit if not c.fulldiff else None

rhodecode/controllers/pullrequests.py

@@ -196,9 +196,9 b' class PullrequestsController(BaseRepoCon'
             return redirect(url('pullrequest_home', repo_name=repo_name))
 
         org_repo = _form['org_repo']
-        org_ref = _form['org_ref']
+        org_ref = 'rev:merge:%s' % _form['merge_rev']
         other_repo = _form['other_repo']
-        other_ref = _form['other_ref']
+        other_ref = 'rev:ancestor:%s' % _form['ancestor_rev']
         revisions = _form['revisions']
         reviewers = _form['review_members']
 
@@ -281,10 +281,6 b' class PullrequestsController(BaseRepoCon'
 
         c.cs_ranges = [org_repo.get_changeset(x) for x in pull_request.revisions]
 
-        other_ref = ('rev', getattr(c.cs_ranges[0].parents[0]
-                                  if c.cs_ranges[0].parents
-                                  else EmptyChangeset(), 'raw_id'))
-
         c.statuses = org_repo.statuses([x.raw_id for x in c.cs_ranges])
 
         c.org_ref = org_ref[1]
@@ -385,6 +381,7 b' class PullrequestsController(BaseRepoCon'
         c.changeset_statuses = ChangesetStatus.STATUSES
 
         c.as_form = False
+        c.ancestor = None # there is one - but right here we don't know which
         return render('/pullrequests/pullrequest_show.html')
 
     @NotAnonymous()

rhodecode/model/forms.py

@@ -395,4 +395,7 b' def PullRequestForm(repo_id):'
         pullrequest_title = v.UnicodeString(strip=True, required=True, min=3)
         pullrequest_desc = v.UnicodeString(strip=True, required=False)
 
+        ancestor_rev = v.UnicodeString(strip=True, required=True)
+        merge_rev = v.UnicodeString(strip=True, required=True)
+
     return _PullRequestForm

rhodecode/model/pull_request.py

@@ -161,7 +161,7 b' class PullRequestModel(BaseModel):'
         pull_request.updated_on = datetime.datetime.now()
         Session().add(pull_request)
 
-    def _get_changesets(self, alias, org_repo, org_ref, other_repo, other_ref):
+    def _get_changesets(self, alias, org_repo, org_ref, other_repo, other_ref, merge):
         """
         Returns a list of changesets that can be merged from org_repo@org_ref
         to other_repo@other_ref ... and the ancestor that would be used for merge
@@ -211,16 +211,21 b' class PullRequestModel(BaseModel):'
             else:
                 hgrepo = other_repo._repo
 
-            revs = ["ancestors(id('%s')) and not ancestors(id('%s'))" %
-                    (other_rev, org_rev)]
-            changesets = [other_repo.get_changeset(cs)
-                          for cs in scmutil.revrange(hgrepo, revs)]
+            if merge:
+                revs = ["ancestors(id('%s')) and not ancestors(id('%s')) and not id('%s')" %
+                        (other_rev, org_rev, org_rev)]
 
-            if org_repo != other_repo:
                 ancestors = scmutil.revrange(hgrepo,
                      ["ancestor(id('%s'), id('%s'))" % (org_rev, other_rev)])
                 if len(ancestors) == 1:
                     ancestor = hgrepo[ancestors[0]].hex()
+            else:
+                # TODO: have both + and - changesets
+                revs = ["id('%s') :: id('%s') - id('%s')" %
+                        (org_rev, other_rev, org_rev)]
+
+            changesets = [other_repo.get_changeset(cs)
+                          for cs in scmutil.revrange(hgrepo, revs)]
 
         elif alias == 'git':
             assert org_repo == other_repo, (org_repo, other_repo) # no git support for different repos
@@ -233,7 +238,7 b' class PullRequestModel(BaseModel):'
 
         return changesets, ancestor
 
-    def get_compare_data(self, org_repo, org_ref, other_repo, other_ref):
+    def get_compare_data(self, org_repo, org_ref, other_repo, other_ref, merge):
         """
         Returns incoming changesets for mercurial repositories
 
@@ -251,5 +256,6 b' class PullRequestModel(BaseModel):'
 
         cs_ranges, ancestor = self._get_changesets(org_repo.scm_instance.alias,
                                                    org_repo.scm_instance, org_ref,
-                                                   other_repo.scm_instance, other_ref)
+                                                   other_repo.scm_instance, other_ref,
+                                                   merge)
         return cs_ranges, ancestor

rhodecode/templates/changelog/changelog.html

@@ -37,7 +37,7 b''
                     <a href="#" class="ui-btn small" id="rev_range_clear" style="display:none">${_('Clear selection')}</a>
 
                     %if c.rhodecode_db_repo.fork:
-                        <a id="compare_fork" title="${_('Compare fork with %s' % c.rhodecode_db_repo.fork.repo_name)}" href="${h.url('compare_url',repo_name=c.rhodecode_db_repo.fork.repo_name,org_ref_type='branch',org_ref='default',other_repo=c.repo_name,other_ref_type='branch',other_ref=request.GET.get('branch') or 'default')}" class="ui-btn small">${_('Compare fork with parent')}</a>
+                        <a id="compare_fork" title="${_('Compare fork with %s' % c.rhodecode_db_repo.fork.repo_name)}" href="${h.url('compare_url',repo_name=c.rhodecode_db_repo.fork.repo_name,org_ref_type='branch',org_ref='default',other_repo=c.repo_name,other_ref_type='branch',other_ref=request.GET.get('branch') or 'default',merge=1)}" class="ui-btn small">${_('Compare fork with parent')}</a>
                     %endif
                     %if h.is_hg(c.rhodecode_repo):
                     <a id="open_new_pr" href="${h.url('pullrequest_home',repo_name=c.repo_name)}" class="ui-btn small">${_('Open new pull request')}</a>

rhodecode/templates/compare/compare_cs.html

@@ -1,9 +1,9 b''
 ## Changesets table !
 <div class="container">
-  <table class="compare_view_commits noborder">
   %if not c.cs_ranges:
     <span class="empty_data">${_('No changesets')}</span>
   %else:
+    <table class="compare_view_commits noborder">
     %for cnt, cs in enumerate(c.cs_ranges):
         <tr>
         <td><div class="gravatar"><img alt="gravatar" src="${h.gravatar_url(h.email_or_none(cs.author),14)}"/></div></td>
@@ -22,7 +22,15 b''
         <td><div class="message tooltip" title="${h.tooltip(cs.message)}" style="white-space:normal">${h.urlify_commit(h.shorter(cs.message, 60),c.repo_name)}</div></td>
         </tr>
     %endfor
-
+    </table>
+    %if c.ancestor:
+    <span class="ancestor">${_('Ancestor')}:
+      ${h.link_to(c.ancestor,h.url('changeset_home',repo_name=c.repo_name,revision=c.ancestor))}
+    </span>
+    %endif
+    %if c.as_form:
+      ${h.hidden('ancestor_rev',c.ancestor)}
+      ${h.hidden('merge_rev',c.cs_ranges[-1].raw_id)}
+    %endif
   %endif
-  </table>
 </div>

rhodecode/templates/pullrequests/pullrequest.html

@@ -133,6 +133,7 b''
                          other_ref_type='__org_ref_type__',
                          other_ref='__org_ref__',
                          as_form=True,
+                         merge=True,
                          )}";
       var org_repo = YUQ('#pull_request_form #org_repo')[0].value;
       var org_ref = YUQ('#pull_request_form #org_ref')[0].value.split(':');

rhodecode/tests/functional/test_compare.py

@@ -106,6 +106,7 b' class TestCompareController(TestControll'
                                     other_repo=repo2.repo_name,
                                     other_ref_type="branch",
                                     other_ref=rev1,
+                                    merge='1',
                                     ))
 
         response.mustcontain('%s@%s -> %s@%s' % (repo1.repo_name, rev2, repo2.repo_name, rev1))
@@ -118,9 +119,9 b' class TestCompareController(TestControll'
         response.mustcontain("""<a href="/%s/changeset/%s">r1:%s</a>""" % (repo2.repo_name, cs1.raw_id, cs1.short_id))
         response.mustcontain("""<a href="/%s/changeset/%s">r2:%s</a>""" % (repo2.repo_name, cs2.raw_id, cs2.short_id))
         ## files
-        response.mustcontain("""<a href="/%s/compare/branch@%s...branch@%s?other_repo=%s#C--826e8142e6ba">file1</a>""" % (repo1.repo_name, rev2, rev1, repo2.repo_name))
+        response.mustcontain("""<a href="/%s/compare/branch@%s...branch@%s?other_repo=%s&amp;merge=1#C--826e8142e6ba">file1</a>""" % (repo1.repo_name, rev2, rev1, repo2.repo_name))
         #swap
-        response.mustcontain("""<a href="/%s/compare/branch@%s...branch@%s?other_repo=%s">[swap]</a>""" % (repo2.repo_name, rev1, rev2, repo1.repo_name))
+        response.mustcontain("""<a href="/%s/compare/branch@%s...branch@%s?other_repo=%s&amp;merge=True">[swap]</a>""" % (repo2.repo_name, rev1, rev2, repo1.repo_name))
 
     def test_compare_forks_on_branch_extra_commits_origin_has_incomming_hg(self):
         self.log_user()
@@ -160,6 +161,7 b' class TestCompareController(TestControll'
                                     other_repo=repo2.repo_name,
                                     other_ref_type="branch",
                                     other_ref=rev1,
+                                    merge='x',
                                     ))
         response.mustcontain('%s@%s -&gt; %s@%s' % (repo1.repo_name, rev2, repo2.repo_name, rev1))
         response.mustcontain("""Showing 2 commits""")
@@ -171,9 +173,9 b' class TestCompareController(TestControll'
         response.mustcontain("""<a href="/%s/changeset/%s">r1:%s</a>""" % (repo2.repo_name, cs1.raw_id, cs1.short_id))
         response.mustcontain("""<a href="/%s/changeset/%s">r2:%s</a>""" % (repo2.repo_name, cs2.raw_id, cs2.short_id))
         ## files
-        response.mustcontain("""<a href="/%s/compare/branch@%s...branch@%s?other_repo=%s#C--826e8142e6ba">file1</a>""" % (repo1.repo_name, rev2, rev1, repo2.repo_name))
+        response.mustcontain("""<a href="/%s/compare/branch@%s...branch@%s?other_repo=%s&amp;merge=x#C--826e8142e6ba">file1</a>""" % (repo1.repo_name, rev2, rev1, repo2.repo_name))
         #swap
-        response.mustcontain("""<a href="/%s/compare/branch@%s...branch@%s?other_repo=%s">[swap]</a>""" % (repo2.repo_name, rev1, rev2, repo1.repo_name))
+        response.mustcontain("""<a href="/%s/compare/branch@%s...branch@%s?other_repo=%s&amp;merge=True">[swap]</a>""" % (repo2.repo_name, rev1, rev2, repo1.repo_name))
 
     def test_compare_cherry_pick_changesets_from_bottom(self):
 
@@ -222,6 +224,7 b' class TestCompareController(TestControll'
                                     other_repo=repo1.repo_name,
                                     other_ref_type="rev",
                                     other_ref=cs4.short_id,
+                                    merge='True',
                                     ))
         response.mustcontain('%s@%s -&gt; %s@%s' % (repo2.repo_name, cs1.short_id, repo1.repo_name, cs4.short_id))
         response.mustcontain("""Showing 3 commits""")
@@ -281,6 +284,7 b' class TestCompareController(TestControll'
                                     org_ref=cs2.short_id, # parent of cs3, not in repo2
                                     other_ref_type="rev",
                                     other_ref=cs5.short_id,
+                                    merge='1',
                                     ))
 
         response.mustcontain('%s@%s -&gt; %s@%s' % (repo1.repo_name, cs2.short_id, repo1.repo_name, cs5.short_id))
@@ -319,6 +323,7 b' class TestCompareController(TestControll'
                                     other_ref_type="rev",
                                     other_ref=rev2,
                                     other_repo=HG_FORK,
+                                    merge='1',
                                     ))
         response.mustcontain('%s@%s -&gt; %s@%s' % (HG_REPO, rev1, HG_FORK, rev2))
         ## outgoing changesets between those revisions
@@ -328,9 +333,9 b' class TestCompareController(TestControll'
         response.mustcontain("""<a href="/%s/changeset/7d4bc8ec6be56c0f10425afb40b6fc315a4c25e7">r6:%s</a>""" % (HG_FORK, rev2))
 
         ## files
-        response.mustcontain("""<a href="/%s/compare/rev@%s...rev@%s?other_repo=%s#C--9c390eb52cd6">vcs/backends/hg.py</a>""" % (HG_REPO, rev1, rev2, HG_FORK))
-        response.mustcontain("""<a href="/%s/compare/rev@%s...rev@%s?other_repo=%s#C--41b41c1f2796">vcs/backends/__init__.py</a>""" % (HG_REPO, rev1, rev2, HG_FORK))
-        response.mustcontain("""<a href="/%s/compare/rev@%s...rev@%s?other_repo=%s#C--2f574d260608">vcs/backends/base.py</a>""" % (HG_REPO, rev1, rev2, HG_FORK))
+        response.mustcontain("""<a href="/%s/compare/rev@%s...rev@%s?other_repo=%s&amp;merge=1#C--9c390eb52cd6">vcs/backends/hg.py</a>""" % (HG_REPO, rev1, rev2, HG_FORK))
+        response.mustcontain("""<a href="/%s/compare/rev@%s...rev@%s?other_repo=%s&amp;merge=1#C--41b41c1f2796">vcs/backends/__init__.py</a>""" % (HG_REPO, rev1, rev2, HG_FORK))
+        response.mustcontain("""<a href="/%s/compare/rev@%s...rev@%s?other_repo=%s&amp;merge=1#C--2f574d260608">vcs/backends/base.py</a>""" % (HG_REPO, rev1, rev2, HG_FORK))
 
     def test_org_repo_new_commits_after_forking_simple_diff(self):
         self.log_user()
@@ -401,6 +406,7 b' class TestCompareController(TestControll'
                                     other_ref_type="branch",
                                     other_ref=rev2,
                                     other_repo=r1_name,
+                                    merge='1',
                                     ))
         response.mustcontain('%s@%s -&gt; %s@%s' % (r2_name, rev1, r1_name, rev2))
         response.mustcontain('No files')
@@ -425,6 +431,7 b' class TestCompareController(TestControll'
                                     other_ref_type="branch",
                                     other_ref=rev2,
                                     other_repo=r1_name,
+                                    merge='1',
                                     ))
 
         response.mustcontain('%s@%s -&gt; %s@%s' % (r2_name, rev1, r1_name, rev2))