Changeset - 35af0bd45bf3
[Not reviewed]
default
0 8 0
Mads Kiilerich (mads) - 6 years ago 2020-06-21 23:20:12
mads@kiilerich.com
diff: drop per file ignore-whitespace and context - it didn't work and had conceptual issue (Issue #344)

Diffs are currently generated at the low level as one big diff between two vcs
resisions, provided global values for diff context size and flag for ignoring
whitespace. All files use the same flags. There is no way to actually compute
the full diff using these use per file flags, and no simple and efficient way
to add it.

The best option is thus to drop the failed attempt at making it per file, and
just rely on the simple global flags in the URL.

The links for changing whitespace and context is sometimes shown for the whole
"page", and sometimes next to the diff for one file. For now, keep showing the
link in these places, but make sure it navigates back to the FID of the section
where the link was clicked.

The implementation is completely rewritten and moved to a more appropriate
location in helpers.

With a more clean implementation, we also consistently use the simple getters
to extract values from the URL.
8 files changed with 58 insertions and 146 deletions:
0 comments (0 inline, 0 general)
kallithea/controllers/changeset.py
Show inline comments
 
@@ -28,7 +28,7 @@ Original author and date, and relevant c
 
import binascii
 
import logging
 
import traceback
 
from collections import OrderedDict, defaultdict
 
from collections import OrderedDict
 

	
 
from tg import request, response
 
from tg import tmpl_context as c
 
@@ -54,113 +54,6 @@ from kallithea.model.pull_request import
 
log = logging.getLogger(__name__)
 

	
 

	
 
def _update_with_GET(params, GET):
 
    for k in ['diff1', 'diff2', 'diff']:
 
        params[k] += GET.getall(k)
 

	
 

	
 
def get_ignore_ws(fid, GET):
 
    ig_ws_global = GET.get('ignorews')
 
    ig_ws = [k for k in GET.getall(fid) if k.startswith('WS')]
 
    if ig_ws:
 
        try:
 
            return int(ig_ws[0].split(':')[-1])
 
        except ValueError:
 
            raise HTTPBadRequest()
 
    return ig_ws_global
 

	
 

	
 
def _ignorews_url(GET, fileid=None, anchor=None):
 
    fileid = str(fileid) if fileid else None
 
    params = defaultdict(list)
 
    _update_with_GET(params, GET)
 
    lbl = _('Show whitespace')
 
    ig_ws = get_ignore_ws(fileid, GET)
 
    ln_ctx = get_line_ctx(fileid, GET)
 
    # global option
 
    if fileid is None:
 
        if ig_ws is None:
 
            params['ignorews'] += [1]
 
            lbl = _('Ignore whitespace')
 
        ctx_key = 'context'
 
        ctx_val = ln_ctx
 
    # per file options
 
    else:
 
        if ig_ws is None:
 
            params[fileid] += ['WS:1']
 
            lbl = _('Ignore whitespace')
 

	
 
        ctx_key = fileid
 
        ctx_val = 'C:%s' % ln_ctx
 
    # if we have passed in ln_ctx pass it along to our params
 
    if ln_ctx:
 
        params[ctx_key] += [ctx_val]
 

	
 
    params['anchor'] = anchor
 
    icon = h.literal('<i class="icon-strike"></i>')
 
    return h.link_to(icon, h.url.current(**params), title=lbl, **{'data-toggle': 'tooltip'})
 

	
 

	
 
def get_line_ctx(fid, GET):
 
    ln_ctx_global = GET.get('context')
 
    if fid:
 
        ln_ctx = [k for k in GET.getall(fid) if k.startswith('C')]
 
    else:
 
        _ln_ctx = [k for k in GET if k.startswith('C')]
 
        ln_ctx = 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 int(retval)
 
    except Exception:
 
        return 3
 

	
 

	
 
def _context_url(GET, fileid=None, anchor=None):
 
    """
 
    Generates url for context lines
 

	
 
    :param fileid:
 
    """
 

	
 
    fileid = str(fileid) if fileid else None
 
    ig_ws = get_ignore_ws(fileid, GET)
 
    ln_ctx = (get_line_ctx(fileid, GET) or 3) * 2
 

	
 
    params = defaultdict(list)
 
    _update_with_GET(params, GET)
 

	
 
    # global option
 
    if fileid is None:
 
        if ln_ctx > 0:
 
            params['context'] += [ln_ctx]
 

	
 
        if ig_ws:
 
            ig_ws_key = 'ignorews'
 
            ig_ws_val = 1
 

	
 
    # per file option
 
    else:
 
        params[fileid] += ['C:%s' % ln_ctx]
 
        ig_ws_key = fileid
 
        ig_ws_val = 'WS:%s' % 1
 

	
 
    if ig_ws:
 
        params[ig_ws_key] += [ig_ws_val]
 

	
 
    lbl = _('Increase diff context to %(num)s lines') % {'num': ln_ctx}
 

	
 
    params['anchor'] = anchor
 
    icon = h.literal('<i class="icon-sort"></i>')
 
    return h.link_to(icon, h.url.current(**params), title=lbl, **{'data-toggle': 'tooltip'})
 

	
 

	
 
def create_cs_pr_comment(repo_name, revision=None, pull_request=None, allowed_to_change_status=True):
 
    """
 
    Add a comment to the specified changeset or pull request, using POST values
 
@@ -287,8 +180,6 @@ class ChangesetController(BaseRepoContro
 

	
 
    def _index(self, revision, method):
 
        c.pull_request = None
 
        c.ignorews_url = _ignorews_url
 
        c.context_url = _context_url
 
        c.fulldiff = request.GET.get('fulldiff') # for reporting number of changed files
 
        # get ranges of revisions if preset
 
        rev_range = revision.split('...')[:2]
 
@@ -351,9 +242,8 @@ class ChangesetController(BaseRepoContro
 

	
 
            cs2 = changeset.raw_id
 
            cs1 = changeset.parents[0].raw_id if changeset.parents else EmptyChangeset().raw_id
 
            diff_context_size = get_line_ctx('', request.GET)
 
            ignore_whitespace_diff = get_ignore_ws('', request.GET)
 

	
 
            ignore_whitespace_diff = h.get_ignore_whitespace_diff(request.GET)
 
            diff_context_size = h.get_diff_context_size(request.GET)
 
            raw_diff = diffs.get_diff(c.db_repo_scm_instance, cs1, cs2,
 
                ignore_whitespace=ignore_whitespace_diff, context=diff_context_size)
 
            diff_limit = None if c.fulldiff else self.cut_off_limit
kallithea/controllers/compare.py
Show inline comments
 
@@ -37,13 +37,12 @@ from tg.i18n import ugettext as _
 
from webob.exc import HTTPBadRequest, HTTPFound, HTTPNotFound
 

	
 
from kallithea.config.routing import url
 
from kallithea.controllers.changeset import _context_url, _ignorews_url
 
from kallithea.lib import diffs
 
from kallithea.lib import helpers as h
 
from kallithea.lib.auth import HasRepoPermissionLevelDecorator, LoginRequired
 
from kallithea.lib.base import BaseRepoController, render
 
from kallithea.lib.graphmod import graph_data
 
from kallithea.lib.utils2 import ascii_bytes, ascii_str, safe_bytes, safe_int
 
from kallithea.lib.utils2 import ascii_bytes, ascii_str, safe_bytes
 
from kallithea.model.db import Repository
 

	
 

	
 
@@ -208,12 +207,8 @@ class CompareController(BaseRepoControll
 
            other_repo=c.a_repo.repo_name,
 
            other_ref_type=org_ref_type, other_ref_name=org_ref_name,
 
            merge=merge or '')
 

	
 
        # set callbacks for generating markup for icons
 
        c.ignorews_url = _ignorews_url
 
        c.context_url = _context_url
 
        ignore_whitespace_diff = request.GET.get('ignorews') == '1'
 
        diff_context_size = safe_int(request.GET.get('context'), 3)
 
        ignore_whitespace_diff = h.get_ignore_whitespace_diff(request.GET)
 
        diff_context_size = h.get_diff_context_size(request.GET)
 

	
 
        c.a_rev = self._get_ref_rev(c.a_repo, org_ref_type, org_ref_name,
 
            returnempty=True)
kallithea/controllers/files.py
Show inline comments
 
@@ -39,14 +39,13 @@ from tg.i18n import ugettext as _
 
from webob.exc import HTTPFound, HTTPNotFound
 

	
 
from kallithea.config.routing import url
 
from kallithea.controllers.changeset import _context_url, _ignorews_url, get_ignore_ws, get_line_ctx
 
from kallithea.lib import diffs
 
from kallithea.lib import helpers as h
 
from kallithea.lib.auth import HasRepoPermissionLevelDecorator, LoginRequired
 
from kallithea.lib.base import BaseRepoController, jsonify, render
 
from kallithea.lib.exceptions import NonRelativePathError
 
from kallithea.lib.utils import action_logger
 
from kallithea.lib.utils2 import asbool, convert_line_endings, detect_mode, safe_int, safe_str
 
from kallithea.lib.utils2 import asbool, convert_line_endings, detect_mode, safe_str
 
from kallithea.lib.vcs.backends.base import EmptyChangeset
 
from kallithea.lib.vcs.conf import settings
 
from kallithea.lib.vcs.exceptions import (ChangesetDoesNotExistError, ChangesetError, EmptyRepositoryError, ImproperArchiveTypeError, NodeAlreadyExistsError,
 
@@ -558,8 +557,8 @@ class FilesController(BaseRepoController
 
    @LoginRequired(allow_default_user=True)
 
    @HasRepoPermissionLevelDecorator('read')
 
    def diff(self, repo_name, f_path):
 
        ignore_whitespace_diff = request.GET.get('ignorews') == '1'
 
        diff_context_size = safe_int(request.GET.get('context'), 3)
 
        ignore_whitespace_diff = h.get_ignore_whitespace_diff(request.GET)
 
        diff_context_size = h.get_diff_context_size(request.GET)
 
        diff2 = request.GET.get('diff2', '')
 
        diff1 = request.GET.get('diff1', '') or diff2
 
        c.action = request.GET.get('diff')
 
@@ -567,8 +566,6 @@ class FilesController(BaseRepoController
 
        c.f_path = f_path
 
        c.big_diff = False
 
        fulldiff = request.GET.get('fulldiff')
 
        c.ignorews_url = _ignorews_url
 
        c.context_url = _context_url
 
        c.changes = OrderedDict()
 
        c.changes[diff2] = []
 

	
 
@@ -641,8 +638,6 @@ class FilesController(BaseRepoController
 

	
 
        else:
 
            fid = h.FID(diff2, node2.path)
 
            diff_context_size = get_line_ctx(fid, request.GET)
 
            ignore_whitespace_diff = get_ignore_ws(fid, request.GET)
 
            diff_limit = None if fulldiff else self.cut_off_limit
 
            c.a_rev, c.cs_rev, a_path, diff, st, op = diffs.wrapped_diff(filenode_old=node1,
 
                                         filenode_new=node2,
 
@@ -651,7 +646,6 @@ class FilesController(BaseRepoController
 
                                         line_context=diff_context_size,
 
                                         enable_comments=False)
 
            c.file_diff_data = [(fid, fid, op, a_path, node2.path, diff, st)]
 

	
 
            return render('files/file_diff.html')
 

	
 
    @LoginRequired(allow_default_user=True)
kallithea/controllers/pullrequests.py
Show inline comments
 
@@ -36,7 +36,7 @@ from tg.i18n import ugettext as _
 
from webob.exc import HTTPBadRequest, HTTPForbidden, HTTPFound, HTTPNotFound
 

	
 
from kallithea.config.routing import url
 
from kallithea.controllers.changeset import _context_url, _ignorews_url, create_cs_pr_comment, delete_cs_pr_comment
 
from kallithea.controllers.changeset import create_cs_pr_comment, delete_cs_pr_comment
 
from kallithea.lib import diffs
 
from kallithea.lib import helpers as h
 
from kallithea.lib.auth import HasRepoPermissionLevelDecorator, LoginRequired
 
@@ -569,10 +569,8 @@ class PullrequestsController(BaseRepoCon
 
        c.cs_comments = c.cs_repo.get_comments(raw_ids)
 
        c.cs_statuses = c.cs_repo.statuses(raw_ids)
 

	
 
        ignore_whitespace_diff = request.GET.get('ignorews') == '1'
 
        diff_context_size = safe_int(request.GET.get('context'), 3)
 
        c.ignorews_url = _ignorews_url
 
        c.context_url = _context_url
 
        ignore_whitespace_diff = h.get_ignore_whitespace_diff(request.GET)
 
        diff_context_size = h.get_diff_context_size(request.GET)
 
        fulldiff = request.GET.get('fulldiff')
 
        diff_limit = None if fulldiff else self.cut_off_limit
 

	
kallithea/lib/helpers.py
Show inline comments
 
@@ -213,12 +213,48 @@ def FID(raw_id, path):
 
    """
 
    Creates a unique ID for filenode based on it's hash of path and revision
 
    it's safe to use in urls
 
    """
 
    return 'C-%s-%s' % (short_id(raw_id), hashlib.md5(safe_bytes(path)).hexdigest()[:12])
 

	
 
    :param raw_id:
 
    :param path:
 
    """
 

	
 
def get_ignore_whitespace_diff(GET):
 
    """Return true if URL requested whitespace to be ignored"""
 
    return bool(GET.get('ignorews'))
 

	
 

	
 
    return 'C-%s-%s' % (short_id(raw_id), hashlib.md5(safe_bytes(path)).hexdigest()[:12])
 
def ignore_whitespace_link(GET, anchor=None):
 
    """Return snippet with link to current URL with whitespace ignoring toggled"""
 
    params = dict(GET)  # ignoring duplicates
 
    if get_ignore_whitespace_diff(GET):
 
        params.pop('ignorews')
 
        title = _("Show whitespace changes")
 
    else:
 
        params['ignorews'] = '1'
 
        title = _("Ignore whitespace changes")
 
    params['anchor'] = anchor
 
    return link_to(
 
        literal('<i class="icon-strike"></i>'),
 
        url.current(**params),
 
        title=title,
 
        **{'data-toggle': 'tooltip'})
 

	
 

	
 
def get_diff_context_size(GET):
 
    """Return effective context size requested in URL"""
 
    return safe_int(GET.get('context'), default=3)
 

	
 

	
 
def increase_context_link(GET, anchor=None):
 
    """Return snippet with link to current URL with double context size"""
 
    context = get_diff_context_size(GET) * 2
 
    params = dict(GET)  # ignoring duplicates
 
    params['context'] = str(context)
 
    params['anchor'] = anchor
 
    return link_to(
 
        literal('<i class="icon-sort"></i>'),
 
        url.current(**params),
 
        title=_('Increase diff context to %(num)s lines') % {'num': context},
 
        **{'data-toggle': 'tooltip'})
 

	
 

	
 
class _FilesBreadCrumbs(object):
kallithea/templates/changeset/changeset.html
Show inline comments
 
@@ -47,8 +47,8 @@ ${self.repo_context_bar('changelog', c.c
 
                  <a href="${h.url('changeset_download_home',repo_name=c.repo_name,revision=c.changeset.raw_id,diff='download')}"
 
                     data-toggle="tooltip"
 
                     title="${_('Download diff')}"><i class="icon-floppy"></i></a>
 
                  ${c.ignorews_url(request.GET)}
 
                  ${c.context_url(request.GET)}
 
                  ${h.ignore_whitespace_link(request.GET)}
 
                  ${h.increase_context_link(request.GET)}
 
                </div>
 
        </div>
 
        <div class="panel-body">
kallithea/templates/changeset/diff_block.html
Show inline comments
 
@@ -65,8 +65,8 @@
 
                      <i class="icon-diff"></i></a>
 
                  <a href="${h.url('files_diff_home',repo_name=cs_repo_name,f_path=cs_filename,diff2=cs_rev,diff1=a_rev,diff='download')}" data-toggle="tooltip" title="${_('Download diff for this file')}">
 
                      <i class="icon-floppy"></i></a>
 
                  ${c.ignorews_url(request.GET, url_fid, id_fid)}
 
                  ${c.context_url(request.GET, url_fid, id_fid)}
 
                  ${h.ignore_whitespace_link(request.GET, id_fid)}
 
                  ${h.increase_context_link(request.GET, id_fid)}
 
                </div>
 
                <div class="pull-right">
 
                    ${_('Show inline comments')}
kallithea/templates/compare/compare_diff.html
Show inline comments
 
@@ -60,9 +60,8 @@ ${self.repo_context_bar('changelog')}
 
                % else:
 
                    ${ungettext('%s file changed with %s insertions and %s deletions','%s files changed with %s insertions and %s deletions', len(c.file_diff_data)) % (len(c.file_diff_data),c.lines_added,c.lines_deleted)}:
 
                %endif
 

	
 
                ${c.ignorews_url(request.GET)}
 
                ${c.context_url(request.GET)}
 
                ${h.ignore_whitespace_link(request.GET)}
 
                ${h.increase_context_link(request.GET)}
 
                </h5>
 
                <div class="cs_files">
 
                  %if not c.file_diff_data:
0 comments (0 inline, 0 general)