compare: swap org and other when they refer to different repos, ie are pull request style

Pull requests will have a different from/to description - but in a consistent
and slightly better way that can be improved later.

A pull request diff is empathic: "How will this look for the peer I'm proposing to" style.

rhodecode/config/routing.py

@@ -493,8 +493,8 b' def make_map(config):'
                  controller='compare', action='index',
                  conditions=dict(function=check_repo),
                  requirements=dict(
-                            org_ref_type='(branch|book|tag|rev|__org_ref_type__)',
-                            other_ref_type='(branch|book|tag|rev|__other_ref_type__)')
+                            org_ref_type='(branch|book|tag|rev|__other_ref_type__)',
+                            other_ref_type='(branch|book|tag|rev|__org_ref_type__)')
                  )
 
     rmap.connect('pullrequest_home',

rhodecode/controllers/compare.py

@@ -132,7 +132,7 b' class CompareController(BaseRepoControll'
 
         c.statuses = c.rhodecode_db_repo.statuses([x.raw_id for x in
                                                    c.cs_ranges])
-        c.target_repo = c.repo_name
+        c.target_repo = c.other_repo.repo_name
         # defines that we need hidden inputs with changesets
         c.as_form = request.GET.get('as_form', False)
         if partial:
@@ -150,7 +150,7 b' class CompareController(BaseRepoControll'
                                       if c.cs_ranges[0].parents
                                       else EmptyChangeset(), 'raw_id'))
             log.debug('Changed org_ref from %s to %s' % (_org_ref, org_ref))
-            other_repo = org_repo
+            org_repo = other_repo
 
         diff_limit = self.cut_off_limit if not fulldiff else None
 

rhodecode/model/pull_request.py

@@ -190,21 +190,21 b' class PullRequestModel(BaseModel):'
 
             #case two independent repos
             if org_repo != other_repo:
-                hgrepo = unionrepo.unionrepository(org_repo.baseui,
-                                                   org_repo.path,
-                                                   other_repo.path)
-                revs = ["ancestors(id('%s')) and not ancestors(id('%s'))" %
-                        (org_rev, other_rev)]
+                hgrepo = unionrepo.unionrepository(other_repo.baseui,
+                                                   other_repo.path,
+                                                   org_repo.path)
+                # all the changesets we are looking for will be in other_repo,
+                # so rev numbers from hgrepo can be used in other_repo
 
             #no remote compare do it on the same repository
             else:
-                hgrepo = org_repo._repo
-                revs = ["ancestors(id('%s')) and not ancestors(id('%s'))" %
-                        (other_rev, org_rev)]
+                hgrepo = other_repo._repo
 
+            revs = ["ancestors(id('%s')) and not ancestors(id('%s'))" %
+                    (other_rev, org_rev)]
             out = scmutil.revrange(hgrepo, revs)
             for cs in (out):
-                changesets.append(org_repo.get_changeset(cs))
+                changesets.append(other_repo.get_changeset(cs))
 
         elif alias == 'git':
             assert org_repo == other_repo, (org_repo, other_repo) # no git support for different repos

rhodecode/templates/base/base.html

@@ -220,7 +220,7 b''
                  <li>${h.link_to(_('open new pull request'),h.url('pullrequest_home',repo_name=c.repo_name),class_='pull_request')}</li>
                 %endif
                 %if c.rhodecode_db_repo.fork:
-                 <li>${h.link_to(_('compare fork'),h.url('compare_url',repo_name=c.repo_name,org_ref_type='branch',org_ref=request.GET.get('branch') or 'default',other_repo=c.rhodecode_db_repo.fork.repo_name,other_ref_type='branch',other_ref='default'),class_='compare_request')}</li>
+                 <li>${h.link_to(_('compare fork'),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_='compare_request')}</li>
                 %endif
                 <li>${h.link_to(_('lightweight changelog'),h.url('shortlog_home',repo_name=c.repo_name),class_='shortlog')}</li>
                 <li>${h.link_to(_('search'),h.url('search_repo',repo_name=c.repo_name),class_='search')}</li>

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 title="${_('compare fork with %s' % c.rhodecode_db_repo.fork.repo_name)}" href="${h.url('compare_url',repo_name=c.repo_name,org_ref_type='branch',org_ref=request.GET.get('branch') or 'default',other_repo=c.rhodecode_db_repo.fork.repo_name,other_ref_type='branch',other_ref='default')}" class="ui-btn small">${_('Compare fork with parent')}</a>
+                        <a 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>
                     %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/forks/forks_data.html

@@ -17,7 +17,7 b''
             <div class="follower_date">${_('forked')} -
                 <span class="tooltip" title="${h.tooltip(h.fmt_date(f.created_on))}"> ${h.age(f.created_on)}</span>
                 <a title="${_('compare fork with %s' % c.repo_name)}"
-                    href="${h.url('compare_url',repo_name=f.repo_name,org_ref_type='branch',org_ref='default',other_repo=c.repo_name,other_ref_type='branch',other_ref='default')}"
+                    href="${h.url('compare_url',repo_name=c.repo_name,org_ref_type='branch',org_ref='default',other_repo=f.repo_name,other_ref_type='branch',other_ref='default')}"
                     class="ui-btn small">${_('Compare fork')}</a>
             </div>
             <div style="border-bottom: 1px solid #DDD;margin:10px 0px 10px 0px"></div>

rhodecode/templates/pullrequests/pullrequest.html

@@ -140,12 +140,12 b''
       YUD.setStyle(YUD.get('pull_request_overview_url').parentElement,'display','none');
       //url template
       var url = "${h.url('compare_url',
-                         repo_name='__org_repo__',
-                         org_ref_type='__org_ref_type__',
-                         org_ref='__org_ref__',
-                         other_repo='__other_repo__',
-                         other_ref_type='__other_ref_type__',
-                         other_ref='__other_ref__',
+                         repo_name='__other_repo__',
+                         org_ref_type='__other_ref_type__',
+                         org_ref='__other_ref__',
+                         other_repo='__org_repo__',
+                         other_ref_type='__org_ref_type__',
+                         other_ref='__org_ref__',
                          as_form=True,
                          rev_start=request.GET.get('rev_start',''),
                          rev_end=request.GET.get('rev_end',''))}";

rhodecode/tests/functional/test_compare.py

@@ -89,16 +89,16 b' class TestCompareController(TestControll'
         rev1 = 'default'
         rev2 = 'default'
         response = self.app.get(url(controller='compare', action='index',
-                                    repo_name=repo2.repo_name,
+                                    repo_name=repo1.repo_name,
                                     org_ref_type="branch",
-                                    org_ref=rev1,
-                                    other_repo=repo1.repo_name,
+                                    org_ref=rev2,
+                                    other_repo=repo2.repo_name,
                                     other_ref_type="branch",
-                                    other_ref=rev2,
+                                    other_ref=rev1,
                                     ))
 
         try:
-            response.mustcontain('%s@%s -> %s@%s' % (repo2.repo_name, rev1, repo1.repo_name, rev2))
+            response.mustcontain('%s@%s -> %s@%s' % (repo1.repo_name, rev2, repo2.repo_name, rev1))
             response.mustcontain("""Showing 2 commits""")
             response.mustcontain("""1 file changed with 2 insertions and 0 deletions""")
 
@@ -108,9 +108,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>""" % (repo2.repo_name, rev1, rev2, repo1.repo_name))
+            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))
             #swap
-            response.mustcontain("""<a href="/%s/compare/branch@%s...branch@%s?as_form=None&amp;other_repo=%s">[swap]</a>""" % (repo1.repo_name, rev1, rev2, repo2.repo_name))
+            response.mustcontain("""<a href="/%s/compare/branch@%s...branch@%s?as_form=None&amp;other_repo=%s">[swap]</a>""" % (repo2.repo_name, rev1, rev2, repo1.repo_name))
         finally:
             RepoModel().delete(r2_id)
             RepoModel().delete(r1_id)
@@ -147,16 +147,16 b' class TestCompareController(TestControll'
         rev1 = 'default'
         rev2 = 'default'
         response = self.app.get(url(controller='compare', action='index',
-                                    repo_name=repo2.repo_name,
+                                    repo_name=repo1.repo_name,
                                     org_ref_type="branch",
-                                    org_ref=rev1,
-                                    other_repo=repo1.repo_name,
+                                    org_ref=rev2,
+                                    other_repo=repo2.repo_name,
                                     other_ref_type="branch",
-                                    other_ref=rev2,
+                                    other_ref=rev1,
                                     ))
 
         try:
-            response.mustcontain('%s@%s -&gt; %s@%s' % (repo2.repo_name, rev1, repo1.repo_name, rev2))
+            response.mustcontain('%s@%s -&gt; %s@%s' % (repo1.repo_name, rev2, repo2.repo_name, rev1))
             response.mustcontain("""Showing 2 commits""")
             response.mustcontain("""1 file changed with 2 insertions and 0 deletions""")
 
@@ -166,9 +166,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>""" % (repo2.repo_name, rev1, rev2, repo1.repo_name))
+            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))
             #swap
-            response.mustcontain("""<a href="/%s/compare/branch@%s...branch@%s?as_form=None&amp;other_repo=%s">[swap]</a>""" % (repo1.repo_name, rev1, rev2, repo2.repo_name))
+            response.mustcontain("""<a href="/%s/compare/branch@%s...branch@%s?as_form=None&amp;other_repo=%s">[swap]</a>""" % (repo2.repo_name, rev1, rev2, repo1.repo_name))
         finally:
             RepoModel().delete(r2_id)
             RepoModel().delete(r1_id)