Edgewall Software

Ticket #2028 (closed task: fixed)

Opened 3 years ago

Last modified 4 months ago

Integrate anydiff branch in the trunk

Reported by: cboos Owned by: cboos
Priority: high Milestone: 0.10
Component: version control/changeset view Version: devel
Severity: major Keywords: branch tracdiff review
Cc: vyt@…, pixelpapst@…, jouvin@…, jeremie.corbier@…

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

trac-0.9b1.tar.gz-anydiff.patch (83.6 kB) - added by cboos 3 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 cboos 3 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 cboos 3 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 cboos 3 years ago.
Patch adding the TracDiff features to trac-0.9b2
trac-diff-for-0.9.patch (91.6 kB) - added by cboos 3 years ago.
Adds theTracDiff features on top of trac-0.9
view_changes_button.png (16.0 kB) - added by cboos 3 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@… 3 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@… 3 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@… 3 years ago.
trac-diff-for-0.9.4.patch (85.4 kB) - added by trac-project@… 3 years ago.
Updated patch to apply against 0.9.4

Change History

Changed 3 years ago by cboos

  • status changed from new to assigned

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.

Changed 3 years ago by cboos

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

Changed 3 years ago by cboos

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

Changed 3 years ago by mgood

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.

Changed 3 years ago by cboos

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.

Changed 3 years ago by cboos

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

Changed 3 years ago by cboos

Patch adding the TracDiff features to trac-0.9b2

Changed 3 years ago by cboos

Adds theTracDiff features on top of trac-0.9

Changed 3 years ago by cboos

  • priority changed from normal to high

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

Please review.

Changed 3 years ago by cmlenz

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.

Changed 3 years ago by cboos

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

Changed 3 years ago by cboos

(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

Changed 3 years ago by cmlenz

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).

Changed 3 years ago by cboos

  • keywords tracdiff review added

Changed 3 years ago by vyt@…

  • cc vyt@… added

IMHO, it's very, very useful feature.

Changed 3 years ago by pixelpapst@…

  • cc pixelpapst@… added

Changed 3 years ago by jeremie.corbier@…

Adds the TracDiff features on top of the 0.9.3

Changed 3 years ago by jeremie.corbier@…

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

Changed 3 years ago by anonymous

  • cc jouvin@… added

Changed 3 years ago by jouvin@…

Changed 3 years ago by jouvin

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.

Changed 3 years ago by cmlenz

  • milestone changed from 1.0 to 0.10

Changed 3 years ago by anonymous

  • cc jeremie.corbier@… added

Thanks for cleaning up the patch.

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

Changed 3 years ago by cboos

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

Changed 3 years ago by cboos

  • status changed from assigned to closed
  • resolution set to fixed

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...)

Changed 3 years ago by trac-project@…

Updated patch to apply against 0.9.4

Add/Change #2028 (Integrate anydiff branch in the trunk)

Author



Change Properties
<Author field>
Action
as closed
Next status will be 'reopened'
 
Note: See TracTickets for help on using tickets.