Georg Baum <[EMAIL PROTECTED]> writes:
Let me begin with saying that I too really really want support for
no-tempdir gone, and have us use a tempdir always.
| I have played with the html export, and it works without tmpdir if the
| "originaldir" flag is interpreted as "Do a 'nice' export and run everything
| in the original dir". This simulates the "use no temp dir" case just for
| this conversion.
good.
| Changes to the first version of the patch:
| - doxygen fix in support/filename.h
| - Warn when reading the now obsolete use_tempdir flag
ok
| - 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.
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.
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...)
| + // Exit if we have no temp dir.
| + // Since the buffer temp dir is created in the global lyx temp dir,
| + // and that was valid at lyx startup, chances that this happens are
| + // minimal, and trying to work around is not worth the effort.
I do not agree with the comment.
| + if (temppath().empty()) {
| + Alert::error(_("Could not create temporary directory"),
| + _("Could not create a temporary directory."
| + " LyX will exit now."));
| + // Try to save changed files
| + LyX::cref().emergencyCleanup();
| + exit(EXIT_FAILURE);
| + }
| }
so: no.
| @@ -317,11 +329,8 @@ pair<Buffer::LogType, string> const Buff
|
| if (filename.empty())
| return make_pair(Buffer::latexlog, string());
| -
| - string path = OnlyPath(filename);
|
| - if (lyxrc.use_tempdir || !IsDirWriteable(path))
| - path = temppath();
| + string const path = temppath();
right.
|
| 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.
| Index: src/lyx_main.C
| ===================================================================
| RCS file: /cvs/lyx/lyx-devel/src/lyx_main.C,v
| retrieving revision 1.182
| diff -u -p -r1.182 lyx_main.C
| --- src/lyx_main.C 2003/12/12 13:57:19 1.182
| +++ src/lyx_main.C 2004/02/21 17:16:50
| @@ -58,7 +58,7 @@ using lyx::support::AddName;
| using lyx::support::AddPath;
| using lyx::support::bformat;
| using lyx::support::createDirectory;
| -using lyx::support::CreateLyXTmpDir;
| +using lyx::support::createLyXTmpDir;
| using lyx::support::FileInfo;
| using lyx::support::FileSearch;
| using lyx::support::GetEnv;
| @@ -390,7 +390,21 @@ void LyX::init(bool gui)
| if (lyxerr.debugging(Debug::LYXRC))
| lyxrc.print();
|
| - os::setTmpDir(CreateLyXTmpDir(lyxrc.tempdir_path));
| + os::setTmpDir(createLyXTmpDir(lyxrc.tempdir_path));
| + if (os::getTmpDir().empty()) {
| + Alert::error(_("Could not create temporary directory"),
| + bformat(_("Could not create a temporary directory in\n"
| + "%1$s. Make sure that this\n"
| + "path exists and is writable and try again."),
| + lyxrc.tempdir_path));
| + // createLyXTmpDir() tries sufficiently hard to create a
| + // usable temp dir, so the probability to come here is
| + // close to zero. We therefore don't try to overcome this
| + // problem with e.g. asking the user for a new path and
| + // trying again but simply exit.
| + exit(EXIT_FAILURE);
| + }
This exit I am happy with.
| 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.
| 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?)
| -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.
| -/// \return the name that LyX will give to the unzipped file.
| +/** \return the name that LyX will give to the unzipped file \p zipped_file
| + if the second argument of unzipFile() is empty.
| + */
here as well.
| std::string const unzippedFileName(std::string const & zipped_file);
|
| -/// unzip a file
| -std::string const unzipFile(std::string const & zipped_file);
| +/** Unzip \p zipped_file.
| + The unzipped file is named \p unzipped_file if \p unzipped_file is not
| + empty, and unzippedFileName(\p zipped_file) otherwise.
| + Will overwrite an already existing unzipped file without warning.
| + */
And here...
\p does not do what you think it does.
--
Lgb
