Re: Improve tab completion for ALTER FUNCTION/PROCEDURE/ROUTINE

On Thu, 5 Jan 2023 at 18:22, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> On Tue, 6 Dec 2022 at 19:12, vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Tue, 6 Dec 2022 at 20:42, Melih Mutlu <m.melihmutlu@gmail.com> wrote:
> > >
> > > Also one little suggestion:
> > >
> > >> + if (ends_with(prev_wd, ')'))
> > >> + COMPLETE_WITH(Alter_routine_options, "CALLED ON NULL INPUT",
> > >> +  "RETURNS NULL ON NULL INPUT", "STRICT", "SUPPORT");
> > >
> > > What do you think about gathering FUNCTION options as you did with ROUTINE options.
> > > Something like the following would seem nicer, I think.
> > >
> > >> #define Alter_function_options \
> > >> Alter_routine_options, "CALLED ON NULL INPUT", \
> > >> "RETURNS NULL ON NULL INPUT", "STRICT", "SUPPORT"
> >
> > I did not make it as a macro for alter function options as it is used
> > only in one place whereas the others were required in more than one
> > place.
>
> My feeling is that having this macro somewhat improves readability and
> consistency between the 3 cases, so I think it's worth it, even if
> it's only used once.
>
> I think it slightly improves readability to keep all the arguments to
> Matches() on one line, and that seems to be the style elsewhere, even
> if that makes the line longer than 80 characters.
>
> Also in the interests of readability, I think it's slightly easier to
> follow if the "ALTER PROCEDURE <name> (...)" and "ALTER ROUTINE <name>
> (...)" cases are made to immediately follow the "ALTER FUNCTION <name>
> (...)" case, with the longer/more complex cases following on from
> that.
>
> That leads to the attached, which barring objections, I'll push shortly.

The changes look good to me.

Regards,
Vignesh