[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    Re: [PATCH] Konqueror Tabs
From:       David Faure <david () mandrakesoft ! com>
Date:       2002-03-21 11:45:11
[Download RAW message or body]

On Tuesday 19 March 2002 01:16, Doug Hanley wrote:
> On Monday 18 March 2002 08:48 am, Chris Howells wrote:
> > On Monday 18 March 2002 4:23 am, Doug Hanley wrote:
> > > OK, thanks to all who responded to my previous message.  I have the \
> > > patch ready now, hope it works well.
> > > 
> > > http://prescott.dyndns.org/konqtabs-031702-2.diff
> > 
> > Arg! Timeout!
> > 
> > Any chance of putting it on a different server?
> 
> Sorry about that, that will teach me to try and host a webserver on a \
> P120. Actually, its probably because of Adelphia (my cable internet \
> provider). 
> Try this one:
> 
> http://www.cse.buffalo.edu/~jphanley/konqtabs-031702-2.diff

Thanks for the updated patch, and for merging with my changes there
(I took your previous patch into account, when e.g. making the main \
container be always created etc.)

I'm surprised that you use a separate dir for developing, instead of \
hacking in the original sources and using "cvs diff". With a separate dir, \
no merging automatically happens, and for that reason your patch reverts \
some of the last fixes, for instance:

-      // The last "true" means: force auto-embed even if user setting is \
                "separate viewer",
-      // since this window has no view yet - we don't want to keep an \
                empty mainwindow.
-      // This can happen with e.g. application/pdf from a target="_blank" \
                link, or window.open.
-      childView = m_pViewManager->splitView( Qt::Horizontal, serviceType, \
serviceName, false, true ); +
+      childView = m_pViewManager->Initialize( serviceType, serviceName );


   // reimplement from KParts::MainWindow
-  virtual void slotSetStatusBarText( const QString &text );
+  void slotSetStatusBarText( const QString &text );
Not very useful ;)

Any reason for the removePart call added to ~KonqView? I'm a bit reluctant
to change anything in the destruction code at this point, unless really \
necessary ;-) Ah, I see changes in the "suicidal parts" support - I assume \
you tested them with typing exit in the embedded konsolepart, too?
That's just some feedback from reading the patch w/o testing it, feel free \
to ignore if it's all working fine.

What's the docContainer notion (in the profiles) ?

Thanks a lot for this work. I'm very happy that Konqueror will have tabs in \
KDE-3.1 :)

PS: please use diff -b for future patches, konq_viewmgr.cc has been \
reindented it seems so the patch is quite hard to read there.

-- 
David FAURE, david@mandrakesoft.com, faure@kde.org
http://people.mandrakesoft.com/~david/, http://www.konqueror.org/
KDE, Making The Future of Computing Available Today


[prev in list] [next in list] [prev in thread] [next in thread]