Edgewall Software
Modify

Opened 16 years ago

Closed 16 years ago

Last modified 14 years ago

#2028 closed task (fixed)

Integrate anydiff branch in the trunk

Reported by: Christian Boos Owned by: Christian Boos
Priority: high Milestone: 0.10
Component: version control/changeset view Version: devel
Severity: major Keywords: branch tracdiff review
Cc: vyt@…, pixelpapst@…, jouvin@…, jeremie.corbier@… Branch:
Release Notes:
API Changes:
Internal Changes:

Description

The anydiff branch is now quite complete and functional.

It implements the following features:

  • diffing between any revision of a given file or directory
  • diffing between any arbitrary pair of (path,revision) of the same type (two files or two directories)
  • viewing the last change that occurred at a given path (the restricted changeset view)
  • downloading a .zip of any folder in the repository

The TracDiff wiki page contains a more in-depth documentation for the above.

I'll also use this ticket for attaching patches against the different 0.9 releases.

Attachments (10)

trac-0.9b1.tar.gz-anydiff.patch (83.6 KB ) - added by Christian Boos 16 years ago.
Adds TracDiff (r2210) functionality to Trac 0.9b1 (applies cleanly to the trac-0.9b1.tar.gz archive)
trac-0.9b1.zip-anydiff.patch (83.6 KB ) - added by Christian Boos 16 years ago.
Adds TracDiff (r2210) functionality to Trac 0.9b1 (applies cleanly to the trac-0.9b1.zip archive)
trac-0.9b1-anydiff.patch (83.6 KB ) - added by Christian Boos 16 years ago.
Adds TracDiff (r2213) functionality to Trac 0.9b1 (applies cleanly to recent .zip and .tar.gz archives, those containing the wiki-macros directory)
trac-0.9b2-anydiff-r2306.patch (92.1 KB ) - added by Christian Boos 16 years ago.
Patch adding the TracDiff features to trac-0.9b2
trac-diff-for-0.9.patch (91.6 KB ) - added by Christian Boos 16 years ago.
Adds theTracDiff features on top of trac-0.9
view_changes_button.png (16.0 KB ) - added by Christian Boos 16 years ago.
Screenshot of the Revision Log view (web browser is Firefox)
trac-diff-for-0.9.3.patch (50.7 KB ) - added by jeremie.corbier@… 16 years ago.
Adds the TracDiff features on top of the 0.9.3
trac-diff-for-0.9.3.2.patch (87.4 KB ) - added by jeremie.corbier@… 16 years ago.
I forgot to add the £$*! N switch for diff, sorry
trac-diff-for-0.9.3.2.2.patch (85.5 KB ) - added by jouvin@… 16 years ago.
trac-diff-for-0.9.4.patch (85.4 KB ) - added by trac-project@… 16 years ago.
Updated patch to apply against 0.9.4

Download all attachments as: .zip

Change History (26)

comment:1 by Christian Boos, 16 years ago

Status: newassigned

To apply the above patches, go in you trac-0.9b1 directory, and in the shell, do:

.../trac-0.9b1> patch -p1 < ../trac-0.9b1.tar.gz-anydiff.patch

Note that due to the fact that the trac/versioncontrol/web_ui/__init__.py has different line endings in the .zip and the .tar.gz, I had to provide two different patches.

by Christian Boos, 16 years ago

Adds TracDiff (r2210) functionality to Trac 0.9b1 (applies cleanly to the trac-0.9b1.tar.gz archive)

by Christian Boos, 16 years ago

Adds TracDiff (r2210) functionality to Trac 0.9b1 (applies cleanly to the trac-0.9b1.zip archive)

comment:2 by Matthew Good, 16 years ago

I'll have to take another look at the diff feature since it's been a while since I've tested this branch. I think this is definitely a good feature to have, but I've found the UI for it in the Wiki a bit awkward. Most of the time I only want to see a single change, which is ok if it's the latest change since it only requires one click of the button (although as mentioned elsewhere having the button at the bottom of the page is not optimal), however now if I want to view any other change I have to click two different radio buttons and submit making three clicks just to view a single change. If you've come up with a better UI for this in the browser I'll have to take a look, but IIRC it was pretty similar. I haven't come up with a better alternative yet, but in the majority of cases I've found the Wiki diffing to be more cumbersome than it was before.

I'm also still quite sceptical about the ZIP downloading. I think the main reasons this was requested was as a round-about solution to either proxy issues that got in the way of a checkout, or content-type issues downloading certain files. In these cases I think the ZIP option is sort of a hack and not really the right way to solve them. It also seems like this could easily suck up a lot of resources on a decent sized repository. I think projects would be much better off setting up a job to make a nightly tarball rather than constantly creating ZIP files when someone requests it through Trac. The ZIP downloading would also not handle things like a regular checkout, so keyword expansion is an issue, as well as things like fetching svn:external resources.

comment:3 by Christian Boos, 16 years ago

Concerning you first point, if you want to actually look at the latest change only, it's even faster with the new UI: in the browser, you have a direct page link to the Last Change that occurred on the path being browsed… It's only if you actually want to see a more complex set of changes, that you need to go in the Revision Log, and select two arbitrary revisions there.

Also, I agree that the ZIP downloading can be resource intensive, so maybe making this feature optional would be useful.

by Christian Boos, 16 years ago

Attachment: trac-0.9b1-anydiff.patch added

Adds TracDiff (r2213) functionality to Trac 0.9b1 (applies cleanly to recent .zip and .tar.gz archives, those containing the wiki-macros directory)

by Christian Boos, 16 years ago

Patch adding the TracDiff features to trac-0.9b2

by Christian Boos, 16 years ago

Attachment: trac-diff-for-0.9.patch added

Adds theTracDiff features on top of trac-0.9

comment:4 by Christian Boos, 16 years ago

Priority: normalhigh

The TracDiff branch is now located in source:sandbox/trac-diff

Please review.

comment:5 by Christopher Lenz, 16 years ago

Okay, a couple of initial comments, without having looked at the code very closely:

  1. Is there a reason that the radio buttons in the revision log are layed out different from those in the wiki page history? Personally, I prefer the wiki version because it seems visually cleaner (less clutter).
  2. The placement of the top "View changes" button on the revision log page is bad: it is too far away from the table to make any intuitive sense. Also, this is related to #2095. For now, I'd suggest removing the top button and addressing the control redundancy issue later.
  3. Performing a diff from the form for arbitrary diffs delivers a page via POST, which is thus not bookmarkable, and will warn about reloading etc. There should be either a redirect, or better, simply use a GET-based form in the first place.
  4. The omission of the View changes button from the root of the repository seems like a fairly arbitrary decision. What's the rationale?
  5. I'm somewhat worried that the additional diff support can be used to request huge pages that are pretty expensive to generate, and thus be a good candidate for DOS attacks. Any ideas?
  6. It should be Latest change instead of Last change.
  7. The heading of the diff page can easily become huge for a long path. Not sure what can be done here, but somehow they need to be less verbose, probably by moving some of the information into the content area.

by Christian Boos, 16 years ago

Attachment: view_changes_button.png added

Screenshot of the Revision Log view (web browser is Firefox)

comment:6 by Christian Boos, 16 years ago

(replies to the above)

  1. The reason is that the revision log layout predates the one from the wiki diff, therefore I couldnt' copy it :) Now I did: r2484
  2. Well, the View changes is actually just above the table, quite close the versions selected by default, or do I miss something?

    Screenshot of the Revision Log view (web browser is Firefox)
  3. r2485
  4. The rationale is still from the time the View changes did a selection of the Base for diff, where it generally didn't make sense to select the root. Now that we have a form, the reason is gone: fixed in r2486
  5. The solution for that would be to leave the actual content of the diff empty and have a link that would either lead to a page containing specifically that diff, or the browser supports it, replace dynamically the empty content by the actual diff (would be #515)
  6. Well, it's actually the Latest change relative to the changeset being browsed. I think that Latest change makes you believe you'll get the very latest change. For that, you first have to click on View Latest Revision (which, until recently, conveyed a similar ambiguity, since r2382 it's really the "latest"). That's why I used Last change. Maybe Previous change would be better?
  7. The "at revision" can be squeezed into "@", as we are now using that convention also for source: TracLinks

comment:7 by Christopher Lenz, 16 years ago

Well, the View changes is actually just above the table, quite close the versions selected by default, or do I miss something?

No I missed something, probably a cached stylesheet. :-P

The solution for that would be to leave the actual content of the diff empty and have a link that would either lead to a page containing specifically that diff ![…]

So we'll defer the decision on this issue…

The "at revision" can be squeezed into "@", ![…]

How about moving the revision range into the diff overview instead? IMHO, the @ looks pretty cryptic in the UI (yeah, also on the revision log page).

comment:8 by Christian Boos, 16 years ago

Keywords: tracdiff review added

comment:9 by vyt@…, 16 years ago

Cc: vyt@… added

IMHO, it's very, very useful feature.

comment:10 by pixelpapst@…, 16 years ago

Cc: pixelpapst@… added

by jeremie.corbier@…, 16 years ago

Attachment: trac-diff-for-0.9.3.patch added

Adds the TracDiff features on top of the 0.9.3

by jeremie.corbier@…, 16 years ago

Attachment: trac-diff-for-0.9.3.2.patch added

I forgot to add the £$*! N switch for diff, sorry

comment:11 by anonymous, 16 years ago

Cc: jouvin@… added

by jouvin@…, 16 years ago

comment:12 by jouvin, 16 years ago

Jeremy,

I updated your anydiff patch for 0.9.3. There are 2 changes :

  • File names are relative to Trac distribution top directory. This is easier to apply the patch, whatever is your directory name.
  • I suppressed siteconfig.py from the the patch. This file is not part of the standard distribution.

comment:13 by Christopher Lenz, 16 years ago

Milestone: 1.00.10

comment:14 by anonymous, 16 years ago

Cc: jeremie.corbier@… added

Thanks for cleaning up the patch.

Next time, I'll pay a little more attention to what I do.

comment:15 by Christian Boos, 16 years ago

The source:sandbox/trac-diff branch has been updated with changes from trunk and should be ready for being integrated in trunk.

comment:16 by Christian Boos, 16 years ago

Resolution: fixed
Status: assignedclosed

The source:sandbox/trac-diff branch has been merged in the trunk as of [2808].

Please let me know if you're interested in a patch for 0.9.[34] (but I'd encourage you to try out the trunk, which I think is currently equally "stable" and has more features…)

by trac-project@…, 16 years ago

Attachment: trac-diff-for-0.9.4.patch added

Updated patch to apply against 0.9.4

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christian Boos to the specified user.

Add Comment


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