diffs: introducing diff menu for whitespace toggle and context changes

rhodecode/apps/repository/views/repo_commits.py

@@ -34,7 +34,9 b' from rhodecode.lib.auth import ('
     LoginRequired, HasRepoPermissionAnyDecorator, NotAnonymous, CSRFRequired)
 
 from rhodecode.lib.compat import OrderedDict
-from rhodecode.lib.diffs import cache_diff, load_cached_diff, diff_cache_exist
+from rhodecode.lib.diffs import (
+    cache_diff, load_cached_diff, diff_cache_exist, get_diff_context,
+    get_diff_whitespace_flag)
 from rhodecode.lib.exceptions import StatusChangeOnClosedPullRequestError
 import rhodecode.lib.helpers as h
 from rhodecode.lib.utils2 import safe_unicode, str2bool
@@ -55,96 +57,8 b' def _update_with_GET(params, request):'
         params[k] += request.GET.getall(k)
 
 
-def get_ignore_ws(fid, request):
-    ig_ws_global = request.GET.get('ignorews')
-    ig_ws = filter(lambda k: k.startswith('WS'), request.GET.getall(fid))
-    if ig_ws:
-        try:
-            return int(ig_ws[0].split(':')[-1])
-        except Exception:
-            pass
-    return ig_ws_global
 
 
-def _ignorews_url(request, fileid=None):
-    _ = request.translate
-    fileid = str(fileid) if fileid else None
-    params = collections.defaultdict(list)
-    _update_with_GET(params, request)
-    label = _('Show whitespace')
-    tooltiplbl = _('Show whitespace for all diffs')
-    ig_ws = get_ignore_ws(fileid, request)
-    ln_ctx = get_line_ctx(fileid, request)
-
-    if ig_ws is None:
-        params['ignorews'] += [1]
-        label = _('Ignore whitespace')
-        tooltiplbl = _('Ignore whitespace for all diffs')
-    ctx_key = 'context'
-    ctx_val = ln_ctx
-
-    # if we have passed in ln_ctx pass it along to our params
-    if ln_ctx:
-        params[ctx_key] += [ctx_val]
-
-    if fileid:
-        params['anchor'] = 'a_' + fileid
-    return h.link_to(label, request.current_route_path(_query=params),
-                     title=tooltiplbl, class_='tooltip')
-
-
-def get_line_ctx(fid, request):
-    ln_ctx_global = request.GET.get('context')
-    if fid:
-        ln_ctx = filter(lambda k: k.startswith('C'), request.GET.getall(fid))
-    else:
-        _ln_ctx = filter(lambda k: k.startswith('C'), request.GET)
-        ln_ctx = request.GET.get(_ln_ctx[0]) if _ln_ctx else ln_ctx_global
-        if ln_ctx:
-            ln_ctx = [ln_ctx]
-
-    if ln_ctx:
-        retval = ln_ctx[0].split(':')[-1]
-    else:
-        retval = ln_ctx_global
-
-    try:
-        return min(diffs.MAX_CONTEXT, int(retval))
-    except Exception:
-        return 3
-
-
-def _context_url(request, fileid=None):
-    """
-    Generates a url for context lines.
-
-    :param fileid:
-    """
-
-    _ = request.translate
-    fileid = str(fileid) if fileid else None
-    ig_ws = get_ignore_ws(fileid, request)
-    ln_ctx = (get_line_ctx(fileid, request) or 3) * 2
-
-    params = collections.defaultdict(list)
-    _update_with_GET(params, request)
-
-    if ln_ctx > 0:
-        params['context'] += [ln_ctx]
-
-    if ig_ws:
-        ig_ws_key = 'ignorews'
-        ig_ws_val = 1
-        params[ig_ws_key] += [ig_ws_val]
-
-    lbl = _('Increase context')
-    tooltiplbl = _('Increase context for all diffs')
-
-    if fileid:
-        params['anchor'] = 'a_' + fileid
-    return h.link_to(lbl, request.current_route_path(_query=params),
-                     title=tooltiplbl, class_='tooltip')
-
 
 class RepoCommitsView(RepoAppView):
     def load_default_context(self):
@@ -162,13 +76,11 b' class RepoCommitsView(RepoAppView):'
     def _commit(self, commit_id_range, method):
         _ = self.request.translate
         c = self.load_default_context()
-        c.ignorews_url = _ignorews_url
-        c.context_url = _context_url
         c.fulldiff = self.request.GET.get('fulldiff')
 
         # fetch global flags of ignore ws or context lines
-        context_lcl = get_line_ctx('', self.request)
-        ign_whitespace_lcl = get_ignore_ws('', self.request)
+        diff_context = get_diff_context(self.request)
+        hide_whitespace_changes = get_diff_whitespace_flag(self.request)
 
         # 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
@@ -258,7 +170,7 b' class RepoCommitsView(RepoAppView):'
                     self.db_repo)
                 cache_file_path = diff_cache_exist(
                     cache_path, 'diff', commit.raw_id,
-                    ign_whitespace_lcl, context_lcl, c.fulldiff)
+                    hide_whitespace_changes, diff_context, c.fulldiff)
 
                 caching_enabled = self._is_diff_cache_enabled(self.db_repo)
                 force_recache = str2bool(self.request.GET.get('force_recache'))
@@ -273,8 +185,8 b' class RepoCommitsView(RepoAppView):'
                 else:
                     vcs_diff = self.rhodecode_vcs_repo.get_diff(
                         commit1, commit2,
-                        ignore_whitespace=ign_whitespace_lcl,
-                        context=context_lcl)
+                        ignore_whitespace=hide_whitespace_changes,
+                        context=diff_context)
 
                     diff_processor = diffs.DiffProcessor(
                         vcs_diff, format='newdiff', diff_limit=diff_limit,
@@ -300,7 +212,7 b' class RepoCommitsView(RepoAppView):'
                 # TODO(marcink): no cache usage here...
                 _diff = self.rhodecode_vcs_repo.get_diff(
                     commit1, commit2,
-                    ignore_whitespace=ign_whitespace_lcl, context=context_lcl)
+                    ignore_whitespace=hide_whitespace_changes, context=diff_context)
                 diff_processor = diffs.DiffProcessor(
                     _diff, format='newdiff', diff_limit=diff_limit,
                     file_limit=file_limit, show_full_diff=c.fulldiff)

rhodecode/apps/repository/views/repo_compare.py

@@ -44,10 +44,7 b' log = logging.getLogger(__name__)'
 class RepoCompareView(RepoAppView):
     def load_default_context(self):
         c = self._get_local_tmpl_context(include_app_defaults=True)
-
         c.rhodecode_repo = self.rhodecode_vcs_repo
-
-
         return c
 
     def _get_commit_or_redirect(
@@ -145,6 +142,10 b' class RepoCompareView(RepoAppView):'
         # c.fulldiff disables cut_off_limit
         c.fulldiff = str2bool(self.request.GET.get('fulldiff'))
 
+        # fetch global flags of ignore ws or context lines
+        diff_context = diffs.get_diff_context(self.request)
+        hide_whitespace_changes = diffs.get_diff_whitespace_flag(self.request)
+
         c.file_path = target_path
         c.commit_statuses = ChangesetStatus.STATUSES
 
@@ -288,7 +289,8 b' class RepoCompareView(RepoAppView):'
 
         txt_diff = source_repo.scm_instance().get_diff(
             commit1=source_commit, commit2=target_commit,
-            path=target_path, path1=source_path)
+            path=target_path, path1=source_path,
+            ignore_whitespace=hide_whitespace_changes, context=diff_context)
 
         diff_processor = diffs.DiffProcessor(
             txt_diff, format='newdiff', diff_limit=diff_limit,

rhodecode/apps/repository/views/repo_pull_requests.py

@@ -219,10 +219,11 b' class RepoPullRequestsView(RepoAppView, '
     def _get_diffset(self, source_repo_name, source_repo,
                      source_ref_id, target_ref_id,
                      target_commit, source_commit, diff_limit, file_limit,
-                     fulldiff):
+                     fulldiff, hide_whitespace_changes, diff_context):
 
         vcs_diff = PullRequestModel().get_diff(
-            source_repo, source_ref_id, target_ref_id)
+            source_repo, source_ref_id, target_ref_id,
+            hide_whitespace_changes, diff_context)
 
         diff_processor = diffs.DiffProcessor(
             vcs_diff, format='newdiff', diff_limit=diff_limit,
@@ -243,11 +244,11 b' class RepoPullRequestsView(RepoAppView, '
 
     def _get_range_diffset(self, source_scm, source_repo,
                            commit1, commit2, diff_limit, file_limit,
-                           fulldiff, ign_whitespace_lcl, context_lcl):
+                           fulldiff, hide_whitespace_changes, diff_context):
         vcs_diff = source_scm.get_diff(
             commit1, commit2,
-            ignore_whitespace=ign_whitespace_lcl,
-            context=context_lcl)
+            ignore_whitespace=hide_whitespace_changes,
+            context=diff_context)
 
         diff_processor = diffs.DiffProcessor(
             vcs_diff, format='newdiff', diff_limit=diff_limit,
@@ -280,6 +281,11 b' class RepoPullRequestsView(RepoAppView, '
         from_version = self.request.GET.get('from_version') or version
         merge_checks = self.request.GET.get('merge_checks')
         c.fulldiff = str2bool(self.request.GET.get('fulldiff'))
+
+        # fetch global flags of ignore ws or context lines
+        diff_context = diffs.get_diff_context(self.request)
+        hide_whitespace_changes = diffs.get_diff_whitespace_flag(self.request)
+
         force_refresh = str2bool(self.request.GET.get('force_refresh'))
 
         (pull_request_latest,
@@ -490,7 +496,8 b' class RepoPullRequestsView(RepoAppView, '
         cache_path = self.rhodecode_vcs_repo.get_create_shadow_cache_pr_path(target_repo)
         cache_file_path = diff_cache_exist(
             cache_path, 'pull_request', pull_request_id, version_normalized,
-            from_version_normalized, source_ref_id, target_ref_id, c.fulldiff)
+            from_version_normalized, source_ref_id, target_ref_id,
+            hide_whitespace_changes, diff_context, c.fulldiff)
 
         caching_enabled = self._is_diff_cache_enabled(c.target_repo)
         force_recache = self.get_recache_flag()
@@ -561,7 +568,8 b' class RepoPullRequestsView(RepoAppView, '
                     c.source_repo.repo_name, commits_source_repo,
                     source_ref_id, target_ref_id,
                     target_commit, source_commit,
-                    diff_limit, file_limit, c.fulldiff)
+                    diff_limit, file_limit, c.fulldiff,
+                    hide_whitespace_changes, diff_context)
 
                 # save cached diff
                 if caching_enabled:

rhodecode/lib/diffs.py

@@ -41,7 +41,16 b' log = logging.getLogger(__name__)'
 
 # define max context, a file with more than this numbers of lines is unusable
 # in browser anyway
-MAX_CONTEXT = 1024 * 1014
+MAX_CONTEXT = 20 * 1024
+DEFAULT_CONTEXT = 3
+
+
+def get_diff_context(request):
+    return MAX_CONTEXT if request.GET.get('fullcontext', '') == '1' else DEFAULT_CONTEXT
+
+
+def get_diff_whitespace_flag(request):
+    return request.GET.get('ignorews', '') == '1'
 
 
 class OPS(object):

rhodecode/model/pull_request.py

@@ -73,7 +73,7 b' class PullRequestModel(BaseModel):'
 
     cls = PullRequest
 
-    DIFF_CONTEXT = 3
+    DIFF_CONTEXT = diffs.DEFAULT_CONTEXT
 
     MERGE_STATUS_MESSAGES = {
         MergeFailureReason.NONE: lazy_ugettext(
@@ -877,19 +877,21 b' class PullRequestModel(BaseModel):'
         diff_context = (
             self.DIFF_CONTEXT +
             CommentsModel.needed_extra_diff_context())
-
+        hide_whitespace_changes = False
         source_repo = pull_request_version.source_repo
         source_ref_id = pull_request_version.source_ref_parts.commit_id
         target_ref_id = pull_request_version.target_ref_parts.commit_id
         old_diff = self._get_diff_from_pr_or_version(
-            source_repo, source_ref_id, target_ref_id, context=diff_context)
+            source_repo, source_ref_id, target_ref_id,
+            hide_whitespace_changes=hide_whitespace_changes, diff_context=diff_context)
 
         source_repo = pull_request.source_repo
         source_ref_id = pull_request.source_ref_parts.commit_id
         target_ref_id = pull_request.target_ref_parts.commit_id
 
         new_diff = self._get_diff_from_pr_or_version(
-            source_repo, source_ref_id, target_ref_id, context=diff_context)
+            source_repo, source_ref_id, target_ref_id,
+            hide_whitespace_changes=hide_whitespace_changes, diff_context=diff_context)
 
         old_diff_data = diffs.DiffProcessor(old_diff)
         old_diff_data.prepare()
@@ -1488,12 +1490,17 b' class PullRequestModel(BaseModel):'
                 raise EmptyRepositoryError()
         return groups, selected
 
-    def get_diff(self, source_repo, source_ref_id, target_ref_id, context=DIFF_CONTEXT):
+    def get_diff(self, source_repo, source_ref_id, target_ref_id,
+                 hide_whitespace_changes, diff_context):
+
         return self._get_diff_from_pr_or_version(
-            source_repo, source_ref_id, target_ref_id, context=context)
+            source_repo, source_ref_id, target_ref_id,
+            hide_whitespace_changes=hide_whitespace_changes, diff_context=diff_context)
 
     def _get_diff_from_pr_or_version(
-            self, source_repo, source_ref_id, target_ref_id, context):
+            self, source_repo, source_ref_id, target_ref_id,
+            hide_whitespace_changes, diff_context):
+
         target_commit = source_repo.get_commit(
             commit_id=safe_str(target_ref_id))
         source_commit = source_repo.get_commit(
@@ -1517,7 +1524,8 b' class PullRequestModel(BaseModel):'
                   safe_unicode(vcs_repo.path))
 
         vcs_diff = vcs_repo.get_diff(
-            commit1=target_commit, commit2=source_commit, context=context)
+            commit1=target_commit, commit2=source_commit,
+            ignore_whitespace=hide_whitespace_changes, context=diff_context)
         return vcs_diff
 
     def _is_merge_enabled(self, pull_request):

rhodecode/templates/changeset/changeset.mako

@@ -150,10 +150,6 b''
               <a href="${h.route_path('repo_commit_download',repo_name=c.repo_name,commit_id=c.commit.raw_id,_query=dict(diff='download'))}" class="tooltip" title="${h.tooltip(_('Download diff'))}">
                 ${_('Download Diff')}
               </a>
-               |
-              ${c.ignorews_url(request)}
-               |
-              ${c.context_url(request)}
             </div>
         </div>
       </div>

rhodecode/templates/changeset/changeset_range.mako

@@ -68,29 +68,6 b''
             </div>
           </div>
 
-         <%doc>
-         ##TODO(marcink): implement this and diff menus
-          <div class="fieldset">
-            <div class="left-label">
-              ${_('Diff options')}:
-            </div>
-            <div class="right-content">
-                <div class="diff-actions">
-                  <a href="${h.route_path('repo_commit_raw',repo_name=c.repo_name,commit_id='?')}"  class="tooltip" title="${h.tooltip(_('Raw diff'))}">
-                    ${_('Raw Diff')}
-                  </a>
-                   |
-                  <a href="${h.route_path('repo_commit_patch',repo_name=c.repo_name,commit_id='?')}"  class="tooltip" title="${h.tooltip(_('Patch diff'))}">
-                    ${_('Patch Diff')}
-                  </a>
-                   |
-                  <a href="${h.route_path('repo_commit_download',repo_name=c.repo_name,commit_id='?',_query=dict(diff='download'))}" class="tooltip" title="${h.tooltip(_('Download diff'))}">
-                    ${_('Download Diff')}
-                  </a>
-                </div>
-            </div>
-          </div>
-        </%doc>
         </div> <!-- end summary-detail -->
 
     </div> <!-- end summary -->

rhodecode/templates/codeblocks/diffs.mako

@@ -467,46 +467,23 b' from rhodecode.lib.diffs import NEW_FILE'
             title="${h.tooltip(_('Show file at commit: %(commit_id)s') % {'commit_id': filediff.diffset.target_ref[:12]})}"
         >
             ${_('Show file after')}
-        </a> |
+        </a>
     %else:
         <span
             class="tooltip"
             title="${h.tooltip(_('File no longer present at commit: %(commit_id)s') % {'commit_id': filediff.diffset.target_ref[:12]})}"
             >
                 ${_('Show file after')}
-        </span> |
-    %endif
-        <a
-            class="tooltip"
-            title="${h.tooltip(_('Raw diff'))}"
-            href="${h.route_path('repo_files_diff',repo_name=filediff.diffset.repo_name,f_path=filediff.target_file_path, _query=dict(diff2=filediff.diffset.target_ref,diff1=filediff.diffset.source_ref,diff='raw'))}"
-        >
-            ${_('Raw diff')}
-        </a> |
-        <a
-            class="tooltip"
-            title="${h.tooltip(_('Download diff'))}"
-            href="${h.route_path('repo_files_diff',repo_name=filediff.diffset.repo_name,f_path=filediff.target_file_path, _query=dict(diff2=filediff.diffset.target_ref,diff1=filediff.diffset.source_ref,diff='download'))}"
-        >
-            ${_('Download diff')}
-        </a>
-        % if use_comments:
-            |
-        % endif
-
-        ## TODO: dan: refactor ignorews_url and context_url into the diff renderer same as diffmode=unified/sideside. Also use ajax to load more context (by clicking hunks)
-        %if hasattr(c, 'ignorews_url'):
-        ${c.ignorews_url(request, h.FID(filediff.raw_id, filediff.patch['filename']))}
-        %endif
-        %if hasattr(c, 'context_url'):
-        ${c.context_url(request, h.FID(filediff.raw_id, filediff.patch['filename']))}
+        </span>
         %endif
 
         %if use_comments:
+        |
         <a href="#" onclick="return Rhodecode.comments.toggleComments(this);">
             <span class="show-comment-button">${_('Show comments')}</span><span class="hide-comment-button">${_('Hide comments')}</span>
         </a>
         %endif
+
 %endif
     </div>
 </%def>
@@ -743,17 +720,24 b' def get_comments_for(diff_type, comments'
             <div class="pull-right">
             <div class="btn-group">
 
+                ## DIFF OPTIONS via Select2
+                <div class="pull-left">
+                ${h.hidden('diff_menu')}
+                </div>
+
                 <a
                   class="btn ${(c.user_session_attrs["diffmode"] == 'sideside' and 'btn-primary')} tooltip"
                   title="${h.tooltip(_('View side by side'))}"
                   href="${h.current_route_path(request, diffmode='sideside')}">
                     <span>${_('Side by Side')}</span>
                 </a>
+
                 <a
                   class="btn ${(c.user_session_attrs["diffmode"] == 'unified' and 'btn-primary')} tooltip"
                   title="${h.tooltip(_('View unified'))}" href="${h.current_route_path(request, diffmode='unified')}">
                     <span>${_('Unified')}</span>
                 </a>
+
                 % if range_diff_on is True:
                     <a
                       title="${_('Turn off: Show the diff as commit range')}"
@@ -784,11 +768,6 b' def get_comments_for(diff_type, comments'
                   class="btn"
                   href="#"
                   onclick="$('input[class=filediff-collapse-state]').prop('checked', true); updateSticky(); return false">${_('Collapse All Files')}</a>
-              <a
-                  class="btn"
-                  href="#"
-                  onclick="updateSticky();return Rhodecode.comments.toggleWideMode(this)">${_('Wide Mode Diff')}</a>
-
           </div>
             </div>
         </div>
@@ -963,6 +942,72 b' def get_comments_for(diff_type, comments'
                 }
             );
 
+            var preloadData = {
+                results: [
+                    ## Wide diff mode
+                    {
+                        id: 1,
+                        text: _gettext('Toggle Wide Mode Diff'),
+                        action: function () {
+                            updateSticky();
+                            Rhodecode.comments.toggleWideMode(this);
+                            return null;
+                        },
+                        url: null,
+                    },
+
+                    ## Whitespace change
+                    % if request.GET.get('ignorews', '') == '1':
+                    {
+                        id: 2,
+                        text: _gettext('Show whitespace changes'),
+                        action: function () {},
+                        url: "${h.current_route_path(request, ignorews=0)|n}"
+                    },
+                    % else:
+                    {
+                        id: 2,
+                        text: _gettext('Hide whitespace changes'),
+                        action: function () {},
+                        url: "${h.current_route_path(request, ignorews=1)|n}"
+                    },
+                    % endif
+
+                    ## FULL CONTEXT
+                    % if request.GET.get('fullcontext', '') == '1':
+                    {
+                        id: 3,
+                        text: _gettext('Hide full context diff'),
+                        action: function () {},
+                        url: "${h.current_route_path(request, fullcontext=0)|n}"
+                    },
+                    % else:
+                    {
+                        id: 3,
+                        text: _gettext('Show full context diff'),
+                        action: function () {},
+                        url: "${h.current_route_path(request, fullcontext=1)|n}"
+                    },
+                    % endif
+
+                ]
+            };
+
+            $("#diff_menu").select2({
+                minimumResultsForSearch: -1,
+                containerCssClass: "drop-menu",
+                dropdownCssClass: "drop-menu-dropdown",
+                dropdownAutoWidth: true,
+                data: preloadData,
+                placeholder: "${_('Diff Options')}",
+            });
+            $("#diff_menu").on('select2-selecting', function (e) {
+                e.choice.action();
+                if (e.choice.url !== null) {
+                    window.location = e.choice.url
+                }
+            });
+            
         });
 
     </script>

rhodecode/templates/compare/compare_diff.mako

@@ -129,30 +129,6 b''
                 </div>
               </div>
 
-              <%doc>
-              ##TODO(marcink): implement this and diff menus
-              <div class="fieldset">
-                <div class="left-label">
-                  ${_('Diff options')}:
-                </div>
-                <div class="right-content">
-                    <div class="diff-actions">
-                      <a href="${h.route_path('repo_commit_raw',repo_name=c.repo_name,commit_id='?')}"  class="tooltip" title="${h.tooltip(_('Raw diff'))}">
-                        ${_('Raw Diff')}
-                      </a>
-                       |
-                      <a href="${h.route_path('repo_commit_patch',repo_name=c.repo_name,commit_id='?')}"  class="tooltip" title="${h.tooltip(_('Patch diff'))}">
-                        ${_('Patch Diff')}
-                      </a>
-                       |
-                      <a href="${h.route_path('repo_commit_download',repo_name=c.repo_name,commit_id='?',_query=dict(diff='download'))}" class="tooltip" title="${h.tooltip(_('Download diff'))}">
-                        ${_('Download Diff')}
-                      </a>
-                    </div>
-                </div>
-              </div>
-              </%doc>
-
               ## commit status form
               <div class="fieldset" id="compare_changeset_status" style="display: none; margin-bottom: -80px;">
                 <div class="left-label">

rhodecode/tests/models/test_pullrequest.py

@@ -373,7 +373,8 b' class TestPullRequestModel(object):'
         source_ref_id = pull_request.source_ref_parts.commit_id
         target_ref_id = pull_request.target_ref_parts.commit_id
         diff = PullRequestModel()._get_diff_from_pr_or_version(
-            source_repo, source_ref_id, target_ref_id, context=6)
+            source_repo, source_ref_id, target_ref_id,
+            hide_whitespace_changes=False, diff_context=6)
         assert 'file_1' in diff.raw
 
     def test_generate_title_returns_unicode(self):

rhodecode/tests/models/test_pullrequest_git.py

@@ -60,7 +60,8 b' class TestGetDiffForPrOrVersion(object):'
         source_ref_id = pr_or_version.source_ref_parts.commit_id
         target_ref_id = pr_or_version.target_ref_parts.commit_id
         diff = PullRequestModel()._get_diff_from_pr_or_version(
-            source_repo, source_ref_id, target_ref_id, context=6)
+            source_repo, source_ref_id, target_ref_id,
+            hide_whitespace_changes=False, diff_context=6)
         assert 'file_b' in diff.raw
 
     def assert_commit_cannot_be_accessed(