[calligra] /: there is always a KoPart for a document

[Boudewijn Rempt]

On Sunday 02 September 2012 Sep, Friedrich W. H. Kossebau wrote:
> Hi Inge,
> 
> Am Sonntag, 2. September 2012, 13:59:31 schrieb Inge Wallin:
> > On Sunday, September 02, 2012 13:16:44 Friedrich W. H. Kossebau wrote:
> > > Git commit 32ab7701154b7906a249d9c029ad9f18f67a1a6a by Friedrich W. H.
> > > Kossebau. Committed on 02/09/2012 at 12:49.
> > > Pushed by kossebau into branch 'master'.
> > > 
> > > there is always a KoPart for a document
> > > 
> > > M  +2    -3    libs/main/KoDocument.cpp
> > > M  +1    -1    words/part/KWDocument.h
> > > 
> > > http://commits.kde.org/calligra/32ab7701154b7906a249d9c029ad9f18f67a1a6a
> > > 
> > > diff --git a/libs/main/KoDocument.cpp b/libs/main/KoDocument.cpp
> > > index bd18279..78e2498 100644
> > > --- a/libs/main/KoDocument.cpp
> > > +++ b/libs/main/KoDocument.cpp
> > > @@ -1002,9 +1002,8 @@ bool KoDocument::openUrl(const KUrl & _url)
> > > 
> > >          QFile::remove(url.toLocalFile()); // and remove the autosave file
> > >      
> > >      }
> > >      else {
> > > 
> > > -        if (d->parentPart) {
> > > -            d->parentPart->addRecentURLToAllShells(_url);
> > > -        }
> > > +        d->parentPart->addRecentURLToAllShells(_url);
> > > +
> > > 
> > >          if (ret) {
> > >          
> > >              // Detect readonly local-files; remote files are assumed to
> > >              be
> > > 
> > > writable, unless we add a KIO::stat here (async). KFileItem file(url,
> > > mimeType(), KFileItem::Unknown);
> > > diff --git a/words/part/KWDocument.h b/words/part/KWDocument.h
> > > index 0803fb9..e0e4dd1 100644
> > > --- a/words/part/KWDocument.h
> > > +++ b/words/part/KWDocument.h
> > > 
> > > @@ -64,7 +64,7 @@ public:
> > >      /**
> > >      
> > >       * Constructor, normally called by the KWFactory::createPartObject()
> > >       */
> > > 
> > > -    explicit KWDocument(KoPart *part = 0);
> > > +    explicit KWDocument(KoPart *part);
> > > 
> > >      ~KWDocument();
> > >      
> > >      // KoShapeBasedDocumentBase interface
> > 
> > I object to this change.  I think it is valuable to be able to represent a
> > document in memory without having a visual view of it. I think a document
> > should be aware of its views as little as possible.
> 
> Sure, that is/might be the long-term goal (my Kasten framework e.g. has 
> completely view-unaware documents :) ).
> 
> But the current state is that the Calligra code everywhere relies on the 
> document to have a parent part. So these lines I changed were just 
> inconsistent with the rest.
> 
> Especially the KWDocument constructor call was simply a crash guarantee with a 
> 0 pointer passed for the parent.
> 
> The other change with the if-parentpart guard removing was done as the code 
> around it does not check for the parent part anyway. Like the whole code, from 
> what I checked.
> 
> Sorry for not passing a review for this, this commit seemed pretty obvious to 
> me, looks I was wrong.
> 

Well, sorta, yes, no -- in between. The thing is, I did a similar commit for krita recently. And I think it's needed as an intermediate step. But we need to be clear on the goal: KoDocument-based classes should be able to function completely withouth a KoPart around. That's what we need to work towards.


-- 
Boudewijn Rempt
http://www.valdyas.org, http://www.krita.org, http://www.boudewijnrempt.nl