Jean-Marc Lasgouttes wrote:
> A few remarks, in no particular order:
>
> - in case you did not notice already: win32_folder_path contains lots
> of unicode junk
I hadn't. Thanks. Now fixed.
> - it is a bit strange to have functions names set_foo() that do not
> set anything but actually return a value. Maybe get_foo() would be
> better?
Sure. Done.
> - in set_build_lyxdir, are you sure there is a real need for following
> symlinks? Since this is basically just a convenience feature for
> programmers, I think we can keep it simple. What examples of use did
> you have in mind?
$ cd $HOME/bin
$ ln -s $HOME/lyx/13x/build-xforms/src/lyx xlyx13x
$ cd
$ xlyx13x
That works at the moment for LyX 1.3.x but does not work for LyX 1.4.x.
I've always found it useful, so why not continue it?
> - for set_documentdir, as far as I know the answer is documentdir for
> everybody except WIN32. You can thus simplify the #if cascade.
Sure. I just wanted to make these #if cascades explicit for now.
> - in general, there is AFAIK no need for special casing EMX and Cygwin
> in directory selection, since they use a unix-like layout. The only
> thing to look for are file name conversions.
Thanks for that. I know nothing at all about OS/2 so any and all info will
be received most gratefully.
> - I am not sure that there is a need to catter for multiple
> directories in HOME. So I doubt that you should use GetEnvPath in
> set_homedir. What you could do however, is test for the existence of
> the directory and revert to some default when it does not exist (or
> abort). This would handle the case of multiple directories.
This all sounds like a good idea to me
string const homedir = os::internal_path(GetEnv("USERPROFILE"));
I'll add some test that homedir exists.
> - in set_tmpdir(), you can use GetTempPath for WIN32. See:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/fileio/base/gettemppath.asp
> For OSX I would suggest to use FSFindFolder:
http://developer.apple.com/documentation/Carbon/Reference/Folder_Manager/folder_manager_ref/function_group_2.html#//apple_ref/doc/uid/TP30000238/F16387
> with folder type kChewableItemsFolderType (first choice) or
> kTemporaryFolderType as a second choice. (I know nothing about that,
> I just searched the Apple site)
Magic, Jean-Marc!
> - in set_system_lyxdir() I think that the code should be refactored to
> that the steps 3 and 4 are made posix-only and the windows and osx
> code use simpler alternatives:
>
> For LyX/Aqua, use the code given here:
> http://doc.trolltech.com/3.3/mac-differences.html#7-1
>
> For LyX/Win, use the directory which holds the binary
Hmmm. Or perhaps add a directory Resources to the directory that holds the
binary, like the Mac?
> - in set_default_user_lyxdir(), I would use for OSX FSFinfFolder with
> type kPreferencesFolderType.
>
> - in extract_env_var_dir, I think using GetEnvPath is wrong, since we
> do not need to catter for the case of multiple paths. I guess
> GetEnv+os::internal_path is enough.
Good.
> - as for other cases, I do not think that
> possible_relative_localedir() is useful. The windows and OSX cases
> should be straightforward, and the more subtle case should be used
> with posix only.
Fair enough. I just wanted to make my prototype transparent.
> Finally, I would propose to change the conditionals in the following
> way: just assume that config.h defines one of the following:
> - USE_WIN_PACKAGING for LyX/Win (probably equivalent to _WIN32)
> - USE_OSX_PACKAGING for LyX/Mac
> - USE_POSIX_PACKAGING otherwise (linux, cygwin, darwin, emx)
>
> I think this will make the code much clearer.
Nice idea.
> Also, an advantage of avoiding the symlink following code on win32 is
> that it will avoid a case of using posix-only features (lstat) on a
> system that lacks them.
I don't think that's so important as the support code will remain anyway,
just will be protected with HAVE_LSTAT.
> I hope you find these remarks useful.
Of course. Many thanks for making such heroic efforts.
Joyeux Noël.
Angus
