Re: [PATCH 6/7] xdiff: simplify style assignments

Phillip Wood wrote:

On 09/06/2021 20:28, Felipe Contreras wrote:

I don't find the commit message explains this change very well

quoted

There is little value in checking that git_xmerge_style isn't 0 before
changing it's default value.

I think the check is actually that git_xmerge_style isn't -1.

Actually I meant "< 0", but yeah, that's mainly to check the -1 case.

Why is there little value in the check?

It's explained in the very next sentence.

quoted

Most of the time it isn't 0 anyway, so just assign the value directly.

Why to the times when it is zero (or -1) not matter?

When it's 0 it's a no-op, and now it can't be -1.

By default structures are zeroed in git, so the defaults of integers
are 0, and in the case of xmparam_t.style that is no exception.

These are all the same:

	static xmparam_t xmp;

	static xmparam_t xmp;
	xmp.style = 0;

	static xmparam_t xmp;
	if (1)
		xmp.style = 0;

But of course as it's explained most of the time that's not what
happens, what happens is:

  if (1)
    xml.style = 1;

Perhaps this is clearer:

  There is little value in checking that git_xmerge_style isn't < 0
  before changing its value to xmp.style. If it's 0 then assigning 0 to
  xmp.style is a no-op, and if it's 1 (as it usually is), we are going
  to assign the value anyway.

  The only exception is when git_xmerge_style is -1, but there is no
  value in having that as default, so we just don't, and set the default
  to 0.

Cheers.

-- 
Felipe Contreras