Re: [PATCH] block: replace TABs with space

[Eric Blake]

On Fri, Mar 10, 2023 at 12:10:08AM +0800, Yeqi Fu wrote:
> Bring the block files in line with the QEMU coding style, with spaces
> for indentation.
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/371
> 
> Signed-off-by: Yeqi Fu <fufuyqqqqqq@gmail.com>
> ---
>  block/bochs.c       | 12 +++++------
>  block/file-posix.c  | 52 ++++++++++++++++++++++-----------------------
>  block/file-win32.c  | 38 ++++++++++++++++-----------------
>  block/parallels.c   | 10 ++++-----
>  block/qcow.c        | 10 ++++-----
>  include/block/nbd.h |  2 +-
>  6 files changed, 62 insertions(+), 62 deletions(-)
> 
> diff --git a/block/bochs.c b/block/bochs.c
> index 2f5ae52c90..8ff38ac0d9 100644
> --- a/block/bochs.c
> +++ b/block/bochs.c
> @@ -293,15 +293,15 @@ static void bochs_close(BlockDriverState *bs)
>  }
>  
>  static BlockDriver bdrv_bochs = {
> -    .format_name     = "bochs",
> -    .instance_size   = sizeof(BDRVBochsState),
> -    .bdrv_probe              = bochs_probe,
> -    .bdrv_open               = bochs_open,
> +    .format_name    = "bochs",
> +    .instance_size  = sizeof(BDRVBochsState),
> +    .bdrv_probe     = bochs_probe,
> +    .bdrv_open      = bochs_open,
>      .bdrv_child_perm     = bdrv_default_perms,
>      .bdrv_refresh_limits = bochs_refresh_limits,

Hmm. When shown in the diff, it looks like you are changing the
alignment of the .bdrv_probe line; but in reality, the extra prefix
from diff changes which tabstop the '=' goes out to, so you are
preserving the original column.  Still, it looks really awkward that
we have one alignment for .format_name through .bdrv_open, and another
alignment for .bdrv_child_perm and .bdrv_refresh_limits.

My personal preference: '=' alignment is a lost cause.  It causes
pointless churn if we later add a field with a longer name (all other
lines have to reindent to match the new length).  I'd prefer exactly
one space before '=' on all lines that you touch (and maybe touch a
few more lines in the process).  But if we DO stick with alignment,
I'd much rather that ALL '=' be in the same column, rather than the
pre-existing half-and-half mix, even though your patch kept that
visually undisturbed (other than when diff injects a prefix that
rejiggers the tab stops).

I wouldn't mind a v2 if we can reach consensus on whether to
consistently align '=' or always use just one space; but I don't know
if that consensus will be easy, so I can also live with:

Reviewed-by: Eric Blake <eblake@redhat.com>

even if we end up using v1 as-is.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org