Обсуждение: Patch to fix Debian Bug #451038

Поиск
Список
Период
Сортировка

Patch to fix Debian Bug #451038

От
Guillaume Lelarge
Дата:
Hi,

Contents of the bug ticket:
  When you try to restore some database from backup file pgadmin3 pops up
  FileSelection dialog from gtk. I can choose here between showing "*.backup"
  or "*.*" files.

  The second filter howerver doesn't show all files. To show all files in this
  dialog there muste be "*" filter.

  Right now you cannot open files named: "my-cool-backup", "something"
  "coolDatabase"... etc..

You can have it all here :
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=451038

So here is a patch to fix this.

PS : btw, I'm working with pgAdmin's Debian package maintainer, Gerfried
Fuchs, so that we can get a package for our last release. I hope to have good
news soon.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Вложения

Re: Patch to fix Debian Bug #451038

От
Dave Page
Дата:
On Thu, Jul 23, 2009 at 6:48 PM, Guillaume
Lelarge<guillaume@lelarge.info> wrote:

> So here is a patch to fix this.

Should we filter on *.* on Windows, and just * everywhere else? It'll
make the code a little messier, but would make more sense to the user.

> PS : btw, I'm working with pgAdmin's Debian package maintainer, Gerfried
> Fuchs, so that we can get a package for our last release. I hope to have good
> news soon.

Nice :-)



--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

Re: Patch to fix Debian Bug #451038

От
Magnus Hagander
Дата:
On Thu, Jul 23, 2009 at 20:56, Dave Page<dpage@pgadmin.org> wrote:
> On Thu, Jul 23, 2009 at 6:48 PM, Guillaume
> Lelarge<guillaume@lelarge.info> wrote:
>
>> So here is a patch to fix this.
>
> Should we filter on *.* on Windows, and just * everywhere else? It'll
> make the code a little messier, but would make more sense to the user.

+1 - it'll be much nicer for the user I think.



--
 Magnus Hagander
 Self: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: Patch to fix Debian Bug #451038

От
Guillaume Lelarge
Дата:
Le jeudi 23 juillet 2009 à 20:56:58, Dave Page a écrit :
> On Thu, Jul 23, 2009 at 6:48 PM, Guillaume
>
> Lelarge<guillaume@lelarge.info> wrote:
> > So here is a patch to fix this.
>
> Should we filter on *.* on Windows, and just * everywhere else? It'll
> make the code a little messier, but would make more sense to the user.
>

Is it really needed? it'll add much more code, and I don't think it will be
easier to use.

> > PS : btw, I'm working with pgAdmin's Debian package maintainer, Gerfried
> > Fuchs, so that we can get a package for our last release. I hope to have
> > good news soon.
>
> Nice :-)

BTW, did you have a look at my previous patches (the bug ones, not the new
functionality)? can I commit them?


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Re: Patch to fix Debian Bug #451038

От
Guillaume Lelarge
Дата:
Le jeudi 23 juillet 2009 à 21:09:26, Magnus Hagander a écrit :
> On Thu, Jul 23, 2009 at 20:56, Dave Page<dpage@pgadmin.org> wrote:
> > On Thu, Jul 23, 2009 at 6:48 PM, Guillaume
> >
> > Lelarge<guillaume@lelarge.info> wrote:
> >> So here is a patch to fix this.
> >
> > Should we filter on *.* on Windows, and just * everywhere else? It'll
> > make the code a little messier, but would make more sense to the user.
>
> +1 - it'll be much nicer for the user I think.

OK, if you're two to think the same, I'll change my patch.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Re: Patch to fix Debian Bug #451038

От
Dave Page
Дата:
On Thu, Jul 23, 2009 at 8:17 PM, Guillaume
Lelarge<guillaume@lelarge.info> wrote:
> BTW, did you have a look at my previous patches (the bug ones, not the new
> functionality)? can I commit them?

Can you point me to them in the archives please? I probably missed
them while I was out having fun and herring...

--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

Re: Patch to fix Debian Bug #451038

От
Guillaume Lelarge
Дата:
Le jeudi 23 juillet 2009 à 21:24:51, Dave Page a écrit :
> On Thu, Jul 23, 2009 at 8:17 PM, Guillaume
>
> Lelarge<guillaume@lelarge.info> wrote:
> > BTW, did you have a look at my previous patches (the bug ones, not the
> > new functionality)? can I commit them?
>
> Can you point me to them in the archives please? I probably missed
> them while I was out having fun and herring...

Lucky you :)

Bug fixes:
  http://archives.postgresql.org/pgadmin-support/2009-07/msg00093.php
  http://archives.postgresql.org/pgadmin-support/2009-07/msg00090.php

New functionality:
  http://archives.postgresql.org/pgadmin-hackers/2009-07/msg00036.php

Nothing really big, but still a bit of work :)


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Re: Patch to fix Debian Bug #451038

От
Guillaume Lelarge
Дата:
Le jeudi 23 juillet 2009 à 21:18:22, Guillaume Lelarge a écrit :
> Le jeudi 23 juillet 2009 à 21:09:26, Magnus Hagander a écrit :
> > On Thu, Jul 23, 2009 at 20:56, Dave Page<dpage@pgadmin.org> wrote:
> > > On Thu, Jul 23, 2009 at 6:48 PM, Guillaume
> > >
> > > Lelarge<guillaume@lelarge.info> wrote:
> > >> So here is a patch to fix this.
> > >
> > > Should we filter on *.* on Windows, and just * everywhere else? It'll
> > > make the code a little messier, but would make more sense to the user.
> >
> > +1 - it'll be much nicer for the user I think.
>
> OK, if you're two to think the same, I'll change my patch.

Here it is.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Вложения

Re: Patch to fix Debian Bug #451038

От
Dave Page
Дата:
On Thu, Jul 23, 2009 at 8:54 PM, Guillaume
Lelarge<guillaume@lelarge.info> wrote:

> Bug fixes:
>  http://archives.postgresql.org/pgadmin-support/2009-07/msg00093.php

Yup. No problem here.

>  http://archives.postgresql.org/pgadmin-support/2009-07/msg00090.php

I'm not thrilled about using the tab count to determine whether or not
to do something. That bites me pretty much every time I hack on a
dialog. Can we do something like "if (txtSql)", or can we not be
certain that it's been zeroed if it's not used?

> New functionality:
>  http://archives.postgresql.org/pgadmin-hackers/2009-07/msg00036.php

Looks OK in principle. I've got a sneaking feeling we've had issues
with using translated strings in SQL queries before though, but I
can't think what/when that was though.


--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

Re: Patch to fix Debian Bug #451038

От
Guillaume Lelarge
Дата:
Le vendredi 24 juillet 2009 à 10:24:06, Dave Page a écrit :
> On Thu, Jul 23, 2009 at 8:54 PM, Guillaume
>
> Lelarge<guillaume@lelarge.info> wrote:
> > Bug fixes:
> >  http://archives.postgresql.org/pgadmin-support/2009-07/msg00093.php
>
> Yup. No problem here.
>

Commited.

> >  http://archives.postgresql.org/pgadmin-support/2009-07/msg00090.php
>
> I'm not thrilled about using the tab count to determine whether or not
> to do something. That bites me pretty much every time I hack on a
> dialog. Can we do something like "if (txtSql)", or can we not be
> certain that it's been zeroed if it's not used?
>

I've tried to check different parameters but failed. The only way I found (and
the last I tried) is using the tab count. The last because I don't like it
either. One other way is to loop though all the tabs and check if we have an
"SQL" tab. It should protect us enough of the issue you're talking about?

> > New functionality:
> >  http://archives.postgresql.org/pgadmin-hackers/2009-07/msg00036.php
>
> Looks OK in principle. I've got a sneaking feeling we've had issues
> with using translated strings in SQL queries before though, but I
> can't think what/when that was though.

We do this on all statistics methods, so we can use a common DisplayStatistics
function. I don't see another way to get this. If we had an issue with this, I
think we would have fixed it.

Thanks for your comments.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Re: Patch to fix Debian Bug #451038

От
Dave Page
Дата:
On Fri, Jul 24, 2009 at 11:14 AM, Guillaume
Lelarge<guillaume@lelarge.info> wrote:

> I've tried to check different parameters but failed. The only way I found (and
> the last I tried) is using the tab count. The last because I don't like it
> either. One other way is to loop though all the tabs and check if we have an
> "SQL" tab. It should protect us enough of the issue you're talking about?

Can we set a flag at the point where we create the tabset, and check that later?

>> > New functionality:
>> >  http://archives.postgresql.org/pgadmin-hackers/2009-07/msg00036.php
>>
>> Looks OK in principle. I've got a sneaking feeling we've had issues
>> with using translated strings in SQL queries before though, but I
>> can't think what/when that was though.
>
> We do this on all statistics methods, so we can use a common DisplayStatistics
> function. I don't see another way to get this. If we had an issue with this, I
> think we would have fixed it.

OK. I think the issue I vaguely recall was in some other
(non-statistics) code, but as I say, I can't recall the details.


--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

Re: Patch to fix Debian Bug #451038

От
Guillaume Lelarge
Дата:
Le vendredi 24 juillet 2009 à 12:20:28, Dave Page a écrit :
> On Fri, Jul 24, 2009 at 11:14 AM, Guillaume
>
> Lelarge<guillaume@lelarge.info> wrote:
> > I've tried to check different parameters but failed. The only way I found
> > (and the last I tried) is using the tab count. The last because I don't
> > like it either. One other way is to loop though all the tabs and check if
> > we have an "SQL" tab. It should protect us enough of the issue you're
> > talking about?
>
> Can we set a flag at the point where we create the tabset, and check that
> later?
>

Another way to do it is to check the object before executing the Enable
method. See patch attached.

> >> > New functionality:
> >> >  http://archives.postgresql.org/pgadmin-hackers/2009-07/msg00036.php
> >>
> >> Looks OK in principle. I've got a sneaking feeling we've had issues
> >> with using translated strings in SQL queries before though, but I
> >> can't think what/when that was though.
> >
> > We do this on all statistics methods, so we can use a common
> > DisplayStatistics function. I don't see another way to get this. If we
> > had an issue with this, I think we would have fixed it.
>
> OK. I think the issue I vaguely recall was in some other
> (non-statistics) code, but as I say, I can't recall the details.

Commited, thanks.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Вложения

Re: Patch to fix Debian Bug #451038

От
Dave Page
Дата:
On Fri, Jul 24, 2009 at 1:23 PM, Guillaume
Lelarge<guillaume@lelarge.info> wrote:
> Le vendredi 24 juillet 2009 à 12:20:28, Dave Page a écrit :
>> On Fri, Jul 24, 2009 at 11:14 AM, Guillaume
>>
>> Lelarge<guillaume@lelarge.info> wrote:
>> > I've tried to check different parameters but failed. The only way I found
>> > (and the last I tried) is using the tab count. The last because I don't
>> > like it either. One other way is to loop though all the tabs and check if
>> > we have an "SQL" tab. It should protect us enough of the issue you're
>> > talking about?
>>
>> Can we set a flag at the point where we create the tabset, and check that
>> later?
>>
>
> Another way to do it is to check the object before executing the Enable
> method. See patch attached.

Seems clean, safe and effective.

:-)


--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

Re: Patch to fix Debian Bug #451038

От
Guillaume Lelarge
Дата:
Le vendredi 24 juillet 2009 à 14:38:03, Dave Page a écrit :
> On Fri, Jul 24, 2009 at 1:23 PM, Guillaume
>
> Lelarge<guillaume@lelarge.info> wrote:
> > Le vendredi 24 juillet 2009 à 12:20:28, Dave Page a écrit :
> >> On Fri, Jul 24, 2009 at 11:14 AM, Guillaume
> >>
> >> Lelarge<guillaume@lelarge.info> wrote:
> >> > I've tried to check different parameters but failed. The only way I
> >> > found (and the last I tried) is using the tab count. The last because
> >> > I don't like it either. One other way is to loop though all the tabs
> >> > and check if we have an "SQL" tab. It should protect us enough of the
> >> > issue you're talking about?
> >>
> >> Can we set a flag at the point where we create the tabset, and check
> >> that later?
> >
> > Another way to do it is to check the object before executing the Enable
> > method. See patch attached.
>
> Seems clean, safe and effective.
>
> :-)

Thanks. Commited.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Re: Patch to fix Debian Bug #451038

От
Guillaume Lelarge
Дата:
Le jeudi 23 juillet 2009 à 22:37:18, Guillaume Lelarge a écrit :
> Le jeudi 23 juillet 2009 à 21:18:22, Guillaume Lelarge a écrit :
> > Le jeudi 23 juillet 2009 à 21:09:26, Magnus Hagander a écrit :
> > > On Thu, Jul 23, 2009 at 20:56, Dave Page<dpage@pgadmin.org> wrote:
> > > > On Thu, Jul 23, 2009 at 6:48 PM, Guillaume
> > > >
> > > > Lelarge<guillaume@lelarge.info> wrote:
> > > >> So here is a patch to fix this.
> > > >
> > > > Should we filter on *.* on Windows, and just * everywhere else? It'll
> > > > make the code a little messier, but would make more sense to the
> > > > user.
> > >
> > > +1 - it'll be much nicer for the user I think.
> >
> > OK, if you're two to think the same, I'll change my patch.
>
> Here it is.

No comments on this. I suppose it's good enough? Can I commit it?


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Re: Patch to fix Debian Bug #451038

От
Dave Page
Дата:
Sorry - please go ahead. I'm sure it's fine.

I'm trying to rely more on the other senior devs these days - it's
hard to find time to check every patch, and you've shown long ago that
your standard of work is at least as  high as mine, probably much
higher :-)

 Lelarge <guillaume@lelarge.info> wrote:
> Le jeudi 23 juillet 2009 à 22:37:18, Guillaume Lelarge a écrit :
>> Le jeudi 23 juillet 2009 à 21:18:22, Guillaume Lelarge a écrit :
>> > Le jeudi 23 juillet 2009 à 21:09:26, Magnus Hagander a écrit :
>> > > On Thu, Jul 23, 2009 at 20:56, Dave Page<dpage@pgadmin.org> wrote:
>> > > > On Thu, Jul 23, 2009 at 6:48 PM, Guillaume
>> > > >
>> > > > Lelarge<guillaume@lelarge.info> wrote:
>> > > >> So here is a patch to fix this.
>> > > >
>> > > > Should we filter on *.* on Windows, and just * everywhere else?
>> > > > It'll
>> > > > make the code a little messier, but would make more sense to the
>> > > > user.
>> > >
>> > > +1 - it'll be much nicer for the user I think.
>> >
>> > OK, if you're two to think the same, I'll change my patch.
>>
>> Here it is.
>
> No comments on this. I suppose it's good enough? Can I commit it?
>
>
> --
> Guillaume.
>  http://www.postgresqlfr.org
>  http://dalibo.com
>


--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

Re: Patch to fix Debian Bug #451038

От
Guillaume Lelarge
Дата:
Le jeudi 6 août 2009 à 18:14:01, Dave Page a écrit :
> Sorry - please go ahead. I'm sure it's fine.
>

Commited, thanks.

> I'm trying to rely more on the other senior devs these days - it's
> hard to find time to check every patch, and you've shown long ago that
> your standard of work is at least as  high as mine, probably much
> higher :-)
>

Not sure you're right. But thanks :)


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com