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

Alexey Neyman <[email protected]> Fri, 30 Sep 2011 12:08:33 -0700
Newsgroups gmane.comp.version-control.cvs.viewcvs.devel
Message-ID <201109301208.33963.stilor__9158.10555820844$1317409734$gmane$org@att.net>
Hi,

On Friday, September 30, 2011 11:34:21 am C. Michael Pilato wrote:
> > - In viewvc-install, a minor bug is fixed which resulted in traceback if
> > [Enter] was pressed in response to "overwrite/do not overwrite" question.
> 
> This is logically unrelated to the other work, thus should be submitted as
> a separate patch.
> }}}

Done.

> > 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?

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

> 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?

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

Regards,
Alexey.

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

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