Re: Shell32 File Property Dialog
Johannes Anderwald wrote:
@@ -1185,6 +1185,7 @@ BOOL SHELL_execute( LPSHELLEXECUTEINFOW
static const WCHAR wFile[] = {'f','i','l','e',0};
static const WCHAR wHttp[] = {'h','t','t','p',':','/','/',0};
static const WCHAR wExplorer[] =
{'e','x','p','l','o','r','e','r','.','e','x','e',0};
+static const WCHAR wProperties[] =
{'p','r','o','p','e','r','t','i','e','s',0};
static const DWORD unsupportedFlags =
SEE_MASK_INVOKEIDLIST | SEE_MASK_ICON | SEE_MASK_HOTKEY |
SEE_MASK_CONNECTNETDRV | SEE_MASK_FLAG_DDEWAIT | SEE_MASK_FLAG_NO_UI |
@@ -1275,7 +1276,11 @@ BOOL SHELL_execute( LPSHELLEXECUTEINFOW
sei-hInstApp = (HINSTANCE) 33;
return TRUE;
}
-
+if (sei_tmp.lpVerb)
+{
+if (!strcmpW(sei_tmp.lpVerb,wProperties))
+return SH_ShowPropertiesDialog(sei_tmp.lpFile);
+}
I should have pointed out earlier that this is not the right way to
activate a properties dialog. This will show the same properties dialog
for all shell objects, which is wrong.
You should create a handler for the properties verb in the IContextMenu
interface of the shell object that you're showing the dialog for. You
need to modify both -QueryContextMenu and -InvokeCommand.
I have implement IContextMenu for the ShellLink object (shelllink.c).
Perhaps try extending that code to make a properties dialog for the
ShellLink object first.
The rest looks better though.
Mike
Re: Shell32 File Property Dialog
Andreas Mohr wrote:
FIXME(Unhandled Verb %xl\n,LOWORD(lpcmi-lpVerb));
What kind of format specifier is that supposed to be?
I don't know that one...
Maybe use %p instead? (or %lx??)
This statement should be %lx. However, this statement is _not_ part of
my patch.
if (GetFileSize(hFile, NULL) == 0x)
{
CloseHandle(hFile);
return FALSE;
}
dwFileLo = GetFileSize(hFile, dwFileHi);
CloseHandle(hFile);
if (dwFileLo == 0x GetLastError() != NO_ERROR)
return FALSE;
This whole check sounds very weird.
Why are you checking with NULL hiword when there might be a 4GB file?
(and to make it worse, even one with e.g. a size of 0x12345678 !!)
And directly after that even doing a full large file check **again**?
Not to mention that you're not using the INVALID_FILE_SIZE macro that I really,
really think should be used since it's been created *exactly* for this purpose
(and for the even better purpose of *never* managing to write/edit/delete a
0xFF or 0xFFF instead...)
I agree that the INVALID_FILE_SIZE should be used. However,
INVALID_FILE_SIZE macro is exactly 0x. Furthermore, this code
does work with file sizes of 0x12345678. Have a look at the MSDN
documentation[1]. Alternatively, GetFileSizeEx could be used.
Andreas
[1]
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/fileio/fs/getfilesize.asp
regards,
--
Johannes Anderwald
Re: Shell32 File Property Dialog
Hi, On Wed, Nov 02, 2005 at 01:15:32AM +0100, Johannes Anderwald wrote: Andreas Mohr wrote: FIXME(Unhandled Verb %xl\n,LOWORD(lpcmi-lpVerb)); What kind of format specifier is that supposed to be? I don't know that one... Maybe use %p instead? (or %lx??) This statement should be %lx. However, this statement is _not_ part of my patch. Doh, sorry, didn't realize that. dwFileLo = GetFileSize(hFile, dwFileHi); CloseHandle(hFile); if (dwFileLo == 0x GetLastError() != NO_ERROR) return FALSE; This whole check sounds very weird. Why are you checking with NULL hiword when there might be a 4GB file? (and to make it worse, even one with e.g. a size of 0x12345678 !!) And directly after that even doing a full large file check **again**? Not to mention that you're not using the INVALID_FILE_SIZE macro that I really, really think should be used since it's been created *exactly* for this purpose (and for the even better purpose of *never* managing to write/edit/delete a 0xFF or 0xFFF instead...) I agree that the INVALID_FILE_SIZE should be used. However, INVALID_FILE_SIZE macro is exactly 0x. Furthermore, this code does work with file sizes of 0x12345678. Have a look at the MSDN documentation[1]. Alternatively, GetFileSizeEx could be used. My point was that using INVALID_FILE_SIZE as intended by the system *ensures* that you may never potentially mis-write or mis-modify 0x. And yes, I had a look at the MSDN page, incidentially that's exactly why I was commenting on it since I knew how horrible GetFileSize() is... And it appears your code was (almost) right indeed, minus the problem that you're calling GetLastError() after CloseHandle() (which will also modify last error on failure). But you probably intended to call CloseHandle() before the check in order to avoid an extra CloseHandle() in the code... Argh, whoever doesn't hate this particular API please raise your hands now... Thanks for your work! Andreas
Re: Shell32 File Property Dialog
Hi,
On Sat, Oct 29, 2005 at 08:59:51PM +0200, Johannes Anderwald wrote:
This patch adds file property dialog to shell32
+ LTEXT Type of file:, 14004, 10, 30, 50, 10
Space missing??
+ LTEXT Modied: , 14016, 10, 90, 45, 10
Modified:
FIXME(Unhandled Verb %xl\n,LOWORD(lpcmi-lpVerb));
What kind of format specifier is that supposed to be?
I don't know that one...
Maybe use %p instead? (or %lx??)
HWND hDlgCtrl = GetDlgItem(hwndDlg, 14005);
if (filext == NULL || hDlgCtrl == NULL)
return FALSE;
Wasteful invocation of GetDlgItem() here, although the failure check is
a bit prettier that way...
result = RegEnumValueW(hKey,0, name, lname, NULL, NULL, (LPBYTE)value,
lvalue);
space missing after hkey ;)
BOOL
SHFileGeneralGetFileTimeString(LPFILETIME lpFileTime, WCHAR * lpResult)
{
FILETIME ft;
SYSTEMTIME dt;
WORD wYear;
WCHAR wFormat[] =
{'%','0','2','d','/','%','0','2','d','/','%','0','4','d',' ','
','%','0','2','d',':','%','0','2','u',0};
static const WCHAR
TRACE(result %s \n,debug_strw(lpResult));
Superfluous space.
WARN(failed to open file \n);
WARN(GetFileTime failed \n);
TRACE(hDlgCtrl %x text %s \n,hDlgCtrl, debug_strw(resultstr));
WARN(failed to open file \n);
WARN(GetFileSize failed \n);
Dito (I prefer Wine to be smaller rather than the error message to be a bit
more readable ;).
(and probably more instances of superfluous space in this patch)
if (GetFileSize(hFile, NULL) == 0x)
{
CloseHandle(hFile);
return FALSE;
}
dwFileLo = GetFileSize(hFile, dwFileHi);
CloseHandle(hFile);
if (dwFileLo == 0x GetLastError() != NO_ERROR)
return FALSE;
This whole check sounds very weird.
Why are you checking with NULL hiword when there might be a 4GB file?
(and to make it worse, even one with e.g. a size of 0x12345678 !!)
And directly after that even doing a full large file check **again**?
Not to mention that you're not using the INVALID_FILE_SIZE macro that I really,
really think should be used since it's been created *exactly* for this purpose
(and for the even better purpose of *never* managing to write/edit/delete a
0xFF or 0xFFF instead...)
FIXME(directory / drive resources dont exist yet \n);
' missing ;)
Sorry for this VERY anal review (it's got to be my most thorough WP review),
and thanks very much for this large patch :)
Andreas
--
GNU/Linux. It's not the software that's free, it's you.
Re: Shell32 File Property Dialog
This version looks alot better than the first one. * SH_FileTimerProc [Internal] * * is invoked every 100ms to check if the property sheet should be closed * A timer is required because the property sheet pages are per default * modeless and the property sheetpage does not have a window procedure This doesn't seem right to me. The property sheet should destroy all its pages when it is closed. Also, instead of attaching the new file, you can do: diff -u /dev/null dlls/shell32/fprop.c fp-051029.diff so that everything is in one diff. Mike
Re: Shell32 File Property dialog
Am Freitag, den 14.10.2005, 13:28 +0200 schrieb Johannes Anderwald: I have a patch which adds file property dialogs to shell32 Welcome to wine. Your Idea is nice and many Thanks for your Wine-Support, but since wine is different from ReactOS, your Patch will not work here. I started half a year ago with similar Problems. Please read take a look in the Wine Developer's Guide: http://www.winehq.com/site/docs/wine-devel/index And the Part about Coding Practice http://www.winehq.com/site/docs/wine-devel/codingpractice - Define a Unicode-String with Ltext does not work with gcc on unix. Please use the same Style as your Patch did for wProperties in shlexec.c (Array of WCHAR). - TCHAR is not used in wine. If there is a UNICODE and a ANSI implementation in Windows, implement the UNICODE-Version and let the ANSI-Version call the UNICODE-Version. - I don't knew, if DPRINT() works in wine. We use here: TRACE(), WARN(), ERR() and FIXME() with WINE_DEFAULT_DEBUG_CHANNEL() This messages go to the console. (There are more Macros for additional Debug-Channels as well) - You use numeric values very often (Dialog-Parameter, Buffer-Length) Are there really no defines for this Numbers? - K.I.S.S = Make your Patch as small as possible. An Idea would be to Start with one Page and add the other Pages later. There are some FIXME in the code (Example: Date/Time - Format). How much work is it to do it the Windows-Way ? The following Link is a useful collection of Coding Hints: http://wine-wiki.org/index.php/Coding_Hints (wine-wiki.org (private) started before the official wiki.winehq.org comes up.) Asking Steven Edwards how to Port from ReactOS to wine is IMHO another good idea. -- By By ... ... Detlef
Re: Shell32 File Property dialog
Johannes Anderwald wrote: I have a patch which adds file property dialogs to shell32 Looks like you've written the patch for ReactOS... it doesn't apply to the current Wine tree. I think you need to do a little bit more work to get it ready for submission: * use TRACE instead of DPRINT * don't use %S in debug statements (with TRACE) it's not portable, use %s and debugstr_w(str) instead * in SH_FileVersionDlgProc you miss a break at the end of WM_COMMAND * the formatting is all over the place. 4 space indent, no tabs is prefered * make sure to use the A or W functions and types explicitly. eg. you use PROPSHEETPAGE where you should use PROPSHEETPAGEW. * this comments looks dodgy: SH_FileTimerProc is invoked every 100ms to check if the property sheet should be closed required because the property sheet pages are MODELESS Your Window proc will get a WM_CLOSE when you need to close, or WM_DESTROY when it is destroyed. Freeing memory and other things that need to be done when the window is closed should happen in the Window procedure, and there should be no need for a timer. Please take the time to build you patch with and generate it against the CVS version of WineHQ. thanks for the patch, Mike
