Lars Gullik Bjønnes wrote:
> Georg Baum <[EMAIL PROTECTED]> writes:
> | - Exit lyx from the buffer constructor if the temp dir is invalid. I
> | tested it (using a static counter that triggered the test in the third
> | buffer), and unsaved changes got written to .emergency files.
> | I guess that doing this "controlled crash" is safe: The new buffer is
> | completely constructed, but empty, so the emergency write will not harm.
>
> I don't agree with this one. Sure we should do a _controlled_ exit if
> we are unable to create the tempdir, And we should notify the user if
> we are unable to create the buffertmpdir, but we should not just exit.
The user is notified.
> If the only way to do this properly is to use exceptions then let's do
> just that. And have the no-exception case exit with a lousy user
> message.
Of course I am not the one to decide this. And I don't want to put too much
work into this, since the primary goal is to make the "include material"
insets working in all cases, and removing the no temp dir case is only
something that makes this easier.
May I suggest to just ignore the case that createBufferTmpDir() could fail
for now and only put in a FIXME? IMHO this does not make it worse compared
to the present state: Right now, a failed createBufferTmpDir() means
implicitly "use no temp dir" at some places and is ignored at others.
> I'll also move the creation of the buffertmpdir out of the buffer
> constructor if that helps. (and tempdir/filesystem stuff does not
> really have anything to do with Buffer anyway...)
Then I wonder why there is a per buffer temp dir at all? This problem would
vanish if we only had the global temp dir.
> | string const fname = AddName(path,
> | OnlyFilename(ChangeExtension(filename,
> | @@ -596,6 +625,15 @@ bool Buffer::readFile(LyXLex & lex, stri
> | filename));
> | } else if (file_format < LYX_FORMAT) {
> | string const tmpfile = tempName();
> | + if (tmpfile.empty()) {
> | + Alert::error(_("Conversion failed"),
> | + bformat(_("%1$s is from an earlier"
> | + " version of LyX, but a temporary"
> | + " file for converting it could"
> | + " not be created."),
> | + filename));
> | + return false;
> | + }
>
> I am not sure who to warn best for this, but this is an option.
I modeled this after the warnigs that are generated in case the conversion
script was not found or failed. I think that this is the same category.
> | Index: src/lyxrc.C
> | ===================================================================
> | RCS file: /cvs/lyx/lyx-devel/src/lyxrc.C,v
> | retrieving revision 1.162
> | diff -u -p -r1.162 lyxrc.C
> | --- src/lyxrc.C 2003/12/14 16:33:53 1.162
> | +++ src/lyxrc.C 2004/02/21 17:17:41
> | @@ -657,7 +658,8 @@ int LyXRC::read(string const & filename)
> |
> | case RC_USETEMPDIR:
> | if (lexrc.next()) {
> | - use_tempdir = lexrc.getBool();
> | + lyxerr << "Ignoring obsolete use_tempdir flag." <<
> endl;
> | + lexrc.getBool();
>
> I do not think that the lexrc.getBool() is needed, the if
> (lexrc.next()) takes care of eating the token.
Ok, I don't know anything about LyXLex.
> | Index: src/lyxvc.C
> | ===================================================================
> | RCS file: /cvs/lyx/lyx-devel/src/lyxvc.C,v
> | retrieving revision 1.54
> | diff -u -p -r1.54 lyxvc.C
> | --- src/lyxvc.C 2003/11/03 17:47:22 1.54
> | +++ src/lyxvc.C 2004/02/21 17:17:52
> | @@ -223,6 +223,11 @@ string const LyXVC::getLogFile() const
> | return string();
> |
> | string tmpf = tempName(string(), "lyxvclog");
> | + if (tmpf.empty()) {
> | + lyxerr[Debug::LYXVC] << "Could not generate logfile "
> | + << tmpf << endl;
> | + return string();
> | + }
> | lyxerr[Debug::LYXVC] << "Generating logfile " << tmpf << endl;
> | vcs->getLog(tmpf);
> | return tmpf;
>
> Have all callers been updated to handle empty string?
> (or did they already do it?)
I assume they can handle this, since it did already return string() if
(!vcs).
> | -std::string const CreateLyXTmpDir(std::string const & deflt);
> | +/** Creates the global LyX temp dir.
> | + \p deflt can be an existing directory name. In this case a new
> | directory
> | + inside \p deflt is created. If \p deflt does not exist yet, \p deflt
> | is
> | + created and used as the temporary directory.
> | + \return the tmp dir name or string() if something went wrong.
> | + */
>
> This is wrong use of doxygen tags.
I don't think so. From
http://www.stack.nl/~dimitri/doxygen/commands.html#cmdp:
"\p <word>
Displays the parameter <word> using a typewriter font. You can use this
command to refer to member function parameters in the running text."
The doxygen output (version 1.3.4) looks ok.
Georg
