Re: pgAdmin Event Trigger Compatibility - Mailing list pgadmin-hackers

OK. Some comments:

+    if(newData->GetMetaType() == PGM_SCHEMA && !newData->IsCollection())

+            {

+                    doNeedEvtTrgRefresh = true;

+            }

+

+ // Event trigger's backend functions are at schema level.

+ // Hence, we can consider the Event Triggers are partially belongs to at schema level.

+ // So, if any schema got refreshed, we are refreshing the event trigger collection as like schema's object.

+ // It's a special case, which effects the schema operations on the event triggers as well.

+ //

+            if (doNeedEvtTrgRefresh)

+            {

+                    doNeedEvtTrgRefresh = false;

+                    Refresh(browser->GetObject(browser->GetNextSibling(browser->GetItemParent(browser->GetSelection()))));

+            } 

* Why both with the doNeedEvtTrgRefresh flag here? As it's not used anywhere else, why not just put the Refresh() call into the first conditional?

* I assume the Refresh call is there to find the "Event Triggers" node and refresh it? If so, there's no guarantee that the next sibling will actually be the event triggers node - in the future, we may add a new node type that sits in that position, or the user may have enabled or disabled some node types, including Event Triggers.

* The same comment as above applies to browser->GetPrevSibling().

Thanks.