Re: [viewvc-dev] [PATCH] Show property diffs in ViewVC

On 09/30/2011 03:08 PM, Alexey Neyman wrote:
>>> In a hindsight, I think the pdiff view may have been made a part of the
>>> regular diff view (with "props changed" link pointing to an anchor in the
>>> diff view). Please advise if you think it would be a better way.
>>
>> Given that we're probably not talking about tons of property changes
>> needing to be displayed on the page, I do think adding these to the bottom
>> of the regular diff view -- recycling the machinery used to choose the
>> diff format, avoiding another view and the configuration mechanics around
>> it, etc. -- would be useful.
> 
> One argument against it that I thought about is that such aggregation would 
> have to share the same format. E.g., if one wanted to see 'full context diff' 
> for props, he would be stuck with a 10000 lines of full context diff of the 
> file contents as well. Is it okay?

Hrm, you make a good point.  I suppose we could start with a shared diff
format selection, and add a distinct pdiff_format CGI variable in the future
if we don't like how the original plan plays out.

I'd eventually like to see the diff view be less "Pick from the pre-chosen
display options" and more "Here's some knobs for you to play with: number of
context lines, whitespace consideration, etc.".  (See
http://viewvc.tigris.org/issues/show_bug.cgi?id=301)  Maybe now is the time
to start considering what that might all look like.  BUT!  Don't let any of
this future-thinkin' get in the way of integrating your work into ViewVC.
No sense in letting the perfect be the enemy of the good.

>> I'm loving some of the abstractions you've mentioned though, and am
>> wondering how many of them could be further used to provide the
>> oft-requested "revision diff" feature for Subversion (see
>> http://viewvc.tigris.org/issues/show_bug.cgi?id=217).
> 
> Actually, that should be pretty straightforward now that the diff_display.ezt 
> template can be iterated over. The only thing I think is needed for that is to 
> make the left/right items a part of the 'diff' hash as well, so that href's 
> can be set (or not set) for each "diffable" object separately.

Cool.

>> EZT is at this point a somewhat divergent reflection of an upstream module.
>>  I'd really like to someday re-sync with the upstream module, but I also
>> know that the primary author and maintainer of that module strongly desires
>> to avoid feature creep in EZT.  (He wasn't pleased with the addition of the
>> immensely useful [define] directive, even.)  So, I'd like to see how far we
>> can go without modifying EZT itself.
> 
> With EZT modification off the table, the template arguments to diff.ezt would 
> have to be changed. As far as I understand, that's allowed for major releases, 
> right?

Yeah, that's no sweat.  I'm thinking that its about time to start looking at
some of these bigger items for a 2.0 release anyway.

>> Thanks so much for this contribution.  I have not reviewed the code
>> changes, but given the size of the patch, I suspect it would be mutually
>> beneficial for you to simply do this work in a branch of the project's
>> repository. What do you think?
> 
> I am okay with that. I think it would be easier to review the changes once 
> they are split in smaller, "one change at a time" blocks.

Cool.  You'll first need to create a Tigris.org account.  After that,
request the Developer role at:

http://viewvc.tigris.org/servlets/ProjectMembershipRequest

Once the role is approved, one of us can create a branch for you to play in.

-- 
C. Michael Pilato <[email protected]>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

------------------------------------------------------
http://viewvc.tigris.org/ds/viewMessage.do?dsForumId=4251&dsMessageId=2848343

To unsubscribe from this discussion, e-mail: [[email protected]].

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAk6GGIEACgkQokEGqRcG/W68TQCeOXqo5VQ7ebdbjgXRFf+cNaf9
nAYAoMxH4vBMIAgTs5Wwep1V7/dKGwTd
=QK+A
-----END PGP SIGNATURE-----