Re: Added missing tab completion for alter subscription set option

On Fri, May 14, 2021 at 7:10 PM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Fri, May 14, 2021 at 6:51 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Fri, May 14, 2021 at 12:25 PM Bharath Rupireddy
> > <bharath.rupireddyforpostgres@gmail.com> wrote:
> > >
> > > On Fri, May 14, 2021 at 12:00 PM vignesh C <vignesh21@gmail.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > While I was reviewing one of the logical decoding features, I found
> > > > Streaming and binary options were missing in tab completion for the
> > > > alter subscription set option, the attached patch has the changes for
> > > > the same.
> > > > Thoughts?
> > >
> > > +1.
> > >
> > > Without patch:
> > > postgres=# alter subscription testsub set (S
> > > SLOT_NAME           SYNCHRONOUS_COMMIT
> > >
> > > With patch:
> > > postgres=# alter subscription testsub set (
> > > BINARY              SLOT_NAME           STREAMING           SYNCHRONOUS_COMMIT
> > >
> > > How about ordering the options alphabetically as the tab complete
> > > output anyways shows that way? I'm not sure if that's the practice,
> > > but just a thought.
> >
> > I did not see any rule for this, but also did not see any harm in
> > keeping it in alphabetical order, so changed it in the attached patch.
>
> Thanks. Just a few nitpicks:
> 1) How about patch name: "Add tab completion for ALTER SUBSCRIPTION
> SET options streaming and binary"?

Modified.

> 2) How about a detailed message: "Tab completion for the options
> streaming and binary were missing in case of ALTER SUBSCRIPTION SET
> command. This patch adds them."?
>

Modified.

> You may want to add this in commitfest so that we don't lose track of it.

I have added a commitfest entry at [1].