Re: Ticket 298: bug on pg_hba.conf editor - Mailing list pgadmin-hackers

On Sun, 2011-07-17 at 10:12 +0100, Dave Page wrote:
> On Saturday, July 16, 2011, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> > On Sat, 2011-07-16 at 21:11 +0100, Dave Page wrote:
> >> On Sat, Jul 16, 2011 at 1:12 PM, Guillaume Lelarge
> >> <guillaume@lelarge.info> wrote:
> >> > Hi,
> >> >
> >> > I worked a bit this morning on this bug. The editor was made in a way
> >> > that invalid configuration lines are not displayed which is wrong
> >> > because you can't fix a line if you stored it wrong once.
> >> >
> >> > So I did the change to allow the change of an invalid configuration
> >> > line, and that works well.
> >> >
> >> > But I now have many other lines that aren't supposed to appear:
> >> >
> >> > # local      DATABASE  USER  METHOD  [OPTIONS]
> >> > # host       DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
> >> > # hostssl    DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
> >> > # hostnossl  DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
> >> > # host name, or it is
> >> >
> >> > All are considered comments, and all have a valid first column, so all
> >> > are displayed. Which is a bit disturbing because they are part of the
> >> > comments in pg_hba.conf, they are not supposed to be "actual" lines.
> >> >
> >> > So, they match our process of identifiying lines, and so they are
> >> > displayed. Do you have any idea how we could not display these? I mean,
> >> > I can simply add a check on the line string to see if they are equal to
> >> > the one of the five strings above, but it seems quite a ugly hack.
> >>
> >> Why don't we just ignore anything that starts with a # ?
> >>
> >
> > Because we need to guess which comment is an actual comment and which
> > comment is a disabled configuration. That allows us to hide actual
> > comments, and show disabled configuration. Problem is that our guess is
> > wrong sometimes.
>
> Sounds like you're trying to be too clever.

I'm not. That's what the code already does since quite a long time.
Problem is it doesn't show lines that are enabled and invalid. And if I
try to show invalid lines, it shows even ones I don't want.

Hmmm, thinking now that I could show invalid but enabled lines, and not
invalid and disabled lines.

>  We don't normally care
> about commented lines in configuration files. If you really want to do
> so tough, check if a token is wrapped in [ ] - that never happen in a
> valid configuration I don't believe.
>

Actually, I'm fine with simply ignoring comments.

> >> > Or do we simply choose to not care? we prefer to have the bugfix even if
> >> > it means to show some not "actual" config lines?
> >>
> >> Not those.
> >>
> >
> > I don't get it, sorry :)
> >
> > What do you mean by "not those"?
>
> We don't want to show those lines.
>

OK.

> >> > Another related question: peer, radius are not available in the method.
> >> > As we are in beta, I won't add them to 1.14 branch, will I?
> >>
> >> I would consider their omission to be a bug.
> >>
> >
> > Hmmm, OK. Will fix then.
>
> Thanks.
>

Done.


--
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com