Thread: [PATCH] fix newly added server being lost when crashing

Ashesh, can you look at this please?

Thanks.

On Sun, Jan 5, 2014 at 10:18 PM, Kaarel Moppel <kaarel.moppel@gmail.com> wrote:
> Hi,
>
> Sending some 3 small fixes in separate emails for issues that me and my
> colleagues found really annoying. This is my first try at Pgadmin and I
> haven't done too much C++ though so in case something should be corrected
> let me know and I'd be happy to do that.
>
> Cheers,
> Kaarel
>
>

> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi,

Sending some 3 small fixes in separate emails for issues that me and my colleagues found really annoying. This is my first try at Pgadmin and I haven't done too much C++ though so in case something should be corrected let me know and I'd be happy to do that.

Cheers,

Kaarel

--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Hi Kaarel,

I think - everything looks good except:

1. Please use TAB instead of 4 spaces

2. You don't require to define Flush function after inheriting the wxConfig class publicly.

     Or,

    Define it only if you keep the wxConfig inherited protectively.

On Mon, Jan 6, 2014 at 3:48 AM, Kaarel Moppel <kaarel.moppel@gmail.com> wrote:

Hi,

Sending some 3 small fixes in separate emails for issues that me and my colleagues found really annoying. This is my first try at Pgadmin and I haven't done too much C++ though so in case something should be corrected let me know and I'd be happy to do that.

Cheers,

Kaarel

--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

--

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

Ok, got it. Corrected version in attach now.

On Mon, Jan 6, 2014 at 12:29 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Hi Kaarel,

I think - everything looks good except:

1. Please use TAB instead of 4 spaces

2. You don't require to define Flush function after inheriting the wxConfig class publicly.

     Or,

    Define it only if you keep the wxConfig inherited protectively.

On Mon, Jan 6, 2014 at 3:48 AM, Kaarel Moppel <kaarel.moppel@gmail.com> wrote:

Hi,

Sending some 3 small fixes in separate emails for issues that me and my colleagues found really annoying. This is my first try at Pgadmin and I haven't done too much C++ though so in case something should be corrected let me know and I'd be happy to do that.

Cheers,

Kaarel

--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

--

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

On Thu, Jan 9, 2014 at 4:41 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Looking good to me.

If nobody has any objection, then I can check-in.

NOTE:
I don't know the reason for the class sysSettings to inherit the wxConfig privately in original implementation.

It was done by Dave in this commit: cfbae7d378766ab8cf138123186a024afffc061e.

That was a long time ago, but if memory serves it was intentionally done to force the use of the sysSettings members, and not wxConfig, because some of them have similar names (because there are only so many ways to name some generic functions), but do additional things that we need; and in the past some code had crept in that used wxConfig directly, bypassing our additional code and subtly breaking things.

 

On Thu, Jan 9, 2014 at 5:42 AM, Kaarel Moppel <kaarel.moppel@gmail.com> wrote:

Ok, got it. Corrected version in attach now.

On Mon, Jan 6, 2014 at 12:29 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Hi Kaarel,

I think - everything looks good except:

1. Please use TAB instead of 4 spaces

2. You don't require to define Flush function after inheriting the wxConfig class publicly.

     Or,

    Define it only if you keep the wxConfig inherited protectively.

On Mon, Jan 6, 2014 at 3:48 AM, Kaarel Moppel <kaarel.moppel@gmail.com> wrote:

Hi,

Sending some 3 small fixes in separate emails for issues that me and my colleagues found really annoying. This is my first try at Pgadmin and I haven't done too much C++ though so in case something should be corrected let me know and I'd be happy to do that.

Cheers,

Kaarel

--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

--

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Dave,

In that case, we should not change the inheritance mode.

Kaarel,

I've attached the patch as per Dave's comment.
Can you please test it at your end?

On Thu, Jan 9, 2014 at 2:35 PM, Dave Page <dpage@pgadmin.org> wrote:

On Thu, Jan 9, 2014 at 4:41 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Looking good to me.

If nobody has any objection, then I can check-in.

NOTE:
I don't know the reason for the class sysSettings to inherit the wxConfig privately in original implementation.

It was done by Dave in this commit: cfbae7d378766ab8cf138123186a024afffc061e.

That was a long time ago, but if memory serves it was intentionally done to force the use of the sysSettings members, and not wxConfig, because some of them have similar names (because there are only so many ways to name some generic functions), but do additional things that we need; and in the past some code had crept in that used wxConfig directly, bypassing our additional code and subtly breaking things.

 

On Thu, Jan 9, 2014 at 5:42 AM, Kaarel Moppel <kaarel.moppel@gmail.com> wrote:

Ok, got it. Corrected version in attach now.

On Mon, Jan 6, 2014 at 12:29 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Hi Kaarel,

I think - everything looks good except:

1. Please use TAB instead of 4 spaces

2. You don't require to define Flush function after inheriting the wxConfig class publicly.

     Or,

    Define it only if you keep the wxConfig inherited protectively.

On Mon, Jan 6, 2014 at 3:48 AM, Kaarel Moppel <kaarel.moppel@gmail.com> wrote:

Hi,

Sending some 3 small fixes in separate emails for issues that me and my colleagues found really annoying. This is my first try at Pgadmin and I haven't done too much C++ though so in case something should be corrected let me know and I'd be happy to do that.

Cheers,

Kaarel

--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

--

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

On Mon, Jan 13, 2014 at 5:13 AM, Kaarel Moppel <kaarel.moppel@gmail.com> wrote:

Hi Ashesh,

Got a "candidate expects 0 arguments, 1 provided".

Thanks.

I will check-in the code.

Changed this so it should be final now.

- settings->FlushChanges(false);

+ settings->FlushChanges();

On Thu, Jan 9, 2014 at 10:34 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Dave,

In that case, we should not change the inheritance mode.

Kaarel,

I've attached the patch as per Dave's comment.
Can you please test it at your end?

On Thu, Jan 9, 2014 at 2:35 PM, Dave Page <dpage@pgadmin.org> wrote:

On Thu, Jan 9, 2014 at 4:41 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Looking good to me.

If nobody has any objection, then I can check-in.

NOTE:
I don't know the reason for the class sysSettings to inherit the wxConfig privately in original implementation.

It was done by Dave in this commit: cfbae7d378766ab8cf138123186a024afffc061e.

That was a long time ago, but if memory serves it was intentionally done to force the use of the sysSettings members, and not wxConfig, because some of them have similar names (because there are only so many ways to name some generic functions), but do additional things that we need; and in the past some code had crept in that used wxConfig directly, bypassing our additional code and subtly breaking things.

 

On Thu, Jan 9, 2014 at 5:42 AM, Kaarel Moppel <kaarel.moppel@gmail.com> wrote:

Ok, got it. Corrected version in attach now.

On Mon, Jan 6, 2014 at 12:29 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Hi Kaarel,

I think - everything looks good except:

1. Please use TAB instead of 4 spaces

2. You don't require to define Flush function after inheriting the wxConfig class publicly.

     Or,

    Define it only if you keep the wxConfig inherited protectively.

On Mon, Jan 6, 2014 at 3:48 AM, Kaarel Moppel <kaarel.moppel@gmail.com> wrote:

Hi,

Sending some 3 small fixes in separate emails for issues that me and my colleagues found really annoying. This is my first try at Pgadmin and I haven't done too much C++ though so in case something should be corrected let me know and I'd be happy to do that.

Cheers,

Kaarel

--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

--

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi