compare: migrated code from pylons to pyramid views.

rhodecode/apps/repository/views/repo_compare.py

@@ -0,0 +1,323 b''
+# -*- coding: utf-8 -*-
+
+# Copyright (C) 2012-2017 RhodeCode GmbH
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License, version 3
+# (only), as published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# This program is dual-licensed. If you wish to learn more about the
+# RhodeCode Enterprise Edition, including its added features, Support services,
+# and proprietary license terms, please see https://rhodecode.com/licenses/
+
+
+import logging
+
+from pyramid.httpexceptions import HTTPBadRequest, HTTPNotFound, HTTPFound
+from pyramid.view import view_config
+from pyramid.renderers import render
+from pyramid.response import Response
+
+
+from rhodecode.apps._base import RepoAppView
+from rhodecode.controllers.utils import parse_path_ref, get_commit_from_ref_name
+from rhodecode.lib import helpers as h
+from rhodecode.lib import diffs, codeblocks
+from rhodecode.lib.auth import LoginRequired, HasRepoPermissionAnyDecorator
+from rhodecode.lib.utils import safe_str
+from rhodecode.lib.utils2 import safe_unicode, str2bool
+from rhodecode.lib.vcs.exceptions import (
+    EmptyRepositoryError, RepositoryError, RepositoryRequirementError,
+    NodeDoesNotExistError)
+from rhodecode.model.db import Repository, ChangesetStatus
+
+log = logging.getLogger(__name__)
+
+
+class RepoCompareView(RepoAppView):
+    def load_default_context(self):
+        c = self._get_local_tmpl_context(include_app_defaults=True)
+
+        # TODO(marcink): remove repo_info and use c.rhodecode_db_repo instead
+        c.repo_info = self.db_repo
+        c.rhodecode_repo = self.rhodecode_vcs_repo
+
+        self._register_global_c(c)
+        return c
+
+    def _get_commit_or_redirect(
+            self, ref, ref_type, repo, redirect_after=True, partial=False):
+        """
+        This is a safe way to get a commit. If an error occurs it
+        redirects to a commit with a proper message. If partial is set
+        then it does not do redirect raise and throws an exception instead.
+        """
+        _ = self.request.translate
+        try:
+            return get_commit_from_ref_name(repo, safe_str(ref), ref_type)
+        except EmptyRepositoryError:
+            if not redirect_after:
+                return repo.scm_instance().EMPTY_COMMIT
+            h.flash(h.literal(_('There are no commits yet')),
+                    category='warning')
+            raise HTTPFound(
+                h.route_path('repo_summary', repo_name=repo.repo_name))
+
+        except RepositoryError as e:
+            log.exception(safe_str(e))
+            h.flash(safe_str(h.escape(e)), category='warning')
+            if not partial:
+                raise HTTPFound(
+                    h.route_path('repo_summary', repo_name=repo.repo_name))
+            raise HTTPBadRequest()
+
+    @LoginRequired()
+    @HasRepoPermissionAnyDecorator(
+        'repository.read', 'repository.write', 'repository.admin')
+    @view_config(
+        route_name='repo_compare_select', request_method='GET',
+        renderer='rhodecode:templates/compare/compare_diff.mako')
+    def compare_select(self):
+        _ = self.request.translate
+        c = self.load_default_context()
+
+        source_repo = self.db_repo_name
+        target_repo = self.request.GET.get('target_repo', source_repo)
+        c.source_repo = Repository.get_by_repo_name(source_repo)
+        c.target_repo = Repository.get_by_repo_name(target_repo)
+
+        if c.source_repo is None or c.target_repo is None:
+            raise HTTPNotFound()
+
+        c.compare_home = True
+        c.commit_ranges = []
+        c.collapse_all_commits = False
+        c.diffset = None
+        c.limited_diff = False
+        c.source_ref = c.target_ref = _('Select commit')
+        c.source_ref_type = ""
+        c.target_ref_type = ""
+        c.commit_statuses = ChangesetStatus.STATUSES
+        c.preview_mode = False
+        c.file_path = None
+
+        return self._get_template_context(c)
+
+    @LoginRequired()
+    @HasRepoPermissionAnyDecorator(
+        'repository.read', 'repository.write', 'repository.admin')
+    @view_config(
+        route_name='repo_compare', request_method='GET',
+        renderer=None)
+    def compare(self):
+        _ = self.request.translate
+        c = self.load_default_context()
+
+        source_ref_type = self.request.matchdict['source_ref_type']
+        source_ref = self.request.matchdict['source_ref']
+        target_ref_type = self.request.matchdict['target_ref_type']
+        target_ref = self.request.matchdict['target_ref']
+
+        # source_ref will be evaluated in source_repo
+        source_repo_name = self.db_repo_name
+        source_path, source_id = parse_path_ref(source_ref)
+
+        # target_ref will be evaluated in target_repo
+        target_repo_name = self.request.GET.get('target_repo', source_repo_name)
+        target_path, target_id = parse_path_ref(
+            target_ref, default_path=self.request.GET.get('f_path', ''))
+
+        # if merge is True
+        #   Show what changes since the shared ancestor commit of target/source
+        #   the source would get if it was merged with target. Only commits
+        #   which are in target but not in source will be shown.
+        merge = str2bool(self.request.GET.get('merge'))
+        # if merge is False
+        #   Show a raw diff of source/target refs even if no ancestor exists
+
+        # c.fulldiff disables cut_off_limit
+        c.fulldiff = str2bool(self.request.GET.get('fulldiff'))
+
+        c.file_path = target_path
+        c.commit_statuses = ChangesetStatus.STATUSES
+
+        # if partial, returns just compare_commits.html (commits log)
+        partial = self.request.is_xhr
+
+        # swap url for compare_diff page
+        c.swap_url = h.route_path(
+            'repo_compare',
+            repo_name=target_repo_name,
+            source_ref_type=target_ref_type,
+            source_ref=target_ref,
+            target_repo=source_repo_name,
+            target_ref_type=source_ref_type,
+            target_ref=source_ref,
+            _query=dict(merge=merge and '1' or '', f_path=target_path))
+
+        source_repo = Repository.get_by_repo_name(source_repo_name)
+        target_repo = Repository.get_by_repo_name(target_repo_name)
+
+        if source_repo is None:
+            log.error('Could not find the source repo: {}'
+                      .format(source_repo_name))
+            h.flash(_('Could not find the source repo: `{}`')
+                    .format(h.escape(source_repo_name)), category='error')
+            raise HTTPFound(
+                h.route_path('repo_compare_select', repo_name=self.db_repo_name))
+
+        if target_repo is None:
+            log.error('Could not find the target repo: {}'
+                      .format(source_repo_name))
+            h.flash(_('Could not find the target repo: `{}`')
+                    .format(h.escape(target_repo_name)), category='error')
+            raise HTTPFound(
+                h.route_path('repo_compare_select', repo_name=self.db_repo_name))
+
+        source_scm = source_repo.scm_instance()
+        target_scm = target_repo.scm_instance()
+
+        source_alias = source_scm.alias
+        target_alias = target_scm.alias
+        if source_alias != target_alias:
+            msg = _('The comparison of two different kinds of remote repos '
+                    'is not available')
+            log.error(msg)
+            h.flash(msg, category='error')
+            raise HTTPFound(
+                h.route_path('repo_compare_select', repo_name=self.db_repo_name))
+
+        source_commit = self._get_commit_or_redirect(
+            ref=source_id, ref_type=source_ref_type, repo=source_repo,
+            partial=partial)
+        target_commit = self._get_commit_or_redirect(
+            ref=target_id, ref_type=target_ref_type, repo=target_repo,
+            partial=partial)
+
+        c.compare_home = False
+        c.source_repo = source_repo
+        c.target_repo = target_repo
+        c.source_ref = source_ref
+        c.target_ref = target_ref
+        c.source_ref_type = source_ref_type
+        c.target_ref_type = target_ref_type
+
+        pre_load = ["author", "branch", "date", "message"]
+        c.ancestor = None
+
+        if c.file_path:
+            if source_commit == target_commit:
+                c.commit_ranges = []
+            else:
+                c.commit_ranges = [target_commit]
+        else:
+            try:
+                c.commit_ranges = source_scm.compare(
+                    source_commit.raw_id, target_commit.raw_id,
+                    target_scm, merge, pre_load=pre_load)
+                if merge:
+                    c.ancestor = source_scm.get_common_ancestor(
+                        source_commit.raw_id, target_commit.raw_id, target_scm)
+            except RepositoryRequirementError:
+                msg = _('Could not compare repos with different '
+                        'large file settings')
+                log.error(msg)
+                if partial:
+                    return Response(msg)
+                h.flash(msg, category='error')
+                raise HTTPFound(
+                    h.route_path('repo_compare_select',
+                                 repo_name=self.db_repo_name))
+
+        c.statuses = self.db_repo.statuses(
+            [x.raw_id for x in c.commit_ranges])
+
+        # auto collapse if we have more than limit
+        collapse_limit = diffs.DiffProcessor._collapse_commits_over
+        c.collapse_all_commits = len(c.commit_ranges) > collapse_limit
+
+        if partial:  # for PR ajax commits loader
+            if not c.ancestor:
+                return Response('')  # cannot merge if there is no ancestor
+
+            html = render(
+                'rhodecode:templates/compare/compare_commits.mako',
+                self._get_template_context(c), self.request)
+            return Response(html)
+
+        if c.ancestor:
+            # case we want a simple diff without incoming commits,
+            # previewing what will be merged.
+            # Make the diff on target repo (which is known to have target_ref)
+            log.debug('Using ancestor %s as source_ref instead of %s'
+                      % (c.ancestor, source_ref))
+            source_repo = target_repo
+            source_commit = target_repo.get_commit(commit_id=c.ancestor)
+
+        # diff_limit will cut off the whole diff if the limit is applied
+        # otherwise it will just hide the big files from the front-end
+        diff_limit = c.visual.cut_off_limit_diff
+        file_limit = c.visual.cut_off_limit_file
+
+        log.debug('calculating diff between '
+                  'source_ref:%s and target_ref:%s for repo `%s`',
+                  source_commit, target_commit,
+                  safe_unicode(source_repo.scm_instance().path))
+
+        if source_commit.repository != target_commit.repository:
+            msg = _(
+                "Repositories unrelated. "
+                "Cannot compare commit %(commit1)s from repository %(repo1)s "
+                "with commit %(commit2)s from repository %(repo2)s.") % {
+                    'commit1': h.show_id(source_commit),
+                    'repo1': source_repo.repo_name,
+                    'commit2': h.show_id(target_commit),
+                    'repo2': target_repo.repo_name,
+                }
+            h.flash(msg, category='error')
+            raise HTTPFound(
+                h.route_path('repo_compare_select',
+                             repo_name=self.db_repo_name))
+
+        txt_diff = source_repo.scm_instance().get_diff(
+            commit1=source_commit, commit2=target_commit,
+            path=target_path, path1=source_path)
+
+        diff_processor = diffs.DiffProcessor(
+            txt_diff, format='newdiff', diff_limit=diff_limit,
+            file_limit=file_limit, show_full_diff=c.fulldiff)
+        _parsed = diff_processor.prepare()
+
+        def _node_getter(commit):
+            """ Returns a function that returns a node for a commit or None """
+            def get_node(fname):
+                try:
+                    return commit.get_node(fname)
+                except NodeDoesNotExistError:
+                    return None
+            return get_node
+
+        diffset = codeblocks.DiffSet(
+            repo_name=source_repo.repo_name,
+            source_node_getter=_node_getter(source_commit),
+            target_node_getter=_node_getter(target_commit),
+        )
+        c.diffset = diffset.render_patchset(
+            _parsed, source_ref, target_ref)
+
+        c.preview_mode = merge
+        c.source_commit = source_commit
+        c.target_commit = target_commit
+
+        html = render(
+            'rhodecode:templates/compare/compare_diff.mako',
+            self._get_template_context(c), self.request)
+        return Response(html)
  No newline at end of file

rhodecode/apps/repository/__init__.py

@@ -32,8 +32,7 b' def includeme(config):'
         name='repo_summary_commits',
         pattern='/{repo_name:.*?[^/]}/summary-commits', repo_route=True)
 
-    # repo commits
-
+    # Commits
     config.add_route(
         name='repo_commit',
         pattern='/{repo_name:.*?[^/]}/changeset/{commit_id}', repo_route=True)
@@ -46,11 +45,6 b' def includeme(config):'
         name='repo_commit_parents',
         pattern='/{repo_name:.*?[^/]}/changeset_parents/{commit_id}', repo_route=True)
 
-    # still working url for backward compat.
-    config.add_route(
-        name='repo_commit_raw_deprecated',
-        pattern='/{repo_name:.*?[^/]}/raw-changeset/{commit_id}', repo_route=True)
-
     config.add_route(
         name='repo_commit_raw',
         pattern='/{repo_name:.*?[^/]}/changeset-diff/{commit_id}', repo_route=True)
@@ -79,7 +73,12 b' def includeme(config):'
         name='repo_commit_comment_delete',
         pattern='/{repo_name:.*?[^/]}/changeset/{commit_id}/comment/{comment_id}/delete', repo_route=True)
 
-    # repo files
+    # still working url for backward compat.
+    config.add_route(
+        name='repo_commit_raw_deprecated',
+        pattern='/{repo_name:.*?[^/]}/raw-changeset/{commit_id}', repo_route=True)
+
+    # Files
     config.add_route(
         name='repo_archivefile',
         pattern='/{repo_name:.*?[^/]}/archive/{fname}', repo_route=True)
@@ -168,7 +167,7 b' def includeme(config):'
         pattern='/{repo_name:.*?[^/]}/create_file/{commit_id}/{f_path:.*}',
         repo_route=True)
 
-    # refs data
+    # Refs data
     config.add_route(
         name='repo_refs_data',
         pattern='/{repo_name:.*?[^/]}/refs-data', repo_route=True)
@@ -192,6 +191,15 b' def includeme(config):'
         name='repo_changelog_elements',
         pattern='/{repo_name:.*?[^/]}/changelog_elements', repo_route=True)
 
+    # Compare
+    config.add_route(
+        name='repo_compare_select',
+        pattern='/{repo_name:.*?[^/]}/compare', repo_route=True)
+
+    config.add_route(
+        name='repo_compare',
+        pattern='/{repo_name:.*?[^/]}/compare/{source_ref_type}@{source_ref:.*?}...{target_ref_type}@{target_ref:.*?}', repo_route=True)
+
     # Tags
     config.add_route(
         name='tags_home',

rhodecode/apps/repository/tests/test_repo_compare.py

@@ -23,12 +23,30 b' import pytest'
 import lxml.html
 
 from rhodecode.lib.vcs.exceptions import RepositoryRequirementError
-from rhodecode.tests import url, assert_session_flash
+from rhodecode.tests import assert_session_flash
 from rhodecode.tests.utils import AssertResponse, commit_change
 
 
+def route_path(name, params=None, **kwargs):
+    import urllib
+
+    base_url = {
+        'repo_compare_select': '/{repo_name}/compare',
+        'repo_compare': '/{repo_name}/compare/{source_ref_type}@{source_ref}...{target_ref_type}@{target_ref}',
+    }[name].format(**kwargs)
+
+    if params:
+        base_url = '{}?{}'.format(base_url, urllib.urlencode(params))
+    return base_url
+
+
 @pytest.mark.usefixtures("autologin_user", "app")
-class TestCompareController(object):
+class TestCompareView(object):
+
+    def test_compare_index_is_reached_at_least_once(self, backend):
+        repo = backend.repo
+        self.app.get(
+            route_path('repo_compare_select', repo_name=repo.repo_name))
 
     @pytest.mark.xfail_backends("svn", reason="Requires pull")
     def test_compare_remote_with_different_commit_indexes(self, backend):
@@ -85,14 +103,14 b' class TestCompareController(object):'
 
         # Comparing the revisions
         response = self.app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=origin.repo_name,
                 source_ref_type="rev",
                 source_ref=commit3.raw_id,
-                target_repo=fork.repo_name,
                 target_ref_type="rev",
                 target_ref=commit4.raw_id,
-                merge='1',))
+                params=dict(merge='1', target_repo=fork.repo_name)
+           ))
 
         compare_page = ComparePage(response)
         compare_page.contains_commits([commit4])
@@ -123,14 +141,14 b' class TestCompareController(object):'
         commit_id2 = repo2.scm_instance().DEFAULT_BRANCH_NAME
 
         response = self.app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=repo1.repo_name,
                 source_ref_type="branch",
                 source_ref=commit_id2,
-                target_repo=repo2.repo_name,
                 target_ref_type="branch",
                 target_ref=commit_id1,
-                merge='1',))
+                params=dict(merge='1', target_repo=repo2.repo_name)
+        ))
 
         response.mustcontain('%s@%s' % (repo1.repo_name, commit_id2))
         response.mustcontain('%s@%s' % (repo2.repo_name, commit_id1))
@@ -180,14 +198,14 b' class TestCompareController(object):'
         commit_id2 = repo2.scm_instance().DEFAULT_BRANCH_NAME
 
         response = self.app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=repo1.repo_name,
                 source_ref_type="branch",
                 source_ref=commit_id2,
-                target_repo=repo2.repo_name,
                 target_ref_type="branch",
                 target_ref=commit_id1,
-                merge='1'))
+                params=dict(merge='1', target_repo=repo2.repo_name),
+            ))
 
         response.mustcontain('%s@%s' % (repo1.repo_name, commit_id2))
         response.mustcontain('%s@%s' % (repo2.repo_name, commit_id1))
@@ -211,17 +229,16 b' class TestCompareController(object):'
         fork = backend.create_repo(number_of_commits=1)
 
         response = self.app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=orig.repo_name,
-                action="compare",
                 source_ref_type="rev",
                 source_ref="tip",
                 target_ref_type="rev",
                 target_ref="tip",
-                merge='1',
-                target_repo=fork.repo_name),
-            status=400)
-
+                params=dict(merge='1', target_repo=fork.repo_name),
+            ),
+            status=302)
+        response = response.follow()
         response.mustcontain("Repositories unrelated.")
 
     @pytest.mark.xfail_backends("svn")
@@ -271,15 +288,15 b' class TestCompareController(object):'
             message='commit6', vcs_type=backend.alias, parent=commit4)
 
         response = self.app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=repo2.repo_name,
                 source_ref_type="rev",
                 # parent of commit2, in target repo2
                 source_ref=commit1.raw_id,
-                target_repo=repo1.repo_name,
                 target_ref_type="rev",
                 target_ref=commit4.raw_id,
-                merge='1',))
+                params=dict(merge='1', target_repo=repo1.repo_name),
+            ))
         response.mustcontain('%s@%s' % (repo2.repo_name, commit1.short_id))
         response.mustcontain('%s@%s' % (repo1.repo_name, commit4.short_id))
 
@@ -337,14 +354,15 b' class TestCompareController(object):'
             message='commit6', vcs_type=backend.alias, parent=commit4)
 
         response = self.app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=repo1.repo_name,
                 source_ref_type="rev",
                 # parent of commit3, not in source repo2
                 source_ref=commit2.raw_id,
                 target_ref_type="rev",
                 target_ref=commit5.raw_id,
-                merge='1',))
+                params=dict(merge='1'),
+        ))
 
         response.mustcontain('%s@%s' % (repo1.repo_name, commit2.short_id))
         response.mustcontain('%s@%s' % (repo1.repo_name, commit5.short_id))
@@ -367,14 +385,14 b' class TestCompareController(object):'
         commit_id2 = repo1.get_commit(commit_idx=6).raw_id
 
         response = self.app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=repo1.repo_name,
                 source_ref_type="rev",
                 source_ref=commit_id1,
                 target_ref_type="rev",
                 target_ref=commit_id2,
-                target_repo=repo2.repo_name,
-                merge='1',))
+                params=dict(merge='1', target_repo=repo2.repo_name),
+            ))
 
         response.mustcontain('%s@%s' % (repo1.repo_name, commit_id1))
         response.mustcontain('%s@%s' % (repo2.repo_name, commit_id2))
@@ -432,14 +450,14 b' class TestCompareController(object):'
         commit_id2 = repo2.scm_instance().DEFAULT_BRANCH_NAME
 
         response = self.app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=r2_name,
                 source_ref_type="branch",
                 source_ref=commit_id1,
                 target_ref_type="branch",
                 target_ref=commit_id2,
-                target_repo=r1_name,
-                merge='1',))
+                params=dict(merge='1', target_repo=r1_name),
+            ))
 
         response.mustcontain('%s@%s' % (r2_name, commit_id1))
         response.mustcontain('%s@%s' % (r1_name, commit_id2))
@@ -453,14 +471,14 b' class TestCompareController(object):'
 
         # compare !
         response = self.app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=r2_name,
                 source_ref_type="branch",
                 source_ref=commit_id1,
                 target_ref_type="branch",
                 target_ref=commit_id2,
-                target_repo=r1_name,
-                merge='1',))
+                params=dict(merge='1', target_repo=r1_name),
+            ))
 
         response.mustcontain('%s@%s' % (r2_name, commit_id1))
         response.mustcontain('%s@%s' % (r1_name, commit_id2))
@@ -476,13 +494,14 b' class TestCompareController(object):'
         commit1 = backend.repo.get_commit(commit_idx=1)
 
         response = self.app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=backend.repo_name,
                 source_ref_type="rev",
                 source_ref=commit0.raw_id,
                 target_ref_type="rev",
                 target_ref=commit1.raw_id,
-                merge='1',),
+                params=dict(merge='1')
+            ),
             extra_environ=xhr_header,)
 
         # outgoing commits between those commits
@@ -494,14 +513,14 b' class TestCompareController(object):'
         badrepo = 'badrepo'
 
         response = self.app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=badrepo,
                 source_ref_type="rev",
                 source_ref='tip',
                 target_ref_type="rev",
                 target_ref='tip',
-                target_repo=repo.repo_name,
-                merge='1',),
+                params=dict(merge='1', target_repo=repo.repo_name)
+            ),
             status=404)
 
     def test_errors_when_comparing_unknown_target_repo(self, backend):
@@ -509,14 +528,14 b' class TestCompareController(object):'
         badrepo = 'badrepo'
 
         response = self.app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=repo.repo_name,
                 source_ref_type="rev",
                 source_ref='tip',
                 target_ref_type="rev",
                 target_ref='tip',
-                target_repo=badrepo,
-                merge='1',),
+                params=dict(merge='1', target_repo=badrepo),
+            ),
             status=302)
         redirected = response.follow()
         redirected.mustcontain(
@@ -526,13 +545,14 b' class TestCompareController(object):'
         commit0 = backend_stub.repo.get_commit(commit_idx=0)
         commit1 = backend_stub.repo.get_commit(commit_idx=1)
 
-        response = self.app.get(url('compare_url',
-                                    repo_name=backend_stub.repo_name,
-                                    source_ref_type="rev",
-                                    source_ref=commit0.raw_id,
-                                    target_ref_type="rev",
-                                    target_ref=commit1.raw_id,
-                                    ),)
+        response = self.app.get(
+            route_path('repo_compare',
+                repo_name=backend_stub.repo_name,
+                source_ref_type="rev",
+                source_ref=commit0.raw_id,
+                target_ref_type="rev",
+                target_ref=commit1.raw_id,
+            ))
 
         # outgoing commits between those commits
         compare_page = ComparePage(response)
@@ -554,15 +574,14 b' class TestCompareController(object):'
             compare_mock.side_effect = RepositoryRequirementError()
 
             response = self.app.get(
-                url('compare_url',
+                route_path('repo_compare',
                     repo_name=orig.repo_name,
-                    action="compare",
                     source_ref_type="rev",
                     source_ref="tip",
                     target_ref_type="rev",
                     target_ref="tip",
-                    merge='1',
-                    target_repo=fork.repo_name),
+                    params=dict(merge='1', target_repo=fork.repo_name),
+                ),
                 status=302)
 
             assert_session_flash(
@@ -577,13 +596,14 b' class TestCompareControllerSvn(object):'
         repo = backend_svn['svn-simple-layout']
         commit_id = repo.get_commit(commit_idx=-1).raw_id
         response = app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=repo.repo_name,
                 source_ref_type="tag",
                 source_ref="%s@%s" % ('tags/v0.1', commit_id),
                 target_ref_type="tag",
                 target_ref="%s@%s" % ('tags/v0.2', commit_id),
-                merge='1',),
+                params=dict(merge='1'),
+            ),
             status=200)
 
         # Expecting no commits, since both paths are at the same revision
@@ -599,13 +619,14 b' class TestCompareControllerSvn(object):'
         source_id = repo.get_commit(commit_idx=-6).raw_id
         target_id = repo.get_commit(commit_idx=-1).raw_id
         response = app.get(
-            url('compare_url',
+            route_path('repo_compare',
                 repo_name=repo.repo_name,
                 source_ref_type="tag",
                 source_ref="%s@%s" % ('tags/v0.1', source_id),
                 target_ref_type="tag",
                 target_ref="%s@%s" % ('tags/v0.2', target_id),
-                merge='1',),
+                params=dict(merge='1')
+            ),
             status=200)
 
         # It should show commits
@@ -673,3 +694,4 b' class ComparePage(AssertResponse):'
     def target_source_are_enabled(self):
         response = self.response
         response.mustcontain("var enable_fields = true;")
+

rhodecode/apps/repository/tests/test_repo_compare_local.py

@@ -20,20 +20,32 b''
 
 import pytest
 
-from rhodecode.tests import url
-from rhodecode.tests.functional.test_compare import ComparePage
+from .test_repo_compare import ComparePage
+
+
+def route_path(name, params=None, **kwargs):
+    import urllib
+
+    base_url = {
+        'repo_compare_select': '/{repo_name}/compare',
+        'repo_compare': '/{repo_name}/compare/{source_ref_type}@{source_ref}...{target_ref_type}@{target_ref}',
+    }[name].format(**kwargs)
+
+    if params:
+        base_url = '{}?{}'.format(base_url, urllib.urlencode(params))
+    return base_url
 
 
 @pytest.mark.usefixtures("autologin_user", "app")
-class TestCompareController:
+class TestCompareView(object):
 
     @pytest.mark.xfail_backends("svn", msg="Depends on branch and tag support")
     def test_compare_tag(self, backend):
         tag1 = 'v0.1.2'
         tag2 = 'v0.1.3'
         response = self.app.get(
-            url(
-                'compare_url',
+            route_path(
+                'repo_compare',
                 repo_name=backend.repo_name,
                 source_ref_type="tag",
                 source_ref=tag1,
@@ -90,8 +102,9 b' class TestCompareController:'
         # functional tests.
         data = revisions[backend.alias]
 
-        response = self.app.get(url(
-            'compare_url',
+        response = self.app.get(
+            route_path(
+            'repo_compare',
             repo_name=backend.repo_name,
             source_ref_type='branch',
             source_ref=data['branch'],
@@ -106,8 +119,9 b' class TestCompareController:'
 
     def test_index_branch(self, backend):
         head_id = backend.default_head_id
-        response = self.app.get(url(
-            'compare_url',
+        response = self.app.get(
+            route_path(
+            'repo_compare',
             repo_name=backend.repo_name,
             source_ref_type="branch",
             source_ref=head_id,
@@ -126,8 +140,9 b' class TestCompareController:'
         commit1 = repo.get_commit(commit_idx=0)
         commit2 = repo.get_commit(commit_idx=1)
 
-        response = self.app.get(url(
-            'compare_url',
+        response = self.app.get(
+            route_path(
+            'repo_compare',
             repo_name=backend.repo_name,
             source_ref_type="rev",
             source_ref=commit1.raw_id,

rhodecode/apps/repository/tests/test_repo_compare_on_single_file.py

@@ -21,13 +21,25 b''
 import pytest
 
 from rhodecode.lib.vcs import nodes
-from rhodecode.tests import url
 from rhodecode.tests.fixture import Fixture
 from rhodecode.tests.utils import commit_change
 
 fixture = Fixture()
 
 
+def route_path(name, params=None, **kwargs):
+    import urllib
+
+    base_url = {
+        'repo_compare_select': '/{repo_name}/compare',
+        'repo_compare': '/{repo_name}/compare/{source_ref_type}@{source_ref}...{target_ref_type}@{target_ref}',
+    }[name].format(**kwargs)
+
+    if params:
+        base_url = '{}?{}'.format(base_url, urllib.urlencode(params))
+    return base_url
+
+
 @pytest.mark.usefixtures("autologin_user", "app")
 class TestSideBySideDiff(object):
 
@@ -45,18 +57,15 b' class TestSideBySideDiff(object):'
             repo.repo_name, filename=f_path, content=commit2_content,
             message='B, child of A', vcs_type=backend.alias, parent=commit1)
 
-        compare_url = url(
-            'compare_url',
+        response = self.app.get(route_path(
+            'repo_compare',
             repo_name=repo.repo_name,
             source_ref_type='rev',
             source_ref=commit1.raw_id,
-            target_repo=repo.repo_name,
             target_ref_type='rev',
             target_ref=commit2.raw_id,
-            f_path=f_path,
-            diffmode='sidebyside')
-
-        response = self.app.get(compare_url)
+            params=dict(f_path=f_path, target_repo=repo.repo_name, diffmode='sidebyside')
+        ))
 
         response.mustcontain('Expand 1 commit')
         response.mustcontain('1 file changed')
@@ -78,18 +87,15 b' class TestSideBySideDiff(object):'
         commit1 = repo.get_commit(commit_idx=0)
         commit2 = repo.get_commit(commit_idx=1)
 
-        compare_url = url(
-            'compare_url',
+        response = self.app.get(route_path(
+            'repo_compare',
             repo_name=repo.repo_name,
             source_ref_type='rev',
             source_ref=commit1.raw_id,
-            target_repo=repo.repo_name,
             target_ref_type='rev',
             target_ref=commit2.raw_id,
-            f_path=f_path,
-            diffmode='sidebyside')
-
-        response = self.app.get(compare_url)
+            params=dict(f_path=f_path, target_repo=repo.repo_name, diffmode='sidebyside')
+        ))
 
         response.mustcontain('Expand 1 commit')
         response.mustcontain('1 file changed')
@@ -124,16 +130,16 b' class TestSideBySideDiff(object):'
         commit2, commit1 = commit_info['commits']
         file_changes = commit_info['changes']
 
-        compare_url = url(
-            'compare_url',
+        response = self.app.get(route_path(
+            'repo_compare',
             repo_name=backend.repo_name,
             source_ref_type='rev',
             source_ref=commit2,
             target_repo=backend.repo_name,
             target_ref_type='rev',
             target_ref=commit1,
-            diffmode='sidebyside')
-        response = self.app.get(compare_url)
+            params=dict(target_repo=backend.repo_name, diffmode='sidebyside')
+        ))
 
         response.mustcontain('Expand 1 commit')
         response.mustcontain(file_changes)
@@ -163,17 +169,15 b' class TestSideBySideDiff(object):'
         commit2, commit1 = commit_info['commits']
         file_changes = commit_info['changes']
 
-        compare_url = url(
-            'compare_url',
+        response = self.app.get(route_path(
+            'repo_compare',
             repo_name=backend.repo_name,
             source_ref_type='rev',
             source_ref=commit2,
-            target_repo=backend.repo_name,
             target_ref_type='rev',
             target_ref=commit1,
-            f_path=f_path,
-            diffmode='sidebyside')
-        response = self.app.get(compare_url)
+            params=dict(f_path=f_path, target_repo=backend.repo_name, diffmode='sidebyside')
+        ))
 
         response.mustcontain('Expand 1 commit')
         response.mustcontain(file_changes)

rhodecode/apps/repository/views/repo_files.py

@@ -392,14 +392,15 b' class RepoFilesView(RepoAppView):'
 
         c.action = self.request.GET.get('diff')
         if c.action not in ['download', 'raw']:
-            compare_url = h.url(
-                'compare_url', repo_name=self.db_repo_name,
+            compare_url = h.route_path(
+                'repo_compare',
+                repo_name=self.db_repo_name,
                 source_ref_type='rev',
                 source_ref=diff1,
                 target_repo=self.db_repo_name,
                 target_ref_type='rev',
                 target_ref=diff2,
-                f_path=f_path)
+                _query=dict(f_path=f_path))
             # redirect to new view if we render diff
             raise HTTPFound(compare_url)
 
@@ -471,15 +472,15 b' class RepoFilesView(RepoAppView):'
                 category='error')
             raise HTTPBadRequest()
 
-        compare_url = h.url(
-            'compare_url', repo_name=self.db_repo_name,
+        compare_url = h.route_path(
+            'repo_compare',
+            repo_name=self.db_repo_name,
             source_ref_type='rev',
             source_ref=diff1,
-            target_repo=self.db_repo_name,
             target_ref_type='rev',
             target_ref=diff2,
-            f_path=f_path,
-            diffmode='sideside')
+            _query=dict(f_path=f_path, diffmode='sideside',
+                        target_repo=self.db_repo_name,))
         raise HTTPFound(compare_url)
 
     @LoginRequired()

rhodecode/config/routing.py

@@ -502,17 +502,6 b' def make_map(config):'
                  conditions={'method': ['GET', 'POST'], 'function': check_repo},
                  requirements=URL_NAME_REQUIREMENTS)
 
-    rmap.connect('compare_home',
-                 '/{repo_name}/compare',
-                 controller='compare', action='index',
-                 conditions={'function': check_repo},
-                 requirements=URL_NAME_REQUIREMENTS)
-
-    rmap.connect('compare_url',
-                 '/{repo_name}/compare/{source_ref_type}@{source_ref:.*?}...{target_ref_type}@{target_ref:.*?}',
-                 controller='compare', action='compare',
-                 conditions={'function': check_repo},
-                 requirements=URL_NAME_REQUIREMENTS, jsroute=True)
 
     rmap.connect('pullrequest_home',
                  '/{repo_name}/pull-request/new', controller='pullrequests',

rhodecode/public/js/rhodecode/routes.js

@@ -15,7 +15,6 b' function registerRCRoutes() {'
     pyroutes.register('new_repo', '/_admin/create_repository', []);
     pyroutes.register('edit_user', '/_admin/users/%(user_id)s/edit', ['user_id']);
     pyroutes.register('edit_user_group_members', '/_admin/user_groups/%(user_group_id)s/edit/members', ['user_group_id']);
-    pyroutes.register('compare_url', '/%(repo_name)s/compare/%(source_ref_type)s@%(source_ref)s...%(target_ref_type)s@%(target_ref)s', ['repo_name', 'source_ref_type', 'source_ref', 'target_ref_type', 'target_ref']);
     pyroutes.register('pullrequest_home', '/%(repo_name)s/pull-request/new', ['repo_name']);
     pyroutes.register('pullrequest', '/%(repo_name)s/pull-request/new', ['repo_name']);
     pyroutes.register('pullrequest_repo_refs', '/%(repo_name)s/pull-request/refs/%(target_repo_name)s', ['repo_name', 'target_repo_name']);
@@ -108,7 +107,6 b' function registerRCRoutes() {'
     pyroutes.register('repo_commit', '/%(repo_name)s/changeset/%(commit_id)s', ['repo_name', 'commit_id']);
     pyroutes.register('repo_commit_children', '/%(repo_name)s/changeset_children/%(commit_id)s', ['repo_name', 'commit_id']);
     pyroutes.register('repo_commit_parents', '/%(repo_name)s/changeset_parents/%(commit_id)s', ['repo_name', 'commit_id']);
-    pyroutes.register('repo_commit_raw_deprecated', '/%(repo_name)s/raw-changeset/%(commit_id)s', ['repo_name', 'commit_id']);
     pyroutes.register('repo_commit_raw', '/%(repo_name)s/changeset-diff/%(commit_id)s', ['repo_name', 'commit_id']);
     pyroutes.register('repo_commit_patch', '/%(repo_name)s/changeset-patch/%(commit_id)s', ['repo_name', 'commit_id']);
     pyroutes.register('repo_commit_download', '/%(repo_name)s/changeset-download/%(commit_id)s', ['repo_name', 'commit_id']);
@@ -116,6 +114,7 b' function registerRCRoutes() {'
     pyroutes.register('repo_commit_comment_create', '/%(repo_name)s/changeset/%(commit_id)s/comment/create', ['repo_name', 'commit_id']);
     pyroutes.register('repo_commit_comment_preview', '/%(repo_name)s/changeset/%(commit_id)s/comment/preview', ['repo_name', 'commit_id']);
     pyroutes.register('repo_commit_comment_delete', '/%(repo_name)s/changeset/%(commit_id)s/comment/%(comment_id)s/delete', ['repo_name', 'commit_id', 'comment_id']);
+    pyroutes.register('repo_commit_raw_deprecated', '/%(repo_name)s/raw-changeset/%(commit_id)s', ['repo_name', 'commit_id']);
     pyroutes.register('repo_archivefile', '/%(repo_name)s/archive/%(fname)s', ['repo_name', 'fname']);
     pyroutes.register('repo_files_diff', '/%(repo_name)s/diff/%(f_path)s', ['repo_name', 'f_path']);
     pyroutes.register('repo_files_diff_2way_redirect', '/%(repo_name)s/diff-2way/%(f_path)s', ['repo_name', 'f_path']);
@@ -145,6 +144,8 b' function registerRCRoutes() {'
     pyroutes.register('repo_changelog', '/%(repo_name)s/changelog', ['repo_name']);
     pyroutes.register('repo_changelog_file', '/%(repo_name)s/changelog/%(commit_id)s/%(f_path)s', ['repo_name', 'commit_id', 'f_path']);
     pyroutes.register('repo_changelog_elements', '/%(repo_name)s/changelog_elements', ['repo_name']);
+    pyroutes.register('repo_compare_select', '/%(repo_name)s/compare', ['repo_name']);
+    pyroutes.register('repo_compare', '/%(repo_name)s/compare/%(source_ref_type)s@%(source_ref)s...%(target_ref_type)s@%(target_ref)s', ['repo_name', 'source_ref_type', 'source_ref', 'target_ref_type', 'target_ref']);
     pyroutes.register('tags_home', '/%(repo_name)s/tags', ['repo_name']);
     pyroutes.register('branches_home', '/%(repo_name)s/branches', ['repo_name']);
     pyroutes.register('bookmarks_home', '/%(repo_name)s/bookmarks', ['repo_name']);

rhodecode/public/js/src/rhodecode.js

@@ -58,7 +58,7 b' var compare_radio_buttons = function(rep'
                 target_ref_type: compare_ref_type,
                 merge: 1
             };
-            window.location = pyroutes.url('compare_url', url_data);
+            window.location = pyroutes.url('repo_compare', url_data);
         }
     });
     $('.compare-radio-button').on('click', function(e){

rhodecode/templates/base/base.mako

@@ -228,9 +228,7 b''
         <li class="${is_active('summary')}"><a class="menulink" href="${h.route_path('repo_summary', repo_name=c.repo_name)}"><div class="menulabel">${_('Summary')}</div></a></li>
         <li class="${is_active('changelog')}"><a class="menulink" href="${h.route_path('repo_changelog', repo_name=c.repo_name)}"><div class="menulabel">${_('Changelog')}</div></a></li>
         <li class="${is_active('files')}"><a class="menulink" href="${h.route_path('repo_files', repo_name=c.repo_name, commit_id=c.rhodecode_db_repo.landing_rev[1], f_path='')}"><div class="menulabel">${_('Files')}</div></a></li>
-        <li class="${is_active('compare')}">
-            <a class="menulink" href="${h.url('compare_home',repo_name=c.repo_name)}"><div class="menulabel">${_('Compare')}</div></a>
-        </li>
+        <li class="${is_active('compare')}"><a class="menulink" href="${h.route_path('repo_compare_select',repo_name=c.repo_name)}"><div class="menulabel">${_('Compare')}</div></a></li>
         ## TODO: anderson: ideally it would have a function on the scm_instance "enable_pullrequest() and enable_fork()"
         %if c.rhodecode_db_repo.repo_type in ['git','hg']:
           <li class="${is_active('showpullrequest')}">
@@ -251,8 +249,19 b''
                    <li><a href="${h.route_path('edit_repo',repo_name=c.repo_name)}">${_('Settings')}</a></li>
              %endif
               %if c.rhodecode_db_repo.fork:
-               <li><a href="${h.url('compare_url',repo_name=c.rhodecode_db_repo.fork.repo_name,source_ref_type=c.rhodecode_db_repo.landing_rev[0],source_ref=c.rhodecode_db_repo.landing_rev[1], target_repo=c.repo_name,target_ref_type='branch' if request.GET.get('branch') else c.rhodecode_db_repo.landing_rev[0],target_ref=request.GET.get('branch') or c.rhodecode_db_repo.landing_rev[1], merge=1)}">
-                   ${_('Compare fork')}</a></li>
+               <li>
+                   <a title="${h.tooltip(_('Compare fork with %s' % c.rhodecode_db_repo.fork.repo_name))}"
+                      href="${h.route_path('repo_compare',
+                            repo_name=c.rhodecode_db_repo.fork.repo_name,
+                            source_ref_type=c.rhodecode_db_repo.landing_rev[0],
+                            source_ref=c.rhodecode_db_repo.landing_rev[1],
+                            target_repo=c.repo_name,target_ref_type='branch' if request.GET.get('branch') else c.rhodecode_db_repo.landing_rev[0],
+                            target_ref=request.GET.get('branch') or c.rhodecode_db_repo.landing_rev[1],
+                            _query=dict(merge=1))}"
+                    >
+                   ${_('Compare fork')}
+                   </a>
+               </li>
               %endif
 
               <li><a href="${h.route_path('search_repo',repo_name=c.repo_name)}">${_('Search')}</a></li>

rhodecode/templates/changelog/changelog.mako

@@ -39,17 +39,15 b''
                         <a  id="compare_fork_button"
                             title="${h.tooltip(_('Compare fork with %s' % c.rhodecode_db_repo.fork.repo_name))}"
                             class="btn btn-small"
-                            href="${h.url('compare_url',
+                            href="${h.route_path('repo_compare',
                                 repo_name=c.rhodecode_db_repo.fork.repo_name,
                                 source_ref_type=c.rhodecode_db_repo.landing_rev[0],
                                 source_ref=c.rhodecode_db_repo.landing_rev[1],
-                                target_repo=c.repo_name,
                                 target_ref_type='branch' if request.GET.get('branch') else c.rhodecode_db_repo.landing_rev[0],
                                 target_ref=request.GET.get('branch') or c.rhodecode_db_repo.landing_rev[1],
-                                merge=1)}"
+                                _query=dict(merge=1, target_repo=c.repo_name))}"
                         >
-                                <i class="icon-loop"></i>
-                                ${_('Compare fork with Parent (%s)' % c.rhodecode_db_repo.fork.repo_name)}
+                        ${_('Compare fork with Parent (%s)' % c.rhodecode_db_repo.fork.repo_name)}
                         </a>
                     </span>
                 %endif

rhodecode/templates/changeset/changeset_range.mako

@@ -55,7 +55,13 b''
             </div>
             <div class="right-content">
               <div class="header-buttons">
-                <a href="${h.url('compare_url', repo_name=c.repo_name, source_ref_type='rev', source_ref=getattr(c.commit_ranges[0].parents[0] if c.commit_ranges[0].parents else h.EmptyCommit(), 'raw_id'), target_ref_type='rev', target_ref=c.commit_ranges[-1].raw_id)}">
+                <a href="${h.route_path('repo_compare',
+                repo_name=c.repo_name,
+                source_ref_type='rev',
+                source_ref=getattr(c.commit_ranges[0].parents[0] if c.commit_ranges[0].parents else h.EmptyCommit(), 'raw_id'),
+                target_ref_type='rev',
+                target_ref=c.commit_ranges[-1].raw_id)}"
+                >
                     ${_('Show combined compare')}
                 </a>
               </div>

rhodecode/templates/compare/compare_diff.mako

@@ -288,7 +288,7 b''
                             target_ref: target.id,
                             target_ref_type: target.type
                         };
-                        window.location = pyroutes.url('compare_url', url_data);
+                        window.location = pyroutes.url('repo_compare', url_data);
                     }
                 });
                 $('#compare_changeset_status_toggle').on('click', function(e) {

rhodecode/templates/files/files.mako

@@ -174,7 +174,7 b''
                         merge: 1,
                         f_path: state.f_path
                     };
-                    window.location = pyroutes.url('compare_url', url_data);
+                    window.location = pyroutes.url('repo_compare', url_data);
                 });
 
                 $('#show_at_commit').on('click', function(e) {

rhodecode/templates/forks/forks_data.mako

@@ -26,8 +26,17 b''
             </td>
             <td class="td-compare">
                 <a title="${h.tooltip(_('Compare fork with %s' % c.repo_name))}"
-                   href="${h.url('compare_url',repo_name=c.repo_name, source_ref_type=c.rhodecode_db_repo.landing_rev[0],source_ref=c.rhodecode_db_repo.landing_rev[1],target_repo=f.repo_name,target_ref_type=c.rhodecode_db_repo.landing_rev[0],target_ref=c.rhodecode_db_repo.landing_rev[1], merge=1)}"
-                   class="btn-link"><i class="icon-loop"></i> ${_('Compare fork')}</a>
+                   class="btn-link"
+                   href="${h.route_path('repo_compare',
+                   repo_name=c.repo_name,
+                   source_ref_type=c.rhodecode_db_repo.landing_rev[0],
+                   source_ref=c.rhodecode_db_repo.landing_rev[1],
+                   target_ref_type=c.rhodecode_db_repo.landing_rev[0],
+                   target_ref=c.rhodecode_db_repo.landing_rev[1],
+                   _query=dict(merge=1, target_repo=f.repo_name))}"
+                >
+                ${_('Compare fork')}
+                </a>
             </td>
         </tr>
     % endfor

rhodecode/templates/pullrequests/pullrequest.mako

@@ -302,7 +302,7 b''
           prButtonLock(true, "${_('Please select source and target')}");
           return;
       }
-      var url = pyroutes.url('compare_url', url_data);
+      var url = pyroutes.url('repo_compare', url_data);
 
       // lock PR button, so we cannot send PR before it's calculated
       prButtonLock(true, "${_('Loading compare ...')}", 'compare');