Thread: Re: psql: Add tab completion for ALTER USER RESET

Re: psql: Add tab completion for ALTER USER RESET

From
Tomas Vondra
Date:
Hi,

On 11/29/24 09:42, Robins Tharakan wrote:
> Hi,
> 
> Please find attached a patch to help tab completion show only currently
> set vars
> during ALTER USER RESET.
> 
> Currently tab completion provides a list of all vars which is only
> partially helpful.
> This patch allows tab completion to see which vars are currently set for
> the role
> and only show those options during tab completion.
> 

Yeah, this looks convenient. Without the patch we'd get

...
Display all 200 possibilities? (y or n)

and then a wall of text with all the options. With the patch we get only
options that are actually set for the user, which is much better.

Two comments:

1) Does it make sense to still show "ALL" when the query returns
nothing? Not sure if we already have a way to handle this.

2) Should we do the same thing for ALTER DATABASE? That also allows
setting options.


thanks

-- 
Tomas Vondra




Re: psql: Add tab completion for ALTER USER RESET

From
Tomas Vondra
Date:

On 2/15/25 12:14, Robins Tharakan wrote:
> Hi Tomas,
> 
> Thanks for taking a look - apologies for the delay here.
> 
> On Tue, 10 Dec 2024 at 09:09, Tomas Vondra <tomas@vondra.me
> <mailto:tomas@vondra.me>> wrote:
>>
>> 1) Does it make sense to still show "ALL" when the query returns
>> nothing? Not sure if we already have a way to handle this.
>>
> 
> +1 - "ALL" is pointless when there is nothing to RESET, but I didn't
> find an easy way to make it conditional (which was partly the
> cause of delay here). For this, I tried (and not yet successful) to
> create a new set of macros - something along the lines of
> COMPLETE_WITH_QUERY_IFNOTEMPTY_PLUS() - where
> the second part is conditional on whether the query list returns
> a non-empty set.
> 
> If this is a blocker, I'll continue working on a macro that allows that
> more easily (although it'd be great if you could point me to a better
> way to implement that). For now, reattached v1 to this email for
> convenience.
> 

Thanks. I don't think it's a blocker - in fact, after thinking about it
a bit more, I believe showing the "ALL" is consistent with what we do in
other places when the query returns nothing. Consider for example CLOSE:

    /* CLOSE */
    else if (Matches("CLOSE"))
        COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_cursors,
                                 "ALL");

Other places do similar stuff. It's just a noop. So I don't think you
need to waste time inventing something for this one place.

> 
>> 2) Should we do the same thing for ALTER DATABASE? That also allows
>> setting options.
>>
> +1. Attached a separate patch for ALTER DATABASE RESET in case
> you think it is good to go alongside (albeit again, this still does show
> ALL even if there's no vars to RESET).
> 

Thanks. These patches look fine to me. I'll get them committed.


regards

-- 
Tomas Vondra




Re: psql: Add tab completion for ALTER USER RESET

From
Tomas Vondra
Date:
On 2/16/25 17:56, Tomas Vondra wrote:
>...
> 
> Thanks. These patches look fine to me. I'll get them committed.
> 

I've pushed both patches. Thanks!

-- 
Tomas Vondra