Hi all, I am trying to implement the Browse button in the winecfg application, that would allow people to browse for a directory to use as their virtual c drive. In order to do so, I though I'd use the SHBrowseForFolder function from shlobj.h (see http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/pla...). However, when I tried this in Wine, I get a compiler warning: In driveui.c I did: case IDC_BUTTON_BROWSE_PATH: SHBrowseForFolder(0); //MessageBox(dialog, "", "Write me!", MB_OK); break; Then, at compile time I get: driveui.c:589: warning: undefined reference to `SHBrowseForFolderA' I made sure that there is a line that says: #include <shlobj.h> When I try to launch the app, I get ./winecfg wine: could not load L"Z:\\home\\robert\\Documents\\devel\\wine\\programs\\winecfg\\winecfg.exe.so": /home/robert/.wine/dosdevices/z:/home/robert/Documents/devel/wine/programs/winecfg/winecfg.exe.so: undefined symbol: SHBrowseForFolderA Does this mean that the SHBrowseForFolder thing is not implemented? Probably it is, because I checked Wine's shlobj.h. What am I doing wrong? Thanks, Robert
On Tue, 23 Nov 2004 17:24:28 +0100, Robert van Herk wrote:
I am trying to implement the Browse button in the winecfg application, that would allow people to browse for a directory to use as their virtual c drive.
Cool! Go for it!
Then, at compile time I get: driveui.c:589: warning: undefined reference to `SHBrowseForFolderA'
At the bottom of the MSDN page you can see it says you need shell32.dll, so try adding -lshell32 to the link line in the Makefile.in (remember to rerun config.status) and try again. Here are some more winecfg todos for everybody to take a crack at now my last patch got checked in: * The drive detection algorithm needs some polishing. It mapped my /boot directory on FC3 even though that's useless. Maybe we need a blacklist? [easy] * We should also ensure the home directory is mapped in the same way that we ensure / and c:\windows is mapped. [easy] * Integrate drive detection with wineprefixcreate. The code nearly supports this and was designed with that in mind, check out the different ways errors are presented from the CLI and GUI. You just need to add a command line switch to the program that runs the autodetect_drives() function. [not too hard] * There's some weird clipping/painting corruption in the advanced view of the drives page. It looks like a WM rewrite regression so I asked Alexandre to check it out but if you're up for a challenge feel free to try and beat him to it! [marine level hardness!] * Unicodify it, at the moment it uses some ANSI APIs and such [intermediate] * Usability review [intermediate] .... and of course, the big cheese .... * Write the migration code to pull the users current config across to the new registry branch so we can actually start thinking about switching the config file off once and for all. Bonus points for adding an explanation of what happened to the old config file itself! [easy in theory, might be hard getting it good enough to get past Alexandre!] thanks -mike
Hi all, Thanks to Mike's help, I succeeded in showing a SHBrowseForFolder thingy, in the winecfg program, so that people can pick a directory they want to use as virtual C drive. However, ofcourse, this directory choosing thingy shows the virtual Windows file system, not the Unix file system. Hence, I think we have a tiny bootstrapping problem here. If I just show the user his windows file system, he might not be able to pick the directory he wants. Is Z:\ always mapped to the root of the unix file system? Because that would seem to solve it: just let them choose a directory from Z:\ and then probably modify the returned string by removing Z: and replacing all /'s bij \'s. Greetings, Robert
Hi, On Wed, Nov 24, 2004 at 09:58:14AM +0100, Robert van Herk wrote:
Hi all,
Thanks to Mike's help, I succeeded in showing a SHBrowseForFolder thingy, in the winecfg program, so that people can pick a directory they want to use as virtual C drive.
However, ofcourse, this directory choosing thingy shows the virtual Windows file system, not the Unix file system. Hence, I think we have a tiny bootstrapping problem here. If I just show the user his windows file system, he might not be able to pick the directory he wants. I should have told you that before, since I've realized that yesterday already.
Is Z:\ always mapped to the root of the unix file system? Because that would seem to solve it: just let them choose a directory from Z:\ and then probably modify the returned string by removing Z: and replacing all /'s bij \'s. Hopefully it isn't: you may not always want to expose the full directory tree to dear little John Rogue Windows program, not even in (by default) mostly read-only access...
IOW, there should be a mechanism which allows to establish a temporary root mapping during winecfg configuration only. However this would require some explanation for the user, since it's not entirely obvious why a drive mapping that existed during configuration won't exist later on. But OTOH this shouldn't be a problem anyway if you clearly list all defined mappings in an obvious list and only have that temporary mapping available during the browser dialog. Andreas Mohr
Hi,
Is Z:\ always mapped to the root of the unix file system? Because that would seem to solve it: just let them choose a directory from Z:\ and then probably modify the returned string by removing Z: and replacing all /'s bij \'s.
Hopefully it isn't: you may not always want to expose the full directory tree to dear little John Rogue Windows program, not even in (by default) mostly read-only access...
IOW, there should be a mechanism which allows to establish a temporary root mapping during winecfg configuration only. However this would require some explanation for the user, since it's not entirely obvious why a drive mapping that existed during configuration won't exist later on.
I don't know how virtual windows drives are mounted at this moment, but I guess it is something that the winecfg applet cannot do... Is it possible (as in: not too dangerous) to make a special API that does not exist in Windows, but then does exist in Wine, that would allow a program to mount the full Unix tree read only, such that only Wine programs can do that? Because I really need to get the unix directory tree in one way or the other. Also, it would probably be nice to show the paths unix style here, with forward slashes and / as the root, so that the user is aware that he is picking a Unix directory instead of a virtual windows one. Ofcourse, another solution would be to make a Unix server that communicates with winecfg and tells it how the Unix directory structure is, but that would be overkill I guess :-). Greetings, Robert
Hi, On Wed, Nov 24, 2004 at 12:45:20PM +0100, Robert van Herk wrote:
Hi,
Is Z:\ always mapped to the root of the unix file system? Because that would seem to solve it: just let them choose a directory from Z:\ and then probably modify the returned string by removing Z: and replacing all /'s bij \'s.
Hopefully it isn't: you may not always want to expose the full directory tree to dear little John Rogue Windows program, not even in (by default) mostly read-only access...
IOW, there should be a mechanism which allows to establish a temporary root mapping during winecfg configuration only. However this would require some explanation for the user, since it's not entirely obvious why a drive mapping that existed during configuration won't exist later on.
I don't know how virtual windows drives are mounted at this moment, but I guess it is something that the winecfg applet cannot do... Sounds difficult, yes.
Is it possible (as in: not too dangerous) to make a special API that does not exist in Windows, but then does exist in Wine, that would allow a program to mount the full Unix tree read only, such that only Wine programs can do that? I don't think that's a clean solution.
Since NT-based systems (or was it XP only?) can mount volumes as a sub directory even after bootup, there could be an already existing device ioctl allowing for a new drive mapping in Windows. We'd then just have to implement this in Wine and call it from winecfg.
Because I really need to get the unix directory tree in one way or the other. Also, it would probably be nice to show the paths unix style here, with forward slashes and / as the root, so that the user is aware that he is picking a Unix directory instead of a virtual windows one.
Ofcourse, another solution would be to make a Unix server that communicates with winecfg and tells it how the Unix directory structure is, but that would be overkill I guess :-). Nah, that seems to be overkill.
If you need that, then we could perhaps even think about embedding standard Gnome/GTK/KDE file browsers in winecfg for this purpose in order to have full root filesystem accessibility. Andreas Mohr
Hi,
Is it possible (as in: not too dangerous) to make a special API that does not exist in Windows, but then does exist in Wine, that would allow a program to mount the full Unix tree read only, such that only Wine programs can do that?
I don't think that's a clean solution.
Since NT-based systems (or was it XP only?) can mount volumes as a sub directory even after bootup, there could be an already existing device ioctl allowing for a new drive mapping in Windows. We'd then just have to implement this in Wine and call it from winecfg.
On the MSDN site I found http://msdn.microsoft.com/library/default.asp?url=/library/en-us/fileio/base..., which is an example that indeed seems to do this. However, mounting in Windows is just connecting a device (e.g. F:) to a directory (e.g. C:\FDrive). What they do is: bFlag = GetVolumeNameForVolumeMountPoint( argv[2], // input volume mount point or directory Buf, // output volume name buffer BUFSIZE // size of volume name buffer ); That gives them the unique name of that drive, and they can then happily mount it somewhere else. So, I guess we'd still need the Unix file system as a drive (or at least it's unique volume name). Further more, if winecfg can use this volume name in order to mount it somewhere, any Windows app could, so still there would be no extra safety. In fact, the only ways I can think of to make this feature available just for Wine programs, is - Calling the Unix api directly from winecfg (I guess that is possible?) just to read out the directory contents and then put them into a list box. This requires copying the code of the SHBrowseForFolder listbox, which is a bit untidy too, since the Windows file system is then not used. - Indeed by making some extra APIs that are not available in Windows. This is indeed a bit untidy, but probably requires less code duplication. Regards, Robert
Hi, On Wed, Nov 24, 2004 at 02:03:16PM +0100, Robert van Herk wrote:
In fact, the only ways I can think of to make this feature available just for Wine programs, is - Calling the Unix api directly from winecfg (I guess that is possible?) just to read out the directory contents and then put them into a list box. This requires copying the code of the SHBrowseForFolder listbox, which is a bit untidy too, since the Windows file system is then not used. - Indeed by making some extra APIs that are not available in Windows. This is indeed a bit untidy, but probably requires less code duplication. Indeed, this is the best bet, actually even much better than embedding non-Wine file browser dialogs. Just make a nice custom dialog which displays directory structures read via direct Unix calls, that should be the way to go. (perhaps starting with the home directory of the user here is better, but more likely it's not).
Andreas Mohr
On Wed, 24 Nov 2004, Andreas Mohr wrote:
On Wed, Nov 24, 2004 at 12:45:20PM +0100, Robert van Herk wrote:
Ofcourse, another solution would be to make a Unix server that communicates with winecfg and tells it how the Unix directory structure is, but that would be overkill I guess :-).
Nah, that seems to be overkill.
But why? If the 'server' gets launched using CreateProcess() and if the communication is done via writing to child's stdin / reading from child's stdout, then the things get quite simple, IMHO. Isn't this redirection working in Wine? Or is additional application in Wine tree an overkill?
If you need that, then we could perhaps even think about embedding standard Gnome/GTK/KDE file browsers in winecfg for this purpose in order to have full root filesystem accessibility.
I like this idea quite much. Though, GTK file browser embedded into a native unix app would be even more clean solution, IMHO. That's because this way we would keep winecfg untied from a unix world in a bigger degree. :-P
On Wed, 24 Nov 2004 09:58:14 +0100, Robert van Herk wrote:
Thanks to Mike's help, I succeeded in showing a SHBrowseForFolder thingy, in the winecfg program, so that people can pick a directory they want to use as virtual C drive.
Cool.
However, ofcourse, this directory choosing thingy shows the virtual Windows file system, not the Unix file system. Hence, I think we have a tiny bootstrapping problem here. If I just show the user his windows file system, he might not be able to pick the directory he wants.
Yes, this was expected. Just ensure only Z: is mapped during the directory browser operation. See the code in drive.c to find out how to modify drive mappings using win32 but to be honest the easiest way would be to make a copy of the current configuration data, modify it so only Z: is mapped, run the picker and then restore the config afterwards with the new drive the user selected mapped. Wine *should* be able to respond to drive remappings on the fly like this. If it can't we need to fix that first.
Is Z:\ always mapped to the root of the unix file system? Because that would seem to solve it: just let them choose a directory from Z:\ and then probably modify the returned string by removing Z: and replacing all /'s bij \'s.
No not always, it's not enforced by Wine. It could be any drive. Just back up the current config, ensure only Z: is mapped to / and then run the picker. You may need to adjust the winecfg drive management API a bit. We then have the UI issue of this random Z thing in the directory browser but we can introduce a magic Wine flag specifically for winecfg to improve usability here. That can be dealt with later. Have a go at writing the patch, post it here, and we'll go from there. thanks -mike
Is Z:\ always mapped to the root of the unix file system? Because that would seem to solve it: just let them choose a directory from Z:\ and then probably modify the returned string by removing Z: and replacing all /'s bij \'s.
No not always, it's not enforced by Wine. It could be any drive.
Just back up the current config, ensure only Z: is mapped to / and then run the picker. You may need to adjust the winecfg drive management API a bit.
We then have the UI issue of this random Z thing in the directory browser but we can introduce a magic Wine flag specifically for winecfg to improve usability here. That can be dealt with later.
Also, we'd have the problem that when other processes are active and using the disks, these disks will be unmounted :-). Therefore, I'd say that using this approach, it is probably nicer to have a magic wine flag that let the SHBrowseForFolder thingy show the unix directory listing, then instead first unmount all drives and then remount just Z: on the / directory. Unmounting, replacing and restoring the entire windows file system seems a bit ugly to me... Also, I think Andreas' approach isn't too bad, namely copying code from the SHBrowseForFolder thing, but then let it show the Unix tree using native system calls. Still though, I am not really fond of the idea that I would have to copy all ui code from SHBrowseForFolder, just because I want to use the Unix file system instead... Oh, by the way, the SHBrowseForFolder thingy allow to specifiy a root folder: * pidlRoot* Pointer to an item identifier list (PIDL) specifying the location of the root folder from which to start browsing. Only the specified folder and any subfolders that are beneath it in the namespace hierarchy will appear in the dialog box. This member can be NULL; in that case, the namespace root (the desktop folder) is used. So I guess all I'm needing is a temporary folder to mount the stuff in and then make that the root... Anyways: the choice is still: should I reimplement SHBrowseForFolder, but then for Unix file system, or should I mount the stuff in a Windows directory and then use that as pidlRoot for the normal SHBrowseForFolder? Robert
"Robert van Herk" <robert(a)robertvanherk.nl> wrote:
Therefore, I'd say that using this approach, it is probably nicer to have a magic wine flag that let the SHBrowseForFolder thingy show the unix directory listing
I wonder whether using a callback in SHBrowseForFolder and reinitializing dialog with unix directory tree is possible. -- Dmitry.
On Wed, 2004-11-24 at 15:53 +0100, Robert van Herk wrote:
Also, we'd have the problem that when other processes are active and using the disks, these disks will be unmounted :-).
Good point. I was assuming that if you're reconfiguring drives in winecfg while other apps are running you'd be aware that may happen but I suppose we should not assume that.
Therefore, I'd say that using this approach, it is probably nicer to have a magic wine flag that let the SHBrowseForFolder thingy show the unix directory listing, then instead first unmount all drives and then remount just Z: on the / directory. Unmounting, replacing and restoring the entire windows file system seems a bit ugly to me...
Yes that'd work as well. I'd rather have a magic winecfg flag than copy/paste the code as that way we get bugfixes in shell32 automatically. But Alexandre tends to prefer copy/paste over hacking code around or using static libraries, so I'm not sure which way he'd prefer here.
So I guess all I'm needing is a temporary folder to mount the stuff in and then make that the root...
That'd work except that GetVolumeNameForVolumeMountPoint is a stub in Wine. I do not know how hard it'd be to implement. Eric?
Anyways: the choice is still: should I reimplement SHBrowseForFolder, but then for Unix file system, or should I mount the stuff in a Windows directory and then use that as pidlRoot for the normal SHBrowseForFolder?
Right now, it'd be easiest to add a winecfg mode to SHBrowseForFolder. But it's Alexandres call. thanks -mike
Yes that'd work as well. I'd rather have a magic winecfg flag than copy/paste the code as that way we get bugfixes in shell32 automatically. But Alexandre tends to prefer copy/paste over hacking code around or using static libraries, so I'm not sure which way he'd prefer here.
Anyways: the choice is still: should I reimplement SHBrowseForFolder, but then for Unix file system, or should I mount the stuff in a Windows directory and then use that as pidlRoot for the normal SHBrowseForFolder?
Right now, it'd be easiest to add a winecfg mode to SHBrowseForFolder. But it's Alexandres call.
OK, I guess I'll just wait for his answer on this then. Also, I am having some troubles into tricking the SHBrowseForFolder thingy into using a different root folder, since I guess I need to use SHParseDisplayName (http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/pla...) for that, but that doesn't seem to be implemented in Wine. Finally, I don't know how to free PIDLS. In the MSDN documentation they use C++ calls for that, but I don't know how to port that to C. Well, anyways, now I understand why you didn't do the directory browser yet :-D. Cheers, Robert
Mike Hearn <mike(a)navi.cx> writes:
Yes, this was expected. Just ensure only Z: is mapped during the directory browser operation. See the code in drive.c to find out how to modify drive mappings using win32 but to be honest the easiest way would be to make a copy of the current configuration data, modify it so only Z: is mapped, run the picker and then restore the config afterwards with the new drive the user selected mapped.
You don't want to do that, there may be other apps running; messing with the current drive config is definitely not an option. We need to somehow extend the Windows file dialogs to allow browsing the Unix file system, this is something many Winelib apps will want too. -- Alexandre Julliard julliard(a)winehq.org
Alexandre Julliard wrote:
Mike Hearn <mike(a)navi.cx> writes:
Yes, this was expected. Just ensure only Z: is mapped during the directory browser operation. See the code in drive.c to find out how to modify drive mappings using win32 but to be honest the easiest way would be to make a copy of the current configuration data, modify it so only Z: is mapped, run the picker and then restore the config afterwards with the new drive the user selected mapped.
You don't want to do that, there may be other apps running; messing with the current drive config is definitely not an option. We need to somehow extend the Windows file dialogs to allow browsing the Unix file system, this is something many Winelib apps will want too.
Jippy! That was my fav. way to go! :-) Robert
Alexandre Julliard wrote:
Mike Hearn <mike(a)navi.cx> writes:
Yes, this was expected. Just ensure only Z: is mapped during the directory browser operation. See the code in drive.c to find out how to modify drive mappings using win32 but to be honest the easiest way would be to make a copy of the current configuration data, modify it so only Z: is mapped, run the picker and then restore the config afterwards with the new drive the user selected mapped.
You don't want to do that, there may be other apps running; messing with the current drive config is definitely not an option. We need to somehow extend the Windows file dialogs to allow browsing the Unix file system, this is something many Winelib apps will want too.
OK, so how to do this? I see 3 ways: 1. Implement an extra api that do the same as the normal file open/save and browse for folder api, but then with the Unix file system 2. Implement a thread local variable so that threads can put the Wine file dialogs into unix file system mode / windows file system mode 3. Define some magic wine flags in the file dialogs that make the dialogs appear in Unix mode There are some pro's and con's to all of them: 1. Well, basically, we have to make an extra api :-), so there are more functions exported. Probably the existing and new functions will need to call each other, so this means an extra level of indirection. 2. Kinda ugly hacking, though we don't loose win32 compatibility 3. Nice, but we loose strict win32 compatibility, since there will be a magic flags that doesn't exist in win32. Robert
On Thu, 25 Nov 2004 14:42:24 +0100, Robert van Herk wrote:
3. Nice, but we loose strict win32 compatibility, since there will be a magic flags that doesn't exist in win32.
This is the most lightweight so I'd go for it for now. If we find that the flag value we pick is already in use we can just change it, no big deal. We already use custom Wine flags in other parts of the code (eg WS_EX_TRAYWINDOW). thanks -mike
Mike Hearn wrote:
On Thu, 25 Nov 2004 14:42:24 +0100, Robert van Herk wrote:
3. Nice, but we loose strict win32 compatibility, since there will be a magic flags that doesn't exist in win32.
This is the most lightweight so I'd go for it for now. If we find that the flag value we pick is already in use we can just change it, no big deal. We already use custom Wine flags in other parts of the code (eg WS_EX_TRAYWINDOW).
Internally though, the SHBrowseForFolder dialog (dlls/shell32/brsfolder.c) uses pidls, which, as far as I know, are a specific win32 thing for identifying paths uniqually. Thus, if I'd make an extra flag in the dialog, I would have to extent pidls to, so that they can hold unix paths. Is that a wise thing to do? Or should I take another approach? Robert
On Sat, 27 Nov 2004 10:37:10 +0100, Robert van Herk wrote:
Thus, if I'd make an extra flag in the dialog, I would have to extent pidls to, so that they can hold unix paths. Is that a wise thing to do? Or should I take another approach?
Hmm, I'm not sure you want to do that. You may wish to abstract that out, eg replace the parts where PIDLs are passed around with a struct path { PIDL pidl; char *path; } then use whichever is non-NULL. Extending PIDLs is possible but probably would get a bit icky.
Mike Hearn wrote:
On Sat, 27 Nov 2004 10:37:10 +0100, Robert van Herk wrote:
Thus, if I'd make an extra flag in the dialog, I would have to extent pidls to, so that they can hold unix paths. Is that a wise thing to do? Or should I take another approach?
Hmm, I'm not sure you want to do that. You may wish to abstract that out, eg replace the parts where PIDLs are passed around with a
struct path { PIDL pidl; char *path; }
Ofcourse, but the api call SHBrowseForFolder returns a pidl. So making unix browsing an extra flag wouldn't be useful, if I couldn't return a valid pidl in the unix case. Hmm, I guess perhaps it is best to make an extra api function for unix browsing anyway. Robert
On Sun, 28 Nov 2004 17:02:05 +0100, Robert van Herk wrote:
Ofcourse, but the api call SHBrowseForFolder returns a pidl. So making unix browsing an extra flag wouldn't be useful, if I couldn't return a valid pidl in the unix case.
Hmm, I guess perhaps it is best to make an extra api function for unix browsing anyway.
Ah OK, I didn't realise that. You could always just cast the result, a PIDL is a pointer at the end of the day, but I agree that's rather ugly. A new Wine API seems like the best route.
Mike Hearn wrote:
On Sun, 28 Nov 2004 17:02:05 +0100, Robert van Herk wrote:
Ofcourse, but the api call SHBrowseForFolder returns a pidl. So making unix browsing an extra flag wouldn't be useful, if I couldn't return a valid pidl in the unix case.
Hmm, I guess perhaps it is best to make an extra api function for unix browsing anyway.
Ah OK, I didn't realise that. You could always just cast the result, a PIDL is a pointer at the end of the day, but I agree that's rather ugly. A new Wine API seems like the best route.
OK! Currently I am trying to add some functionality for this in brsfolder.c. However, I ran into a (probably small) problem: I added a new function SHBrowseForUNIXDirectory in brsfolder.c and added it to shlobj.h. Reinstalled the shlobj.h, compiled the shell32.dll, and installed that new one too. Also runned ldconfig (don't know if that is a must). Though, when I try to compile winecfg (with the SHBrowseForUNIXDirectory call) I get: make ../../tools/winegcc/winegcc -B../../tools/winebuild -mwindows appdefaults.o audio.o drive.o drivedetect.o driveui.o libraries.o main.o properties.o winecfg.o x11drvdlg.o winecfg.exe.dbg.o winecfg.res -o winecfg.exe.so -L../../dlls -lcomdlg32 -lcomctl32 -luser32 -ladvapi32 -lkernel32 -lshell32 -L../../libs/wine -lwine -L../../libs/port -lwine_port /usr/lib/gcc-lib/i586-suse-linux/3.3.3/../../../../i586-suse-linux/bin/ld: cannot find -lshell32 collect2: ld returned 1 exit status winegcc: gcc failed. make: *** [winecfg.exe.so] Error 2 I haved checked that the new version is actually in /usr/local/lib/wine/shell32.dll.so. What am I doing wrong? Robert
Never mind, doing a new make from the root directory of the wine source solved it... Robert Robert van Herk wrote:
Mike Hearn wrote:
On Sun, 28 Nov 2004 17:02:05 +0100, Robert van Herk wrote:
Ofcourse, but the api call SHBrowseForFolder returns a pidl. So making unix browsing an extra flag wouldn't be useful, if I couldn't return a valid pidl in the unix case.
Hmm, I guess perhaps it is best to make an extra api function for unix browsing anyway.
Ah OK, I didn't realise that. You could always just cast the result, a PIDL is a pointer at the end of the day, but I agree that's rather ugly. A new Wine API seems like the best route.
OK! Currently I am trying to add some functionality for this in brsfolder.c. However, I ran into a (probably small) problem:
I added a new function SHBrowseForUNIXDirectory in brsfolder.c and added it to shlobj.h. Reinstalled the shlobj.h, compiled the shell32.dll, and installed that new one too. Also runned ldconfig (don't know if that is a must).
Though, when I try to compile winecfg (with the SHBrowseForUNIXDirectory call) I get:
make ../../tools/winegcc/winegcc -B../../tools/winebuild -mwindows appdefaults.o audio.o drive.o drivedetect.o driveui.o libraries.o main.o properties.o winecfg.o x11drvdlg.o winecfg.exe.dbg.o winecfg.res -o winecfg.exe.so -L../../dlls -lcomdlg32 -lcomctl32 -luser32 -ladvapi32 -lkernel32 -lshell32 -L../../libs/wine -lwine -L../../libs/port -lwine_port /usr/lib/gcc-lib/i586-suse-linux/3.3.3/../../../../i586-suse-linux/bin/ld: cannot find -lshell32 collect2: ld returned 1 exit status winegcc: gcc failed. make: *** [winecfg.exe.so] Error 2
I haved checked that the new version is actually in /usr/local/lib/wine/shell32.dll.so.
What am I doing wrong?
Robert
On Sun, 28 Nov 2004 18:47:18 +0100, Robert van Herk wrote:
I added a new function SHBrowseForUNIXDirectory in brsfolder.c and added it to shlobj.h. Reinstalled the shlobj.h, compiled the shell32.dll, and installed that new one too. Also runned ldconfig (don't know if that is a must).
I don't think we have a specific policy on extending Win32 like this, but it's probably a good idea to make it obvious this API is Wine specific, perhaps by using the GNU coding conventions, eg: wine_shell_browse_for_folder() thanks -mike
On Sat, 27 Nov 2004, Robert van Herk wrote: [...]
Internally though, the SHBrowseForFolder dialog (dlls/shell32/brsfolder.c) uses pidls, which, as far as I know, are a specific win32 thing for identifying paths uniqually.
Thus, if I'd make an extra flag in the dialog, I would have to extent pidls to, so that they can hold unix paths. Is that a wise thing to do? Or should I take another approach?
Is there actually anything that prevents us from storing a Unix path in a PIDL? It's been quite some time since I looked at PIDLs and my memory is a bit fuzzy but I believe it would be quite feasible. If there's something preventing it, wouln't a Shell Namespace extension be the right solution? -- Francois Gouget fgouget(a)free.fr http://fgouget.free.fr/ War doesn't determine who's right. War determines who's left.
Hi, --- Francois Gouget <fgouget(a)free.fr> wrote:
If there's something preventing it, wouln't a Shell Namespace extension be the right solution?
This would be ideal. You there is lots of example code for extending the namespace for CVS and SVN so adding support for a Unix filesystem should not be too hard. Thanks Steven __________________________________ Do you Yahoo!? The all-new My Yahoo! - What will yours do? http://my.yahoo.com
Steven Edwards wrote:
Hi,
--- Francois Gouget <fgouget(a)free.fr> wrote:
If there's something preventing it, wouln't a Shell Namespace extension be the right solution?
This would be ideal. You there is lots of example code for extending the namespace for CVS and SVN so adding support for a Unix filesystem should not be too hard.
Oops, I guess this suggestion was a bit too late, since I commited my patch this afternoon... thanks, Robert
On Tue, 30 Nov 2004 22:08:26 +0100, Robert van Herk wrote:
Oops, I guess this suggestion was a bit too late, since I commited my patch this afternoon...
I wouldn't worry too much. I'm not sure a PIDL extension is a good idea anyway - it's probably the Microsoft way, but this is a Wine extension API so we don't have to follow in their footsteps exactly. It would be a harder to use API if it was PIDL based. thanks -mike
Mike Hearn wrote:
On Tue, 30 Nov 2004 22:08:26 +0100, Robert van Herk wrote:
Oops, I guess this suggestion was a bit too late, since I commited my patch this afternoon...
I wouldn't worry too much. I'm not sure a PIDL extension is a good idea anyway - it's probably the Microsoft way, but this is a Wine extension API so we don't have to follow in their footsteps exactly. It would be a harder to use API if it was PIDL based.
Exactly. Perhaps a shell namespace extension would be nice though. I don't know what exactly those are, but at least it makes a great buzz word :-). Anyways: I already implemented it (as you perhaps saw) using UNIX api calls. I think I did OK, though I am not very experienced in neither C programming or win32. Mike, perhaps you could take a look at my patch? It is not perfect yet (for example: folder icons are missing in UNIX browsing) but at least people can happily select a UNIX path now from winecfg and that was the main point :-). Regards, Robert
On Wed, 2004-12-01 at 17:08 +0100, Robert van Herk wrote:
Anyways: I already implemented it (as you perhaps saw) using UNIX api calls. I think I did OK, though I am not very experienced in neither C programming or win32. Mike, perhaps you could take a look at my patch?
Sure. Here you go.
@@ -46,7 +48,8 @@ shpolicy.c \ shv_bg_cmenu.c \ shv_item_cmenu.c \ - systray.c + systray.c \ + unixTools.c
s/unixTools.c/unix_tools.c/ for consistency +/************************************************************************* + * wine_shell_browse_for_UNIX_directory [SHELL32.@] + */ +BOOL WINAPI wine_shell_browse_for_UNIX_directoryA (LPBROWSEINFOA lpbi) +{ + if (!lpbi) + return FALSE; UNIX here should be lower case IMHO, makes it look nicer. Also you want to crash or do an ERR if a wine API is abused like this, silently returning false can lead to hard to find bugs. + + /*For UNIX browsing, pidls are not used. Hence, if the + user passes a pidl that should be root for this browse + anyway, we refuse to show the dialog:*/ Could you put spaces between the comment markers and the text? Also consistent indentation would be good. + /*For UNIX browsing, we always use the pszDiplayName to RETURN + the chosen path in, thus:*/ + if (!lpbi->pszDisplayName) + { + ERR("pszDisplayName must be set for UNIX browsing.\n"); + return FALSE; + } How does this work? Is the pszDisplayName pointing to a preallocated buffer (if so how do you know how big it should be?). +@ stdcall wine_shell_browse_for_UNIX_directory(ptr) wine_shell_browse_for_UNIX_directoryA +@ stdcall wine_shell_browse_for_UNIX_directoryA(ptr) +@ stdcall wine_shell_browse_for_UNIX_directoryW(ptr) Don't bother with a forwarder here, the WINELIB_NAME_AW macro should be sufficient.
C_SRCS = \ appdefaults.c \ @@ -16,7 +16,7 @@ main.c \ properties.c \ winecfg.c \ - x11drvdlg.c + x11drvdlg.c \
That looks wrong, a \ is only needed when there's something on the next line.
+ bi->hwndOwner = dialog; + bi->pszDisplayName = HeapAlloc(GetProcessHeap(), 0, MAX_PATH); + bi->lpszTitle = "Please choose a directory"; /*FIXME: Get this multilingual*/
Oh, I see. This is a rather unintuitive API design, why not simply return the path or have a char ** out parameter instead of setting the input structure?
+ if ((dirp=readdir(dirdata->dp))!=NULL) { + /* We are done, if we found an entry of the correct type: */ + if (strcmp(dirp->d_name, ".") == 0) continue; /*Ignore "." */ + if (strcmp(dirp->d_name, "..") == 0) continue; /*Ignore ".." */
Excellent! I was wondering if you'd make the mistake of assuming . and .. were the first two elements in the list, good to see you didn't. By the way, same comments as with Aneurin, could you please space things out rather than jamming it all together, eg if ((dirp = readdir(dirdata->dp) != NULL)
+static void prepNextFile(LP_UNIXTOOLS_DIRDATA dirdata) +{ + TRACE("hallo\n"); +
Well, the patch clearly still needs a bit more work ;) Still, it looks like it's on the right track. Good going! thanks -mike
+static void prepNextFile(LP_UNIXTOOLS_DIRDATA dirdata) +{ + TRACE("hallo\n"); +
Well, the patch clearly still needs a bit more work ;) Still, it looks like it's on the right track. Good going!
Haha! I am glad there is still a "Hallo" in the code :-). Thanks for your advice Mike. I will change it today or tomorrow. Robert
Hi Mike, I have some questions about your comments:
@@ -46,7 +48,8 @@ shpolicy.c \ shv_bg_cmenu.c \ shv_item_cmenu.c \ - systray.c + systray.c \ + unixTools.c
s/unixTools.c/unix_tools.c/ for consistency
What do you mean? Does s/x/y/ mean that I should change x into y?
Also consistent indentation would be good.
I did that, because the original code had some weird indentation and didn't want to change that where possible. So I kept the weird indentation where applicable. Overall, it gives it a bit a strange look, I agree.
+ /*For UNIX browsing, we always use the pszDiplayName to RETURN + the chosen path in, thus:*/ + if (!lpbi->pszDisplayName) + { + ERR("pszDisplayName must be set for UNIX browsing.\n"); + return FALSE; + }
How does this work? Is the pszDisplayName pointing to a preallocated buffer (if so how do you know how big it should be?).
Yes, this is the way MS intended SHBrowseForFolder to work, so I tried to stay consistent:
From http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/pla...:
*pszDisplayName* Address of a buffer to receive the display name of the folder selected by the user. The size of this buffer is assumed to be MAX_PATH characters. Of course, MAX_PATH might be different from the UNIX max path constant, though I am hoping to get away with that :-). When I copy the resulting path back into pszDisplayName, I truncate at MAX_PATH, just to be sure. Another solution would be to let the API indeed allocate the buffer, though the user will have to free it, which he might forget more easily, since he didn't do the allocation.
+ bi->hwndOwner = dialog; + bi->pszDisplayName = HeapAlloc(GetProcessHeap(), 0, MAX_PATH); + bi->lpszTitle = "Please choose a directory"; /*FIXME: Get this multilingual*/
Oh, I see. This is a rather unintuitive API design, why not simply return the path or have a char ** out parameter instead of setting the input structure?
See above. Also, in the future, the pszDisplayName could perhaps be use to preselect an entry, or, the way it works in win32, define the root of the dialog. Robert
On Wed, 2004-12-01 at 20:32 +0100, Robert van Herk wrote:
What do you mean? Does s/x/y/ mean that I should change x into y?
Yep, it's from vi regex syntax. Rather obscure geek notation I agree ...
I did that, because the original code had some weird indentation and didn't want to change that where possible. So I kept the weird indentation where applicable. Overall, it gives it a bit a strange look, I agree.
OK, this is all a bit of a grey area. Generally when modifying existing code you want to keep the differential to a minimum so it's easy to review, so don't modify indentation/code style etc alongside functional changes. Patches which are functional no-ops and just clean up code are OK but they should be separate. When writing new code though don't feel obliged to be consistent with the oddness in the rest of the code. Choose a style and stick to it. The rest of the code can always be adjusted later.
Yes, this is the way MS intended SHBrowseForFolder to work, so I tried to stay consistent:
From http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/pla...:
OK. I can't say whether you're right or wrong to do this, I don't have a strong opinion either way. Consistency with existing APIs is good because we can recycle the MSDN documentation for it, but it's bad because to quote Alexandre "Win32 is a textbook case of how not to design an API". So I'll pass the buck on this one. AJ can decide.
Another solution would be to let the API indeed allocate the buffer, though the user will have to free it, which he might forget more easily, since he didn't do the allocation.
Quite a lot of native APIs work this way, and I wouldn't say it's all that easy to forget.
Also, in the future, the pszDisplayName could perhaps be use to preselect an entry, or, the way it works in win32, define the root of the dialog.
OK, that makes sense. So maybe keep it like that. thanks -mike
participants (9)
-
Alexandre Julliard -
Andreas Mohr -
Dmitry Timoshkov -
Francois Gouget -
Mike Hearn -
Mike Hearn -
Robert van Herk -
Saulius Krasuckas -
Steven Edwards