Re: [PATCH] capget.2, execve.2, readv.2, socketpair.2, utime.2, utimensat.2, getloadavg.3, proc.5, mount_namespaces.7, unix.7: ffix

["Alejandro Colomar (man-pages)" <alx.manpages@gmail.com>]

Hi Michael,

As you said in other patches about global changes,
I completely agree in that such changes,
if automated in scripts,
are very dangerous.

That said, yes, internally there's something in my head
telling me to do such changes when I see them.

And yes, one good reason to fix them is that consistency
simplifies scripting a lot.

So I tend to slowly fix some of them
as I see them while fixing similar things.
But I try not to add so many of those fixes that
I would distract from the main fix.

The rationale for why some an not other fixes in this specific case:
I first grepped to find the files the files that contained
{.IR var [x]}:
$ grep -rn "^\.I[ |R].* \\[.*\\]" |sort
(BTW, I forgot to add that script to the commit msg,
I'll add it in the next version).

And then inside the file I ctrl+F'd '[' to find them.
That showed me a few more lines than I searched for,
and found a few more fixes to do.
They weren't completely unrelated,
so I added them to the same patch.
That's why I only changed some of:
>> -(26) \fIstartcode\fP \ %lu \ [PT]
>> +.RI "(26) " startcode "  %lu  [PT]"
They showed up while finding branckets.

However... if you feel that's still too much for a patch,
I completely understand it, so I can separate the changes.

Please, tell me your thoughts.

Cheers,

Alex


On 11/20/20 10:27 PM, Michael Kerrisk (man-pages) wrote:
> Hi ALex,
> 
> On 11/19/20 6:46 PM, Alejandro Colomar wrote:
>> The main fix is {.IR var [x]} -> {.I var[x]}
>>
>> There were around 20 entries of the former,
>> and around 360 of the latter.
> 
> Yes, that's a worthwhile consistency fix. Thanks!
> 
>> While fixing that, I came across some obvious srcfixes,
>> which I also added to this patch.
> 
> But when you do this:
> 1) It make it harder to review your patch in order to spot
> any mistakes.
> 2) You mix multiple types of change into one patch, and I 
> want to take one type of change, but not the other. In particular,
> changes of the form:
> 
> [[
> -.IR wword
> +.I word
> ]]
> 
> create what I consider to be needless churn. Yes, the .I form 
> is sufficient, but the .IR form is not harmful, and I really
> don't went global edits that make these sorts of changes,
> because of the churn.
> 
> Notwithstanding the above, I would have applied this patch,
> except for a problem noted below. In the next version of the patch,
> I'd be much happier if you made just the {.IR var [x]} -> {.I var[x]}
> change.
[...]
>> -(26) \fIstartcode\fP \ %lu \ [PT]
>> +.RI "(26) " startcode "  %lu  [PT]"
> 
> With this change, now the long list is using two different forms,
> since you change just some of them. I'm not sure why you did that,
> but again, if you wanted to make a change like this (to the whole
> list), then it should be split out into another patch. But, again,
> it feels a bit like unnecessary churn, since the resulting output
> is unchanged. (By the way, I'm not 100% against global edits that
> don't change the rendered output. For example, and argument for
> such a change might be that by improving consistency in the
> page sources, it makes future scripted edits easier.)
> 
> [...]
> 
> Thanks,
> 
> Michael
> 
> 
>