[PATCH] minor sublime modifications
mbr.nxi at gmail.com
Tue Jul 29 07:44:35 UTC 2008
On Monday 28 July 2008 22:14:30 Andreas Pakulat wrote:
> On 28.07.08 12:46:20, Manuel Breugelmans wrote:
> Here are the comments on the actual patch:
> > Index: interfaces/iuicontroller.h
> > ===================================================================
> > --- interfaces/iuicontroller.h (revision 838440)
> > +++ interfaces/iuicontroller.h (working copy)
> > @@ -69,12 +69,20 @@
> > virtual void switchToArea(const QString &areaName, SwitchMode
> > switchMode) = 0;
> > + /** Register a toolview and try to add it to the UI.
> > + A user will be able to create new tool views like this one. */
> > virtual void addToolView(const QString &name, IToolViewFactory
> > *factory) = 0; + /** Spawn a toolview, return true if successful.
> > + Use it for one-off views, which should not be added by a user
> > directly. */ + virtual bool showToolView(const QString &name,
> > IToolViewFactory* factory) = 0;
> ?? I have no idea what the difference between showToolView and
> addToolView is from this comment. What does "one off" mean?
'one-off' stands for 'once'. I agree that these methods are not ideally named.
> > virtual void removeToolView(IToolViewFactory *factory) = 0;
> > /** @return active mainwindow or 0 if no such mainwindow is
> > active.*/ virtual KParts::MainWindow *activeMainWindow() = 0;
> > + /** Do not store view configuration for toolviews with id @param
> > toolViewId */ + virtual void excludeFromSave(const QString&
> > toolViewId) = 0; +
> This should be a flag for addToolView. Either that or we should have a
> virtual function on a base class of the widget thats returned from the
> factory that actually does the saving. (which we will have at some point
> anyway). Then a subclass of this base-widget can decide to override it
> and instead of calling the super-implementation it only does what it
> wants to do.
As you want it ... adding boolean parameters is bad taste though (also see
> > + if
> > (d->saveBlackList.contains(view->document()->documentType())) continue;
> That should be two lines for better readability.
> About the rest: the new virtuals and that stuff I'm not sure thats ok
> all through. I'd have to apply the patch and re-check everything, not
> time tonight for that unfortunately and also not tomorrow.
UiToolViewFactory wraps an IToolVIewFactory, instead of implementing it. I
don't see a problem (assuming you refer to viewCreated()). The only small
change in semantics is the time at which it gets invoked, a wee bit earlier in
the chain now.
More information about the KDevelop-devel