[PATCH 1 of 1 default] hgweb: column / side-by-side diff functionality

[Angel Ezquerra]

On Sun, Jun 10, 2012 at 9:20 PM, Patrick Mézard <patrick at mezard.eu> wrote:
> Le 10/06/12 20:52, Wujek Srujek a écrit :
>> Hi Patrick.
>>
>> Thanks for your tips. How do I proceed now? I assume I fix the patch
>> according to your input and send it once more, right?
>> I am actually working on the other templates now, I also divided the patch
>> locally into more:
>> 1. the core coldiff functionality, new hgrc text, but not called by anybody
>> yet (the templates don't know it)
>> 2. the templates for each style, one style - one patch (the hgweb tests
>> will fail when this patch is applied, until you apply:
>> 3. the 2 new tests and old tests fixed
>>
>> Do you think the new queue is better, or should it all really by one patch?
>> (I did it because I thought the patch was too big and got lost / ignored.)
>
> Individual patches must pass the test suite, so moving the tests after the changes not the way to go.
>
> I think your initial patch was very nice to review: it provided a new, self-contained, testable feature. What I would do is keep sending an updated version of it until we agree on the code and features, then merge the other template fixes for the final submission. Matt may have another opinion.
>
>> I am not really convinced by the partial mode. Does it bring any
>>> information compared to a regular diff, besides a different formatting? For
>>> me, this file comparison view is really useful when diff does not give
>>> enough context to understand the change, in which case I prefer to have the
>>> full file.
>>>
>>> Compare:
>>>
>>> http://mezard.eu/hg/hg-does-it-look-good-for-you/diff/e1aa1ed30030/mercurial/revset.py
>>>
>>> http://mezard.eu/hg/hg-does-it-look-good-for-you/coldiff/e1aa1ed30030/mercurial/revset.py
>>>
>>> http://mezard.eu/hg/hg-does-it-look-good-for-you/coldiff/e1aa1ed30030/mercurial/revset.py?fullcoldiff=1
>>>
>>> I agree the last one would be better with a mean to navigate between
>>> groups.
>>
>> It brings no additional information, I think, but it is just much easier to
>> read than regular diff, and much more concise than full side-by-side diff,
>> when the changes are scarce and at the end of a long file, for instance. In
>> your example link, one needs to scroll a long way to find the changes.
>> That's also because there is no UI to click to go to the next change, like
>> IDEs do, but I am not sure if I can do it without processing the whole diff
>> first, so that I can put such link at the top of a page (it is currently
>> streamed with generators). I think that's what you mean 'navigate between
>> groups'? Unless someone has another idea.
>> In my company, when I introduced mercurial, one of the pain-points was that
>> people said that they don't understand the regular diff (we use Jenkins CI
>> and we can see what scm changes triggered the build, and people often check
>> what others checked in), and showed me their old viewvc side-by-side diff.
>> (That's actually the reason I decided to try and implement it.) That tool
>> allows one to pick the full or partial mode, and since it isn't really a
>> big deal to have it as well in hgweb, here it is. But it can be dropped
>> easily ;d
>
> Fine, I am not really surprised by the reasons behind it. I am not suggesting to drop it, but if you can read diffs, it is useless, so I question making it the default. Maybe the full version without navigation help is useless though. I would like to hear what people think, you can leave it as it is for now.

I've also heard people say that they find the regular diff view
confusing. I don't know, however, if many of those people really use
the hgweb interface to look at the changes introduced by revisions.

I think the lack of navigation on big file when using a side-by-side
view would be a problem. So I think that having a partial mode would
be nice. I don't know if it should be the default though.

Angel