Re: [FEATURE] Add schema option to all relevant objects

On Fri, 2011-07-08 at 20:06 +0100, Thom Brown wrote:
> On 8 July 2011 19:46, Guillaume Lelarge <guillaume(at)lelarge(dot)info> wrote:
> > On Thu, 2011-07-07 at 23:24 +0100, Thom Brown wrote:
> >> On 7 July 2011 23:20, Guillaume Lelarge <guillaume(at)lelarge(dot)info> wrote:
> >> > On Wed, 2011-07-06 at 11:59 +0100, Thom Brown wrote:
> >> >> add_schemas_to_all_items_filtered.patch - same patch as before, but
> >> >> with the above patch contents removed or adjusted to assume the fixes
> >> >> have been applied.
> >> >>
> >> >
> >> > Not yet done. I'll try to take care of this tomorrow.
> >>
> >> Awesomes. Thanks Guillaume. Please ensure I haven't sneaked anything
> >> in I shouldn't have, and that I don't break anything for previous
> >> versions.
> >>
> >
> > OK, just a few things that I fixed on your patch:
> > * pgadmin/schema/pgIndex.cpp contained reversed change of one of your
> > previous patches, I don't keep that.
>
> Good spot.
>
> > * all extension changes are wrong according to me because they aren't
> > schema objects, but database objects. I don't keep them.
>
> The reason why I based it on schema is because I wanted it to inherit
> the schema combobox object and its source for a list of schemas so
> lots of redundant code could be removed. There should be no
> functional difference, but I'm probably missing the point here. :)
>
> > * you added possibility to change a type, and a sequence, but forgot to
> > add the code to detect the changes in CheckChange. I fixed that
>
> Good spot for sequence, but looking at the patch, the check is in
> there for dlgType.
>

I should have forgotten to "git diff" once more.

> > * in dlgForeignTable.cpp, you declare a direction variable, but don't
> > use it. Fixed that too.
>
> Hehe, that wasn't mine. It was in there already. :)
>

Oops, sorry :)

> > * still in dlgForeignTable.cpp, you enable the schema widget only when
> > the user is connected in 9.1 but this guy can have this window only
> > when he's on 9.1. Fixed that.
>
> Yes, that check didn't need to be in there.
>
> > And that should be all. The fixed patch is attached. There is one
> > remaining issue: how to refresh the object's parent node in the new
> > schema?
>
> I played around with rebuilding the node path with the new schema name
> to refresh it, but I kept getting endless data type issues, and have
> no idea how this node stuff works, so I'm not sure what to do about
> that. Have any guidance? If not, I might have another attempt at
> working out how to manipulate nodes.
>

I'll try to look into this, but I don't think I'll be lucky.

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