Jakub Narebski <jnareb@xxxxxxxxx> wrote:
> Michał Kiedrowicz <michal.kiedrowicz@xxxxxxxxx> writes:
>
> > Currently, print_sidebyside_diff_chunk() does two things: it
> > accumulates diff lines and prints them. Accumulation may be used to
> > perform additional operations on diff lines, so it makes sense to split
> > these two things. Thus, the code that prints diff lines in a side-by-side
> > manner is moved out of print_sidebyside_diff_chunk() to a separate
> > subroutine.
> >
> > The outcome of this patch is that print_sidebyside_diff_chunk() is now
> > much shorter and easier to read.
> >
> > This change is meant to be a simple code movement. No behavior change is
> > intended.
> >
> > Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@xxxxxxxxx>
>
> As it is pure refactoring, and improves readibility of code quite a
> bit, I am all for it, but...
>
> You replace the following set of conditions
>
> > - ## print from accumulator when type of class of lines change
> > - # empty contents block on start rem/add block, or end of chunk
> > - if (@ctx && (!$class || $class eq 'rem' || $class eq 'add')) {
>
> and
>
> > - # empty add/rem block on start context block, or end of chunk
> > - if ((@rem || @add) && (!$class || $class eq 'ctx')) {
> > - if (!@add) {
> [...]
> > - } elsif (!@rem) {
> [...]
> > - } else {
>
>
> with these (I have reindented the code to match)
>
> > + ## print from accumulator when have some add/rem lines or end
> > + # of chunk (flush context lines)
> > + if (((@rem || @add) && $class eq 'ctx') || !$class) {
>
> > + if (@$ctx) {
> [...]
> > + }
> > + if (!@$add) {
> [...]
> > + } elsif (!@$rem) {
> [...]
> > + } else {
>
> It is not obvious that the final result is the same.
You are right. I should have described it in the commit message.
>
> BTW doesn't new code print context when printing added and removed
> lines, and not as soon as class changes? This doesn't change the
> output, but it slightly changes behavior.
>
This is also true. I'll describe it in the commit message. I could also
create two functions: one that prints @ctx and second one that prints
@rem and @add but then I would have to create two other functions for
inline diff.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html