Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless) - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless)
Date
Msg-id alpine.DEB.2.20.1702011631060.31128@lancre
Whole thread Raw
In response to Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)  ("Daniel Verite" <daniel@manitou-mail.org>)
List pgsql-hackers
<sorry, resent, wrong from again *$%@#&!?>

Hello Corey,

>> There is a spurious empty line added at the very end of "mainloop.h":
>>
>>   +
>>    #endif   /* MAINLOOP_H */
> 
> Not in my diff, but that's been coming and going in your diff reviews.

Strange. Maybe this is linked to the warning displayed with "git apply" when 
I apply the diff.

>> I would suggest to add a short one line comment before each test to
>> explain what is being tested, like "-- test \elif execution", "-- test
>> \else execution"...
> 
> Where are you suggesting this?

In "regress/sql/psql.sql", in front of each group which starts a test.

>> Debatable suggestion about "psql_branch_empty":
> 
> The name isn't great. Maybe psql_branch_stack_empty()?
 Yep, maybe, or "empty_stack" or "stack_is_empty" or IDK...

> "psql_branch_end_state": it is a pop, it could be named "psql_branch_pop"
> 
> Yeah, we either need to go fully with telling the programmer that it's a
> stack (push/pop/empty) or (begin_branch/end_branch/not_branching). I'm
> inclined to go full-stack, as it were.

Anything consistent is ok. I'm fine with calling a stack a stack:-)

-- 
Fabien.



pgsql-hackers by date:

Previous
From: Vik Fearing
Date:
Subject: Re: [HACKERS] new autovacuum criterion for visible pages
Next
From: Konstantin Knizhnik
Date:
Subject: Re: [HACKERS] Deadlock in XLogInsert at AIX