meson icon indicating copy to clipboard operation
meson copied to clipboard

meson format: mangles with `if` statements with indented conditionals

Open dcbaker opened this issue 1 year ago • 7 comments

Describe the bug It's fairly common on projects using 2 space indent to use 4 space indent for multi line conditionals, so that the condition and the body of the block are visually distinct:

if (cc.links('''
    void do_func(int foo) { return INT_MAX - foo; }
    '''))
  deps += dependency('foo')
endif

meson format mangles this into something resembling the nonsense that black produces:

if (cc.links('''
    bool func(uint foo) { return foo < MAX_INT; }
    '''))
  cargs += ['-DHAS_MAX_INT']
endif

meson format turns this reasonable and readable code into:

if (cc.links(
    '''
    bool func(uint foo) { return foo < MAX_INT; }
    ''',
))
    cargs += ['-DHAS_MAX_INT']
endif

Expected behavior Does not mangle the if so that the closing parans are on the same indent level as the if/endif

system parameters Meson 1.5 and master

dcbaker avatar Aug 19 '24 20:08 dcbaker

@dcbaker So you would prefer this?

if (cc.links(
    '''
    bool func(uint foo) { return foo < MAX_INT; }
    ''',
    ))
    cargs += ['-DHAS_MAX_INT']
endif

What is the actual output you want? (asking for a friend)

annacrombie avatar Aug 20 '24 15:08 annacrombie

Well, since I use the canonical two space indent (:D), I'd expect this:

if (cc.links(
    '''
    bool func(uint foo) { return foo < MAX_INT; }
    ''',
    ))
  cargs += ['-DHAS_MAX_INT']
endif

But if you used four space indent, I'd expect what python that doesn't use black (like Meson itself) does:

if (cc.links(
        '''
        bool func(uint foo) { return foo < MAX_INT; }
        ''',
        ))
    cargs += ['-DHAS_MAX_INT']
endif

dcbaker avatar Aug 20 '24 16:08 dcbaker

I see, so you wouldn't like it to just be what it originally was (with the trailing parens stuck to the line)?

Also, I disagree with your second example because changing the indentation inside ''' is actually modifying the string itself which a formatter should never do imo.

annacrombie avatar Aug 20 '24 17:08 annacrombie

My principle problem is that whenever I see:

if (
  ...
)
  ...

This is super confusing, because ^\) It makes me think "This if block is done, we are now outside of the if", it's visually confusing. Indenting a second level (whether the )) stays attached or moves to its own line), makes it clear to me visual "I am still inside the if block". This is similar to the C mistake of:

if (condition)
  "something in the if block"
  "oops! looks like we're in the if block, but were not!"

And I get that the first ''' is changing, it just isn't relevant to my example so I was being lose with it.

dcbaker avatar Aug 20 '24 17:08 dcbaker

Do we want this as the only way to format, or do we want this as an option?

bruchar1 avatar Aug 20 '24 19:08 bruchar1

I suspect we'll want an option for this, because from my experience in python people either really love the:

if (
   condition
)
  body
endif

style, or they really hate it (and vice versa)

dcbaker avatar Aug 20 '24 21:08 dcbaker

I think the option could be called sticky_parens, and it controls wether parenthesis stick to the wrapping expression, e.g.

if (cc.links('''
    bool func(uint foo) { return foo < MAX_INT; }
    '''))
  cargs += ['-DHAS_MAX_INT']
endif

It also causes extra indentation in conditionals when enabled to make the conditional part clear, e.g:

if (not cc.compiles('...') 
    or cc.compiles('...') 
    or cc.compiles('...'))
  do
  stuff
endif

With sticky_parens off:

if (
  not cc.compiles('...') 
  or cc.compiles('...') 
  or cc.compiles('...')
)
  do
  stuff
endif

annacrombie avatar Aug 21 '24 10:08 annacrombie