Fwd: How to hide/show toolbar from extension?

Development
1

It doesn’t seem to be necessary at this point, I think just adding a dock
extension, and commenting that is we break ABI then a common base might be
appropriate. How much would the two classes share in a common base? If not
very much it would be more appropriate to keep the inheritance relatively
shallow, if lots then I would go for it.

My proposal:

#define AVOGADRO_EXTENSION(i, t, d)
…
#define AVOGADRO_EXTENSION_FACTORY(c)
…
class A_EXPORT AbstractExtension : public Plugin
{
Q_OBJECT
public:
AbstractExtension(QObject *parent = 0);
~AbstractExtension
enum MoleculeChangedHint {…};
Plugin::Type type() const;
QString typeName() const;
virtual void writeSettings(QSettings &settings) const;//=0; // don’t understand why they are here, should be =0 or removed
virtual void readSettings(QSettings &settings);//=0; // or introduce some automagical thing that should read&save properties~variables, declared by extension
public Q_SLOTS:
virtual void setMolecule(Molecule *molecule);
Q_SIGNALS:
void message(const QString &m);
void moleculeChanged(Molecule , int);
void performCommand(QUndoCommand);
};

// DialogExtension
class Extension : public AbstractExtension
{
virtual QList actions() const = 0;
virtual QString menuPath(QAction action) const = 0; // maybe all this extensions must have action in menu? or how it could be called?
virtual int usefulness() const; //=0; // menu order is needed only for dialog extensions
virtual QUndoCommand performAction(QAction *action, GLWidget *widget) = 0;
// maybe something else
};

class DockExtension : public AbstractExtension
{
virtual QDockWidget * dockWidget() = 0;
virtual Qt::DockWidgetArea prefferredDockArea() const; // one prefferred area
// maybe something else
};

// maybe also useful
class NetworkUser // is not inherited from anything; will be mixed in extension classes which want network as second base class
{
//some network related stuff
};

See that dialog and dock extensions must have different sets of methods. Also, type() could be overridden to distinct ones from anothers
If implement DockExtension if separate class, too many methods will be duplicated
Addition of property will be painless, but it’ll intoduce more mess in API: currently, some functions must be overridden in any cases, even if they aren’t needed, other need this only for some kinds of extension (docks), some things are set up with properties, etc.
I know it can wait, and there aren’t any massive code duplications, but making API more clear will simplify task of extension writing. Almost everything works correctly in my dock, I onlu wished to contribute into core structure to make it more clear.
Also I stay for more dock extensions, because docks doesn’t appear on top of target molecule

–
Regards,
Konstantin

2

This seems like the most reasonable course of action to me. Thoughts from
anyone else about this? I think we can break ABI, but it should not be
done lightly.

IMHO, Avogadro is not so ready for production to be so worried about ABI :slight_smile:
Moreover, it’s not Java, and recompilation is inevitable in many
situations (e.g., update of Qt in distribution)
I disagree. Qt does guarantee API/ABI stability (as does kdelibs), and so
recompilation is not necessarily required. Very pleased it is not Java, but I
disagree with your premise that we should give up ABI stability.

  1. Nevertheless, sometimes issues occur
  2. Distributions are compiled with fixed version of all libraries (e.g, Ubuntu 9.04 uses Qt 4.5.0 and will not update to 4.5.2(3) it until the end of lifecycle)

In my opinion creating a more specialized plugin type that dock

extensions derive from is preferable in this instance at least. I have
done a lot of packaging for Gentoo Linux, and breaking ABI in minor
releases is a major headache for source and binary distros.

Distros often include outdated version of Avogadro or doesn’t include at
all (only in 3rd party repositories)
I am not sure what point you are making here. It is being included in more and
more places, shouldn’t we try to make a high quality, stable library that is
easy to package?

Returning to mentioned Ubuntu distribution, recent 9.10 release includes avogadro 0.9.7. I don’t think that 1.1 (if released) will be included into major distributions soon, they’ll take 1.0.x series

It means all applications
using that library have to be locked to minor versions, rebuilt and
bumped at the same time.

Do you think there are lots of them right now?
No, does that mean we should not try?

Of course, we should try, but probably we shouldn’t delay refactorization if it’s needed.

If we want Avogadro to be a useful and easy to use library we
should be doing our best to preserve both API and ABI.

If we will not make reasonable changes to ABI/API now, we will face
horrible problems with refactoring some years later
I am all for making reasonable changes when necessary. I think you are too
willing to break ABI.

No, I only proposed change that seems to be obvious for me

After having worked on Avogadro for several years, and
worrying about these issues. I never wanted to rule out ABI breaks, but making
them every minor revision would IMHO be a poor development philosophy.
In this case you can simply make a new DockExtension class, derive from
Plugin.

OK.

Also I can propose to create base class AbstractExtension, and inherit DockExtension from it. When you are ready to break ABI, Extension will be inherited from it too. Do you like this idea?

What is wrong with doing that and preserving the ABI as well? As a
community we decided ABI stability was important, and a milestone of 1.0. As a
dependency of Kalzium we made promised to provide an ABI stable library for
use in KDE. I think we should do our best to honor that promise.

Is Avogadro part of KDE? I thought it’s DE independent application

–
Regards,
Konstantin

P.S. Who is admin of the list? Do you need any help with setting “Explicit Reply-To: header” in Maiman Admin?

3

P.S. Who is admin of the list? Do you need any help with setting “Explicit Reply-To: header” in Maiman Admin?

I’m an admin of this list. I hate lists with explicit reply-to headers. Just “reply-to-all” and everyone will be happy.

-Geoff

P.S. For more on “Reply-To Considered Harmful”
http://www.unicom.com/pw/reply-to-harmful.html

4

On Saturday 20 February 2010 14:52:19 Geoffrey Hutchison wrote:

P.S. Who is admin of the list? Do you need any help with setting
“Explicit Reply-To: header” in Maiman Admin?

I’m an admin of this list. I hate lists with explicit reply-to headers.
Just “reply-to-all” and everyone will be happy.

-Geoff

P.S. For more on “Reply-To Considered Harmful”
Telepathy - Powering Successful Brands

I am totally with you on this. Hope that it is not changed.

Marcus

5

20.02.10, 15:04, “Marcus D. Hanwell” marcus@cryos.org:

On Saturday 20 February 2010 14:52:19 Geoffrey Hutchison wrote:

P.S. Who is admin of the list? Do you need any help with setting
“Explicit Reply-To: header” in Maiman Admin?

I’m an admin of this list. I hate lists with explicit reply-to headers.
Just “reply-to-all” and everyone will be happy.

-Geoff

P.S. For more on “Reply-To Considered Harmful”
Telepathy - Powering Successful Brands

I am totally with you on this. Hope that it is not changed.
Marcus

Ok, let it be

Regards,
Konstantin

6

On Saturday 20 February 2010 14:41:05 Konstantin Tokarev wrote:

This seems like the most reasonable course of action to me. Thoughts
from anyone else about this? I think we can break ABI, but it should
not be done lightly.

IMHO, Avogadro is not so ready for production to be so worried about
ABI :slight_smile: Moreover, it’s not Java, and recompilation is inevitable in
many situations (e.g., update of Qt in distribution)

I disagree. Qt does guarantee API/ABI stability (as does kdelibs), and so
recompilation is not necessarily required. Very pleased it is not Java,
but I disagree with your premise that we should give up ABI stability.

  1. Nevertheless, sometimes issues occur
  2. Distributions are compiled with fixed version of all libraries (e.g,
    Ubuntu 9.04 uses Qt 4.5.0 and will not update to 4.5.2(3) it until the end
    of lifecycle)

I have packaged libraries and applications for Gentoo, and have worked closely
with packagers for other distributions such as Ubuntu, Fedora, OpenSUSE. It
causes headaches ensuring all the reverse deps are satisfied when a library
breaks ABI. They can handle it, but are more likely to package newer versions
when the potential for breakage is reduced. We are not Qt, but it doesn’t mean
we should not aspire to providing a high quality library that can easily be
used and packaged.

In my opinion creating a more specialized plugin type that dock

extensions derive from is preferable in this instance at least. I
have done a lot of packaging for Gentoo Linux, and breaking ABI in
minor releases is a major headache for source and binary distros.

Distros often include outdated version of Avogadro or doesn’t include
at all (only in 3rd party repositories)

I am not sure what point you are making here. It is being included in
more and more places, shouldn’t we try to make a high quality, stable
library that is easy to package?

Returning to mentioned Ubuntu distribution, recent 9.10 release includes
avogadro 0.9.7. I don’t think that 1.1 (if released) will be included into
major distributions soon, they’ll take 1.0.x series

We missed their release deadline, it is all about timing and getting things to
line up. ABI stability is not relevant to the discussion of how far distros
lag behind our current release though IMHO.

It means all applications
using that library have to be locked to minor versions, rebuilt and
bumped at the same time.

Do you think there are lots of them right now?

No, does that mean we should not try?

Of course, we should try, but probably we shouldn’t delay refactorization
if it’s needed.

In this case I don’t think it is. We already have a common base for all
plugins, and they all derive from that. Right now the best solution for 1.y
series is to derive from that, and make a DockExtension plugin class, that
looks similar to Extension where it makes sense. They don’t need to have a
common abstract base, but if we were writing the classes today with no
constraints then they probably would.

If we want Avogadro to be a useful and easy to use library we
should be doing our best to preserve both API and ABI.

If we will not make reasonable changes to ABI/API now, we will face
horrible problems with refactoring some years later

I am all for making reasonable changes when necessary. I think you are
too willing to break ABI.

No, I only proposed change that seems to be obvious for me

Obvious, but not necessarily best for Avogadro. I think taking some extra care
to maintain both API and ABI stability where possible is well worth it. There
is no reason to make Extension so generic it tries to do everything when we
could make an additional, specialized extension type.

After having worked on Avogadro for several years, and

worrying about these issues. I never wanted to rule out ABI breaks, but
making them every minor revision would IMHO be a poor development
philosophy. In this case you can simply make a new DockExtension class,
derive from Plugin.

OK.

Also I can propose to create base class AbstractExtension, and inherit
DockExtension from it. When you are ready to break ABI, Extension will be
inherited from it too. Do you like this idea?

It doesn’t seem to be necessary at this point, I think just adding a dock
extension, and commenting that is we break ABI then a common base might be
appropriate. How much would the two classes share in a common base? If not
very much it would be more appropriate to keep the inheritance relatively
shallow, if lots then I would go for it.

What is wrong with doing that and preserving the ABI as well? As a
community we decided ABI stability was important, and a milestone of 1.0.
As a dependency of Kalzium we made promised to provide an ABI stable
library for use in KDE. I think we should do our best to honor that
promise.

Is Avogadro part of KDE? I thought it’s DE independent application

Avogadro has benefited from working closely with KDE. We are a pure Qt
library, but have had two Google Summer of Code projects, me being the first
in 2007. We replaced the young KDE chemical visualization code, had Benoit and
Carsten (primarily KDE devs) work on our code, worked with Eigen.

So there is certainly a history of collaboration, and I think that is a great
thing that we should endeavour to continue. We are an independent project
though, managing our own releases, version control, web sites, direction etc.
Much of this is documented in blog posts and mailing lists.

Marcus

7

Also it could be useful to add to DialogExtension virtual QDialog * dialog() const; to give application more control over extensions’ dialogs

8

On Saturday 20 February 2010 16:46:55 Konstantin Tokarev wrote:

It doesn’t seem to be necessary at this point, I think just adding a dock
extension, and commenting that is we break ABI then a common base might
be appropriate. How much would the two classes share in a common base?
If not very much it would be more appropriate to keep the inheritance
relatively shallow, if lots then I would go for it.

My proposal:

#define AVOGADRO_EXTENSION(i, t, d)
…
#define AVOGADRO_EXTENSION_FACTORY©
…
class A_EXPORT AbstractExtension : public Plugin
{
Q_OBJECT
public:
AbstractExtension(QObject *parent = 0);
~AbstractExtension
enum MoleculeChangedHint {…};
Plugin::Type type() const;
QString typeName() const;
virtual void writeSettings(QSettings &settings) const;//=0; // don’t
understand why they are here, should be =0 or removed virtual void
readSettings(QSettings &settings);//=0; // or introduce some automagical
thing that should read&save properties~variables, declared by extension
public Q_SLOTS:
virtual void setMolecule(Molecule *molecule);
Q_SIGNALS:
void message(const QString &m);
void moleculeChanged(Molecule , int);
void performCommand(QUndoCommand);
};

Like I said - at this stage I don’t think the AbstractExtension class makes
sense.

// DialogExtension
class Extension : public AbstractExtension
{
virtual QList actions() const = 0;
virtual QString menuPath(QAction action) const = 0; // maybe all this
extensions must have action in menu? or how it could be called? virtual
int usefulness() const; //=0; // menu order is needed only for dialog
extensions virtual QUndoCommand performAction(QAction *action, GLWidget
*widget) = 0; // maybe something else
};

You cannot change the inheritance of Extension. That is as bad as adding a new
virtual. Extension should be left out of any of this work. If you really want
to have an AbstractExtension you could, but that should probably just be added
when the code is refactored.

class DockExtension : public AbstractExtension
{
virtual QDockWidget * dockWidget() = 0;
virtual Qt::DockWidgetArea prefferredDockArea() const; // one
prefferred area // maybe something else
};

You would be better off leaving Extension alone, copying it and renaming to
DockExtension, adding your new virtuals and using that as an additional plugin
type. Extension can not have new virtuals, nor can its inheritance change.

// maybe also useful
class NetworkUser // is not inherited from anything; will be mixed in
extension classes which want network as second base class {
//some network related stuff
};

I don’t think it would be worth it. We use the Qt networking classes, which
are easy to use. Any extension can take advantage of them. We might need to
add some method of timing out globally and not making network requests. If
they are asynchronous it doesn’t matter too much though.

See that dialog and dock extensions must have different sets of methods.
Also, type() could be overridden to distinct ones from anothers If
implement DockExtension if separate class, too many methods will be
duplicated Addition of property will be painless, but it’ll intoduce more
mess in API: currently, some functions must be overridden in any cases,
even if they aren’t needed, other need this only for some kinds of
extension (docks), some things are set up with properties, etc. I know it
can wait, and there aren’t any massive code duplications, but making API
more clear will simplify task of extension writing. Almost everything
works correctly in my dock, I onlu wished to contribute into core
structure to make it more clear. Also I stay for more dock extensions,
because docks doesn’t appear on top of target molecule

You should definitely override type(). Duplication of methods is not ideal,
but not the end of the world. These methods are basically stubs anyway.
Another option is to have DockExtension inherit from Extension, and implement
the pure virtuals that DockExtensions do not need, so they could be ignored in
the derived plugins.

Is that clearer?

Marcus

9

OK, I’ve inherited DockExtension without breaking ABI.
How do I use new methods of DockExtensions from mainwindow? Is dynamic_cast ok?

–
Regards,
Konstantin

10

Hi,

On Mon, Feb 22, 2010 at 5:55 PM, Konstantin Tokarev annulen@yandex.ru wrote:

OK, I’ve inherited DockExtension without breaking ABI.
How do I use new methods of DockExtensions from mainwindow? Is dynamic_cast ok?

If you want to determine if a class (Extension pointer) inherits from
DockExtension, you could use dynamic_cast. However, since you also
inherit from QObject (via Plugin), qobject_cast can also be used and
is faster.

for example:
foreach(Extension *extension, …) {
DockExtension ptr = qobject_cast<DockExtension>(extension)
if (ptr)
ptr->dockExtensionSpecificFunction(…)
}

Does this help?

Cheers,
Tim

–
Regards,
Konstantin

Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

Avogadro-devel mailing list
Avogadro-devel@lists.sourceforge.net
avogadro-devel List Signup and Options

11

OK, I’ve inherited DockExtension without breaking ABI.
How do I use new methods of DockExtensions from mainwindow? Is dynamic_cast ok?
If you want to determine if a class (Extension pointer) inherits from
DockExtension, you could use dynamic_cast. However, since you also
inherit from QObject (via Plugin), qobject_cast can also be used and
is faster.
for example:
foreach(Extension *extension, …) {
DockExtension *ptr = qobject_cast(extension)
if (ptr)
ptr->dockExtensionSpecificFunction(…)
}
Does this help?

Yes, it works, thanks!
I’ve asked about dynamic_cast because Qt applications are often compiled Windows without RTTI; qobject_cast seems not to use RTTI

Regards,
Konstantin