diffs: introducing diff menu for whitespace toggle and context changes

This commit is contained in:
Daniel Dourvaris 2018-10-23 17:02:26 +02:00
parent 175a9b344e
commit 94d74336d1
11 changed files with 141 additions and 206 deletions

View file

@ -34,7 +34,9 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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)

View file

@ -44,10 +44,7 @@ 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 @@ 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 @@ 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,

View file

@ -219,10 +219,11 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
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 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
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 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
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 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
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 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
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:

View file

@ -41,7 +41,16 @@ 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):

View file

@ -73,7 +73,7 @@ class PullRequestModel(BaseModel):
cls = PullRequest
DIFF_CONTEXT = 3
DIFF_CONTEXT = diffs.DEFAULT_CONTEXT
MERGE_STATUS_MESSAGES = {
MergeFailureReason.NONE: lazy_ugettext(
@ -877,19 +877,21 @@ 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 @@ 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 @@ 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):

View file

@ -150,10 +150,6 @@
<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>

View file

@ -68,29 +68,6 @@
</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 -->

View file

@ -467,46 +467,23 @@ from rhodecode.lib.diffs import NEW_FILENODE, DEL_FILENODE, \
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> |
${_('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']))}
%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
%endif
</div>
</%def>
@ -743,17 +720,24 @@ def get_comments_for(diff_type, comments, filename, line_version, line_number):
<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,12 +768,7 @@ def get_comments_for(diff_type, comments, filename, line_version, line_number):
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>
</div>
<div class="fpath-placeholder">
@ -963,6 +942,72 @@ def get_comments_for(diff_type, comments, filename, line_version, line_number):
}
);
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>

View file

@ -129,30 +129,6 @@
</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">

View file

@ -373,7 +373,8 @@ 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):

View file

@ -60,7 +60,8 @@ 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(