[PATCH] Re: graphics cache file path horkage

2003-05-30 Thread John Levon
On Fri, May 30, 2003 at 02:24:22AM +0100, John Levon wrote:

> > I believe we should *always* store an absolute path at
> > runtime. At save time, we can convert it to a relative path.

Fixed patch. It seems to work for me. It fixes the Save As bug.

Please please test.

john

p.s. sorry, no, I'm not taking maintainership of this code

Index: factory.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/factory.C,v
retrieving revision 1.29
diff -u -p -r1.29 factory.C
--- factory.C   22 May 2003 10:40:55 -  1.29
+++ factory.C   30 May 2003 02:15:38 -
@@ -211,8 +211,7 @@ Inset * createInset(FuncRequest const & 
InsetGraphicsParams igp;
InsetGraphicsMailer::string2params(cmd.argument, igp);
InsetGraphics * inset = new InsetGraphics;
-   string const fpath = cmd.view()->buffer()->filePath();
-   inset->setParams(igp, fpath);
+   inset->setParams(igp);
return inset;
 
} else if (name == "include") {
Index: insets/insetgraphics.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/insets/insetgraphics.C,v
retrieving revision 1.176
diff -u -p -r1.176 insetgraphics.C
--- insets/insetgraphics.C  28 May 2003 23:09:14 -  1.176
+++ insets/insetgraphics.C  30 May 2003 02:15:44 -
@@ -182,7 +182,7 @@ void InsetGraphics::Cache::update(string
lyx::Assert(!file_with_path.empty());
 
string const path = OnlyPath(file_with_path);
-   loader.reset(file_with_path, parent_.params().as_grfxParams(path));
+   loader.reset(file_with_path, parent_.params().as_grfxParams());
 }
 
 
@@ -192,19 +192,20 @@ InsetGraphics::InsetGraphics()
 {}
 
 
-InsetGraphics::InsetGraphics(InsetGraphics const & ig,
-string const & filepath)
+#warning I have zero idea about the trackable()
+InsetGraphics::InsetGraphics(InsetGraphics const & ig)
: Inset(ig),
+ boost::signals::trackable(ig),
  graphic_label(uniqueID()),
  cache_(new Cache(*this))
 {
-   setParams(ig.params(), filepath);
+   setParams(ig.params());
 }
 
 
-Inset * InsetGraphics::clone(Buffer const & buffer) const
+Inset * InsetGraphics::clone(Buffer const &) const
 {
-   return new InsetGraphics(*this, buffer.filePath());
+   return new InsetGraphics(*this);
 }
 
 
@@ -222,8 +223,7 @@ dispatch_result InsetGraphics::localDisp
InsetGraphicsParams p;
InsetGraphicsMailer::string2params(cmd.argument, p);
if (!p.filename.empty()) {
-   string const filepath = cmd.view()->buffer()->filePath();
-   setParams(p, filepath);
+   setParams(p);
cmd.view()->updateInset(this);
}
return DISPATCHED;
@@ -327,18 +327,6 @@ BufferView * InsetGraphics::view() const
 void InsetGraphics::draw(BufferView * bv, LyXFont const & font,
 int baseline, float & x) const
 {
-   // MakeAbsPath returns params().filename unchanged if it absolute
-   // already.
-   string const file_with_path =
-   MakeAbsPath(params().filename, bv->buffer()->filePath());
-
-   // A 'paste' operation creates a new inset with the correct filepath,
-   // but then the 'old' inset stored in the 'copy' operation is actually
-   // added to the buffer.
-   // Thus, we should ensure that the filepath is correct.
-   if (file_with_path != cache_->loader.filename())
-   cache_->update(file_with_path);
-
cache_->view = bv->owner()->view();
int oasc = cache_->old_ascent;
 
@@ -408,10 +396,10 @@ Inset::EDITABLE InsetGraphics::editable(
 }
 
 
-void InsetGraphics::write(Buffer const *, ostream & os) const
+void InsetGraphics::write(Buffer const * buf, ostream & os) const
 {
os << "Graphics\n";
-   params().Write(os);
+   params().Write(os, buf->filePath());
 }
 
 
@@ -420,15 +408,15 @@ void InsetGraphics::read(Buffer const * 
string const token = lex.getString();
 
if (token == "Graphics")
-   readInsetGraphics(lex);
+   readInsetGraphics(lex, buf->filePath());
else
lyxerr[Debug::GRAPHICS] << "Not a Graphics inset!\n";
 
-   cache_->update(MakeAbsPath(params().filename, buf->filePath()));
+   cache_->update(params().filename);
 }
 
 
-void InsetGraphics::readInsetGraphics(LyXLex & lex)
+void InsetGraphics::readInsetGraphics(LyXLex & lex, string const & bufpath)
 {
bool finished = false;
 
@@ -455,7 +443,7 @@ void InsetGraphics::readInsetGraphics(Ly
// TODO: Possibly open up a dialog?
}
else {
-   if (! param

Re: graphics cache file path horkage

2003-05-30 Thread John Levon
On Fri, May 30, 2003 at 01:49:28AM +0100, John Levon wrote:

> I believe we should *always* store an absolute path at
> runtime. At save time, we can convert it to a relative path.

Here's an attempt at a patch. Not tested

john


Index: insetgraphics.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/insets/insetgraphics.C,v
retrieving revision 1.176
diff -u -p -r1.176 insetgraphics.C
--- insetgraphics.C 28 May 2003 23:09:14 -  1.176
+++ insetgraphics.C 30 May 2003 01:21:21 -
@@ -182,7 +182,7 @@ void InsetGraphics::Cache::update(string
lyx::Assert(!file_with_path.empty());
 
string const path = OnlyPath(file_with_path);
-   loader.reset(file_with_path, parent_.params().as_grfxParams(path));
+   loader.reset(file_with_path, parent_.params().as_grfxParams());
 }
 
 
@@ -192,19 +192,20 @@ InsetGraphics::InsetGraphics()
 {}
 
 
-InsetGraphics::InsetGraphics(InsetGraphics const & ig,
-string const & filepath)
+#warning I have zero idea about the trackable()
+InsetGraphics::InsetGraphics(InsetGraphics const & ig)
: Inset(ig),
+ boost::signals::trackable(ig),
  graphic_label(uniqueID()),
  cache_(new Cache(*this))
 {
-   setParams(ig.params(), filepath);
+   setParams(ig.params());
 }
 
 
-Inset * InsetGraphics::clone(Buffer const & buffer) const
+Inset * InsetGraphics::clone(Buffer const &) const
 {
-   return new InsetGraphics(*this, buffer.filePath());
+   return new InsetGraphics(*this);
 }
 
 
@@ -222,8 +223,7 @@ dispatch_result InsetGraphics::localDisp
InsetGraphicsParams p;
InsetGraphicsMailer::string2params(cmd.argument, p);
if (!p.filename.empty()) {
-   string const filepath = cmd.view()->buffer()->filePath();
-   setParams(p, filepath);
+   setParams(p);
cmd.view()->updateInset(this);
}
return DISPATCHED;
@@ -327,18 +327,6 @@ BufferView * InsetGraphics::view() const
 void InsetGraphics::draw(BufferView * bv, LyXFont const & font,
 int baseline, float & x) const
 {
-   // MakeAbsPath returns params().filename unchanged if it absolute
-   // already.
-   string const file_with_path =
-   MakeAbsPath(params().filename, bv->buffer()->filePath());
-
-   // A 'paste' operation creates a new inset with the correct filepath,
-   // but then the 'old' inset stored in the 'copy' operation is actually
-   // added to the buffer.
-   // Thus, we should ensure that the filepath is correct.
-   if (file_with_path != cache_->loader.filename())
-   cache_->update(file_with_path);
-
cache_->view = bv->owner()->view();
int oasc = cache_->old_ascent;
 
@@ -408,10 +396,10 @@ Inset::EDITABLE InsetGraphics::editable(
 }
 
 
-void InsetGraphics::write(Buffer const *, ostream & os) const
+void InsetGraphics::write(Buffer const * buf, ostream & os) const
 {
os << "Graphics\n";
-   params().Write(os);
+   params().Write(os, buf->filePath());
 }
 
 
@@ -420,15 +408,15 @@ void InsetGraphics::read(Buffer const * 
string const token = lex.getString();
 
if (token == "Graphics")
-   readInsetGraphics(lex);
+   readInsetGraphics(lex, buf->filePath());
else
lyxerr[Debug::GRAPHICS] << "Not a Graphics inset!\n";
 
-   cache_->update(MakeAbsPath(params().filename, buf->filePath()));
+   cache_->update(params().filename);
 }
 
 
-void InsetGraphics::readInsetGraphics(LyXLex & lex)
+void InsetGraphics::readInsetGraphics(LyXLex & lex, string const & bufpath)
 {
bool finished = false;
 
@@ -455,7 +443,7 @@ void InsetGraphics::readInsetGraphics(Ly
// TODO: Possibly open up a dialog?
}
else {
-   if (! params_.Read(lex, token))
+   if (!params_.Read(lex, token, bufpath))
lyxerr << "Unknown token, " << token << ", skipping."
<< std::endl;
}
@@ -518,17 +506,13 @@ string const InsetGraphics::prepareFile(
 {
// LaTeX can cope if the graphics file doesn't exist, so just return the
// filename.
-   string const orig_file = params().filename;
-   string orig_file_with_path =
-   MakeAbsPath(orig_file, buf->filePath());
-   lyxerr[Debug::GRAPHICS] << "[InsetGraphics::prepareFile] orig_file = "
-   << orig_file << "\n\twith path: "
-   << orig_file_with_path << endl;
+   string orig_file = params().filename;
+   string const rel_file = MakeRelPath(orig_file, buf->filePath());
 
-   if (!IsFileReadable(orig_file_with_path))
-   return orig_file;

graphics cache file path horkage

2003-05-30 Thread John Levon

Is there anyone who knows this code ? Angus ?

We seem to be storing relative paths in the InsetGraphics
cache (see cache_->update()). This is broken across e.g. file->save as.
It also causes significant problems in trying to fix the current
massive horkage in the tree.

I believe we should *always* store an absolute path at
runtime. At save time, we can convert it to a relative path.

Does this make sense ?

regards
john