Hi
2014-08-08 13:58 GMT+02:00 Fujii Masao <masao.fu...@gmail.com>:
> On Fri, Aug 8, 2014 at 4:31 AM, Pavel Stehule <pavel.steh...@gmail.com>
> wrote:
> >
> >
> >
> > 2014-08-07 7:10 GMT+02:00 Fujii Masao <masao.fu...@gmail.com>:
> >
> >> On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <pavel.steh...@gmail.com>
> >> wrote:
> >> > Hello
> >> >
> >> > updated version patch in attachment
> >>
> >> Thanks! But ISTM you forgot to attached the patch.
> >
> >
> > grr .. I am sorry
>
> No problem. Thanks for the patch! Attached is the revised version of the
> patch.
>
> >> >> + /* all psql known variables are included in list by default */
> >> >> + for (known_varname = known_varnames; *known_varname;
> >> >> known_varname++)
> >> >> + varnames[nvars++] = pg_strdup(*known_varname);
> >> >>
> >> >> Don't we need to append both prefix and suffix to even known
> variables?
> >> >
> >> >
> >> > ??? I am not sure if I understand well - probably system "read only"
> >> > variables as DBNAME, USER, VERSION should not be there
> >>
> >> I had that question because complete_from_variables() is also called by
> >> the
> >> tab-completion of "\echo :" and it shows half-baked variables list. So
> >> I thought probably we need to change complete_from_variables() more to
> >> fix the problem.
> >
> >
> > I understand now.
> >
> > I fixed it.
> >
> > I have a question.\echo probably should not to show empty known variable.
> >
> > data for autocomplete for \echo should be same as result of "\set"
>
> Agreed. I think that only the variables having the set values should be
> displayed in "\echo :" case. So I modified complete_from_variables()
> so that the unset variables are not shown in that case but all the
> variables
> are shown in the tab-completion of "\set".
>
> >> >> + else if (strcmp(prev2_wd, "\\set") == 0)
> >> >> + {
> >> >> + if (strcmp(prev_wd, "AUTOCOMMIT") == 0)
> >> >>
> >> >> ISTM that some psql variables like IGNOREEOF are not there. Why not?
> >> >
> >> >
> >> > yes, there are not complete for DBNAME, ENCODING, FETCH_COUNT,
> >> > HISTCONTROL,
> >> > HISTFILE, HISTSIZE, HOST, IGNOREEOFF, PROMPT*,USER, VERSION
> >> >
> >> > There are more reasons:
> >> >
> >> > * paremeter is not a enum (string, number or both): FETCH_COUNT,
> PROMPT,
> >> > HISTSIZE, ..
> >> >
> >> > * variable is pseudo read only - change has not any effect: DBNAME,
> >> > ENCODING, VERSION
> >>
> >> So HISTCONTROL should be there because it doesn't have such reasons at
> >> all?
> >>
> >
> > yes
>
> ISTM that you forgot to add HISTCONTROL to your patch. So I just added
> that.
>
> I added the tab-completion for "\unset" command. Like "\echo :", only
> the variables having the set values should be displayed in "\unset" case.
>
perfect
>
> I changed complete_from_variables() so that it checks the memory size of
> the variable array even when appending the known variables. If the memory
> size is not enough, it's dynamically increased. Practically this check
> would
> not be required because the initial size of the array is enough larger than
> the number of the known variables. I added this as the safe-guard.
>
> Typo: IGNOREEOFF -> IGNOREEOF
>
> I removed the value "none" from the value list of "ECHO" because it's not
> documented and a user might get confused when he or she sees the
> undocumented
> value "none". Thought?
>
isn't better to fix doc? I don't know any reason why we should not to
support "none"
I looked to code, you removed a check against duplicate varname in list. Is
it ok?
Regards
Pavel
>
> Regards,
>
> --
> Fujii Masao
>
