Edgewall Software

Opened 2 years ago

Last modified 2 years ago

#13083 new enhancement

Unified diff with contextall

Reported by: anonymous Owned by:
Priority: normal Milestone: undecided
Component: version control/changeset view Version:
Severity: normal Keywords: diff
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description (last modified by Jun Omae)

The changeset view (example 3: changeset:16824) allows to "Show the changes in full context" via ?contextall=1 (example 2: changeset:16824?contextall=1).

It also has a link to "Download in other formats: Unified Diff" (example 3: changeset:16824?format=diff&new=16824).

But combining these two features is not currently possible. On the example 2 page the contextall=1 parameter is not included in the Unified Diff link. And even if it is manually added it does not seem to have any effect (example 4: changeset:16824?format=diff&new=16824&contextall=1).

Would it be possible to make this work? This is very useful for people that prefer using an offline diff viewer tool.

Attachments (0)

Change History (3)

comment:1 by Jun Omae, 2 years ago

Description: modified (diff)
Milestone: undecided

Currently, Unified diff link in Download in other formats doesn't respect all options of prefs form in changeset view (e.g. Blank lines, …).

I think we should keep it but it might be a bit nice to show another unified diff link respected the options.

comment:2 by anonymous, 2 years ago

  • trac/versioncontrol/web_ui/changeset.py

    diff -r ec566f6d999f trac/versioncontrol/web_ui/changeset.py
    a b  
    349349        self._render_html(req, repos, chgset, restricted, data)
    351351        if chgset:
    352             diff_params = 'new=%s' % new
     352            diff_params = {'new': new}
    353353        else:
    354             diff_params = unicode_urlencode({
     354            diff_params = {
    355355                'new_path': full_new_path, 'new': new,
    356                 'old_path': full_old_path, 'old': old})
    357         add_link(req, 'alternate', '?format=diff&' + diff_params,
     356                'old_path': full_old_path, 'old': old}
     357        diff_params_without_opts = unicode_urlencode(diff_params)
     358        diff_params.update(diff_opts)
     359        diff_params_with_opts = unicode_urlencode(diff_params)
     360        add_link(req, 'alternate', '?format=diff&' + diff_params_with_opts,
    358361                 _('Unified Diff'), 'text/plain', 'diff')
    359         add_link(req, 'alternate', '?format=zip&' + diff_params,
     362        if diff_params_with_opts != diff_params_without_opts:
     363            add_link(req, 'alternate', '?format=diff&' + diff_params_without_opts,
     364                     _('Plain Unified Diff'), 'text/plain', 'diff')
     365        add_link(req, 'alternate', '?format=zip&' + diff_params_without_opts,
    360366                 _('Zip Archive'), 'application/zip', 'zip')
    361367        add_script(req, 'common/js/diff.js')
    362368        add_stylesheet(req, 'common/css/changeset.css')
    747753                options = data['diff']['options']
    748754                context = options.get('contextlines', 3)
    749755                if context < 0 or options.get('contextall'):
    750                     context = 3  # FIXME: unified_diff bugs with context=None
     756                    context = None
    751757                ignore_blank_lines = options.get('ignoreblanklines')
    752758                ignore_case = options.get('ignorecase')
    753759                ignore_space = options.get('ignorewhitespace')

This seems to work. Does anyone know what that FIXME comment refers to? It is very old: It was first(?) added here and then (via this move) here. This and this may also be related. But I see no hint about what the "bug" is or was. Is it still relevant?

comment:3 by Jun Omae, 2 years ago

Thanks for the patch and investigation.

I re-investigated the unified diff link.

The context lines, ignore blank lines, ignore case changes and ignore white space changes options are saved in the user session when prefs form is submitted and the unified diff link uses the saved options. However, context all option is not saved in the user session. The behavior is introduced in r9322 and comment:2:ticket:6423.

Then, I consider the unified diff link should respect contextall=1 parameter.

[d87e6dc3c/jomae.git] (I'll add unit tests).

Another thing, I noticed, after submitting negative integer or non-digits in context lines textbox, showing all in the textbox and the changeset view shows contextall diff. I think we should remove the weird behavior or Show the changes in full context radio should be selected.

Modify Ticket

Change Properties
Set your email in Preferences
as new The ticket will remain with no owner.
The ticket will be disowned.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment

E-mail address and name can be saved in the Preferences .
Note: See TracTickets for help on using tickets.