Georg Baum wrote:
Abdelrazak Younes wrote:
The problem was not in reading the rc but when reading the Qt edit
boxes. This patch fixes that plus a few more. It also convert
internal_path() to unicode:
Please don't do that, see below for the reason.
docstring internal_path(docstring const & p)
{
// FIXME UNICODE
return from_utf8(subst(get_long_path(to_utf8(p)), "\\", "/"));
}
Am I right here that this should solve problems with special characters
in filenames used within the GUI?
No, that will not work. GetLongPathName expects and returns the path in file
system encoding, so you should modify get_long_path() to convert from/to
utf8 (and probably audit all calls of internal_path for correct encoding).
I know that and the solution is to use GetLongPathNameW. But as I don't
know where we're going with filenames, I didn't do the extra work.
That's the reason why there is a FIXME.
By the way, most of lyxrc is still using std::string for paths...
This is on purpose. If you look at filetools.h you will see that all file
names are either passed as FileName or std::string. FileName is used if the
file is supposed to exist, and either the function or the caller will
access it, and std::string is used if nothing is done with the file itself,
but rather some operations with the name (e.g. add an extension).
I am strongly against changing some file utility functions to docstring and
others not,
Fine, I will just do the conversion in qt_helpers then.
and I am also against introducing "convenience" functions like
internal_path() taking a QString. If we start to do this the number of file
realted functions will explode (and we have too many already).
The moment one utility function is used in more than one class, it
should be shared. That's the case with "internal_path()". I don't like
at all these fromqstr/toqstr/etc.
We have three type of string internally, string, docstring and QString;
it's just fair to have helper methods for QString also. And the source
code looks much nicer this way.
I did not
convert all the std::string file names to docstring for two reasons:
- We use several of these functions heavily in bibtex manipulation, and that
cannot easily be changed to docstring (regex stuff)
- We should clean up the file related stuff anyway, and use more
boost::filesystem or even use QFile internally as Andre suggested, so this
work would be partially wasted.
I know that and that's why I limited the patch to internal_path() only
as this is used heavily in the frontend code.
- We should use an external bibtex parser and get rid of our own, then a
conversionn to docstrign will be easy
For now the std::string versions should be enough.
I just see that you committed your patch, please revert it unless you have
some convincing arguments why internal_path should be docstring and other
file related functions not.
OK, I will revert the os.h part.
Abdel.
