Vincent van Ravesteijn wrote:
Yes, but the problem is that you CAN'T do this just by comparing
strings. In many cases, to be sure, you could tell that the file is
loaded by comparing strings. But you can't be sure it isn't loaded
just by comparing strings. Maybe it's loaded, but under a different
pathname.
I'd think that this could be circumvented pretty easily. When the
InsetInclude needs to load the file with the "user specified file
name" FileName::user_defined, it resolves the "normalized" name,
caches it as FileName::normalized and loads the file. Now the file is
in the BufferList under the normalized name (and we could make sure
that the normalized name is just everywhere internally). loadIfNeeded
would then only have to check whether the FileName::normalized is in
the BufferList (when it is not empty of course). This can be a simple
string comparison.
I pretty much think (and Georg sort of agreed) that this was the
design behind the FileName class, but things got messed up when using
qt-stuff as the real backbone.
OK, so one idea would be something like the attached. I haven't actually
tested this---no time at the moment---but surely something along these
lines works, and works as a general solution. The idea is to cache the
return value of QFileInfo::canonicalFilePath() in FileName itself, and
use it to do the comparisons.
One thing I'm not sure about is whether the weird cases previously
handled in operator== have been correctly transferred to the new
canonicalFilename() method. Btw, at present, we could just put that in
Private, since it's only used internally, but I can imagine wanting it.
That said, QFileInfo already caches this data. So why wasn't that good
enough? Why did Bennett have the problem that required the refresh()
calls that lead to the additional filesystem access? I think the answer
is that the first time the comparison was made, the file didn't exist,
and THAT fact got cached, so that the next time the comparison was made,
it returned false, even though the filenames were the same. (This is the
odd behavior of QFileInfo that Vincent ran into somewhere else.) So we
had to refresh. I think this case is dealt with properly here. If we get
nothing back from canonicalFilename(), then we just compare the paths.
Then again, if there were some other way to avoid the refresh() call,
that would also solve this problem, since then we can let Qt cache for
us. But I don't see one.
Richard
Index: src/support/FileName.cpp
===================================================================
--- src/support/FileName.cpp (revision 28882)
+++ src/support/FileName.cpp (working copy)
@@ -119,9 +119,16 @@
#else
fi = QFileInfo(fi.absoluteFilePath());
#endif
+ canonicalFilename_.clear();
}
+ void clone(Private rhs)
+ {
+ fi = rhs.fi;
+ canonicalFilename_ = rhs.canonicalFilename_;
+ }
+
static
bool isFilesystemEqual(QString const & lhs, QString const & rhs)
{
@@ -131,6 +138,8 @@
///
QFileInfo fi;
+ /// cache this, since it can be expensive to look it up
+ std::string canonicalFilename_;
};
/////////////////////////////////////////////////////////////////////
@@ -159,7 +168,7 @@
FileName::FileName(FileName const & rhs) : d(new Private)
{
- d->fi = rhs.d->fi;
+ d->clone(rhs);
}
@@ -171,7 +180,7 @@
FileName & FileName::operator=(FileName const & rhs)
{
- d->fi = rhs.d->fi;
+ d->clone(rhs);
return *this;
}
@@ -194,9 +203,30 @@
}
+string FileName::canonicalFilename() const
+{
+ if (!d->canonicalFilename_.empty())
+ return d->canonicalFilename_;
+
+ // FIXME: In future use Qt.
+ // Qt 4.4: We need to solve this warning from Qt documentation:
+ // * Long and short file names that refer to the same file on Windows are
+ // treated as if they referred to different files.
+ // This is supposed to be fixed for Qt5.
+ FileName const f(os::internal_path(absFilename()));
+
+ if (f.empty())
+ return d->canonicalFilename_; // still empty
+
+ d->canonicalFilename_ = fromqstr(f.d->fi.canonicalFilePath());
+ return d->canonicalFilename_;
+}
+
+
void FileName::set(string const & name)
{
d->fi.setFile(toqstr(name));
+ d->canonicalFilename_.clear();
}
@@ -206,12 +236,14 @@
d->fi.setFile(rhs.d->fi.filePath() + toqstr(suffix));
else
d->fi.setFile(QDir(rhs.d->fi.absoluteFilePath()), toqstr(suffix));
+ d->canonicalFilename_.clear();
}
void FileName::erase()
{
d->fi = QFileInfo();
+ d->canonicalFilename_.clear();
}
@@ -921,49 +953,13 @@
}
-// Note: According to Qt, QFileInfo::operator== is undefined when
-// both files do not exist (Qt4.5 gives true for all non-existent
-// files, while Qt4.4 compares the filenames).
-// see:
-// http://www.qtsoftware.com/developer/task-tracker/
-// index_html?id=248471&method=entry.
-bool operator==(FileName const & l, FileName const & r)
+bool operator==(FileName const & lhs, FileName const & rhs)
{
- // FIXME: In future use Qt.
- // Qt 4.4: We need to solve this warning from Qt documentation:
- // * Long and short file names that refer to the same file on Windows are
- // treated as if they referred to different files.
- // This is supposed to be fixed for Qt5.
- FileName const lhs(os::internal_path(l.absFilename()));
- FileName const rhs(os::internal_path(r.absFilename()));
-
- if (lhs.empty())
- // QFileInfo::operator==() returns false if the two QFileInfo are empty.
- return rhs.empty();
-
- if (rhs.empty())
- // Avoid unnecessary checks below.
- return false;
-
- lhs.d->refresh();
- rhs.d->refresh();
-
- if (!lhs.d->fi.isSymLink() && !rhs.d->fi.isSymLink()) {
- // Qt already checks if the filesystem is case sensitive or not.
- // see note above why the extra check with fileName is needed.
- return lhs.d->fi == rhs.d->fi
- && lhs.d->fi.fileName() == rhs.d->fi.fileName();
- }
-
- // FIXME: When/if QFileInfo support symlink comparison, remove this code.
- QFileInfo fi1(lhs.d->fi);
- if (fi1.isSymLink())
- fi1 = QFileInfo(fi1.symLinkTarget());
- QFileInfo fi2(rhs.d->fi);
- if (fi2.isSymLink())
- fi2 = QFileInfo(fi2.symLinkTarget());
- // see note above why the extra check with fileName is needed.
- return fi1 == fi2 && fi1.fileName() == fi2.fileName();
+ string lhsName = lhs.canonicalFilename();
+ string rhsName = rhs.canonicalFilename();
+ if (lhsName.empty() && rhsName.empty())
+ return lhs.absFilename() == rhs.absFilename();
+ return lhsName == rhsName;
}
Index: src/support/FileName.h
===================================================================
--- src/support/FileName.h (revision 28882)
+++ src/support/FileName.h (working copy)
@@ -64,6 +64,10 @@
/// get the absolute file name in UTF-8 encoding
std::string absFilename() const;
+ /// get the "normalized" filename, i.e., with symlinks etc resolved
+ /// returns empty if there is an error, or if the file does not exist
+ std::string canonicalFilename() const;
+
/**
* Get the file name in the encoding used by the file system.
* Only use this for accessing the file, e.g. with an fstream.
