Dave Page wrote:
-----Original Message----- From: "Andreas
Pflug"<[EMAIL PROTECTED]> Sent: 30/04/06 01:44:21 To:
"pgadmin-hackers"<pgadmin-hackers@postgresql.org>, "Dave
Page"<dpage@vale-housing.co.uk> Subject: Lightspeed for frmQuery and
other issues.
as announced I realized the virtual listview implementation in
frmQuery; the code became amazingly slim. Since there's no more
data retrieval, the retrieval time shrinks to precisely 0.00
microseconds which should be sufficiently fast to justify omitting
this value :-)
Cool, nice work. I assume all the clipboard stuff still works?
Depends on what you call "the clipboard stuff". Everything that worked
in 1.4 still works, i.e. single/multiple row copy. To select columns,
there's a special syntax to the SELECT command....
I left grid stuff #ifdef'ed in the source, but it can't work for
now. When this is reworked, I'd really appreciate if the interface
of ctlSQLResult isn't altered again (there shouldn't be a need to
touch frmQuery), and until a different solution is accepted the
alternate #define USE_LISTVIEW should remain.
If it is fully working, I see no reason to change it again. The only
other mod I had in mind was full/multiple row pasting in the edit
grid.
pasting?
At the same time, I noticed how reporting was added to pgAdmin, and
was quite horrified. The menu refactoring was done to avoid base
class
Had a feeling you would be.
cluttering, and now it is massively reinvented. Any adding to
menu.h is plain wrong for frmMain menu, any code adding in
events.cpp (beyond minor submenu handling, i.e. calling
enableSubmenu) too, touching base factory classes even more. All
reporting stuff should be implemented in frmReport, not in pgObject
or so. Please do refactor it using factories.
Well the code was modelled as closely as possible on the 'new object'
menu code, and given the total lack of comments or other documentaion
of the factory code it's not exactly easy to understand either intent
or implementation.
You could have asked...
Here's (from memory) what was done:
- The new menu was added using the menu factories, per the new object
menu.
Reporting isn't comparable to that. See Edit(filtered/lmited)Grid (or
the new scripting stuff)
- menu ids were added in menu.h because multiple object types need to
share specific IDs for the same report.
menu ids are supposed to be handled by the factory.
- Each object type, via the base class provides it's own menu, per
the new object menu.
- event.cpp has a minimal handler for the menu.
- Each report is produced by the object itself (because that's where
the info is, per the main UI views.
No, it isn't; its switch-coded in the base class. Certainly ok to add
some object specific helper methods to pgXXX classes, but not to create
a form from pgObject.
- Properties/stats reports etc. Are implemented in
pgObject/pgCollection to minimise code duplication.
Common methods might go here (AFAICS none is necessary), but the work
itself should be done outside. object->CreateReport is the wrong style
to do that; in the sense of the menu factory stuff it's an action that's
performed on an object, not an object's method.
To be precise: pgObject, pgCollection, should be rolled back
except for HasXXX helper methods, base/xxx completely.
If there's something wrong with any particular part of that you'll
need to explain why, and how you think it should be don in a lot more
detail, 'cos as far as I can see I've followed existing design pretty
closely.
No, you did the opposite; you modified base classes that aren't supposed
to be touched. To add features like this is what has to be done:
- add frmXXX with its factories
Every factory (one factory per menu entry!) has
- constructor for use in frmMain
- CheckEnable (virtual)
- StartDialog (virtual)
- add instantiating the factories in frmMain
- when a new submenu is involved, add enableSubmenu(xxx) to events.cpp.
Actually, even touching events.cpp is too much, should done by
registering it in frmMain too. I'll refactor that, so that only *one*
single core file has to be modified.
Since frmQuery et al aren't supposed to be extended too often, the usual
MNU_XXX style is to be used there, so MNU_QUICKREPORT is fine.
BTW, I wonder why the reporting is creating HTML, not XML.
Because XML is good for data exchange, not presentation. You will
note that it is XHTML 1.0 Strict though.
Isn't XSL/XSLT supposed to deliver the specific rendering? The current
implementation looks very special to me, not reusable e.g. for a
compilation of multiple reports.
Another topic: I realized that the maxRows option (which is obsolete for
frmQuery now) is used for some job statistics. I'm not sure if using
that limit is a good idea.
Regards,
Andreas
---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
