Minor whitespace changes relating to lines of code that are being changed

[Michael Weghorn]

On 2025-02-13 14:18, Stephan Bergmann wrote:
> On 13.02.25 11:36, Chris Sherlock wrote:
>> I see commits all the time where someone changes a line of code and 
>> changes the whitespacing in that same line of code.
> 
> I occasionally see such too, and always wonder why people do that.  My 
> objections are twofold:
> 
> For one, unnecessary and unrelated changes are distractions when looking 
> at a commit.  When a commit wants to do X, then it should do that, but 
> not also randomly add or remove whitespace here and there, even in lines 
> it touches for other reasons.  There are cases where every little iota 
> counts when you look at a commit, so do others a favor and keep your 
> commits simple.
> 
> And for another, what is the benefit of such random whitespace changes? 
> The code is readable, before and after.  Everybody has their preferred 
> style rules, and we have to be able to cope with a mix of styles anyway. 
>   The goal should not be that the code is formatted according to the 
> preferred style of programmer A (and then programmer B comes along and 
> reformats lines according to their preferences)

While I'm aware the level to which people (dis)like clang-format (and 
the config we use for it) varies greatly, I personally usually (roughly) 
stick to its "taste" also in excluded files when introducing new code 
and to some extent when changing existing code (like whitespace in a 
line changed anyway, but not reformatting whole blocks of code in a way 
that would in my perspective more significantly clutter the commit diff 
and make it harder to see the "actual change").

Thinking about why I do that, it's probably because I saw others doing 
it earlier and it seems like a way to slowly move towards a code base 
that more consistently uses a "common" coding style (that is mandated by 
clang-format in other places), without changing unrelated lines (and 
thus having those show up in a `git blame` etc. while they wouldn't 
otherwise).

To me, that currently feels like a reasonable balance between

1) minimizing the commit diff
2) moving towards a more consistent coding style across the codebase

I see how this is motivated by myself considering 2) a desirable goal, 
while others may disagree.

Now I don't know whether it would make sense to try to get a consensus 
on that across contributors and then either consider that kind of change 
acceptable or discourage it "officially" (or whether it's better to 
leave it for everyone to decide what they do in their own commits and 
reviews)?

(Using a strategy that tries to stick to the coding style used elsewhere 
in the file for consistency when doing changes turned out not to be a 
workable approach in general as files often don't use a consistent 
coding style anyway. Using (git-)clang-format also seemed to be the 
"easy way" here without having to think about where exactly to break 
long lines in newly introduced code,...)


> ---the goal should rather 
> be that the code is roughly readable and understandable.  (To paraphrase 
> an example from the original PragProg book:  Mechanical workers also 
> don't spend time making their power drills look beautiful.  It's tools 
> for them that need to get their job done in a professional way, but it's 
> irrelevant that they may look patched and ugly.)

For me personally, "ugly code" generally tends to be less readable, and 
given the overall complexity of LO code sometimes adds to my confusion 
while debugging/reading code.
For me personally, consistency ("principle of least surprise") helps 
reduce distraction and focus on the actual issue I'm looking into.
(Now this arguably applies less for the exact placement of the space 
character in an if statement and more for things like extra/missing 
indentation of a block, misleading variable names (local variable 
`double mrMyInt = 42.0;`), and other creative things that can be done in 
LO source code.)

In the illustration of the power drill: As long as the power drill (the 
code) does what it should, I usually don't care what it looks like. Only 
when I have to look at it more closely to figure out why it doesn't 
work, the ugliness starts to bother/distract me.

> That's my take on it.  Of course, YMMV

Same here: The above is just my personal take, and if the majority 
considers "minimizing the commit diff" the guideline that should be 
followed, I'll adhere to that in the future.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/libreoffice/attachments/20250214/9882077c/attachment.sig>