On 18.06.2009 03:58, Yuki KODAMA wrote: > On Thu, Jun 18, 2009 at 8:49 AM, Yuki KODAMA<[email protected]> wrote: >> On Thu, Jun 18, 2009 at 8:00 AM, Adrian Buehlmann<[email protected]> wrote: >>> On 18.06.2009 00:51, Adrian Buehlmann wrote: >>>> I'm looking at >>>> http://bitbucket.org/tortoisehg/crew/src/e214839bf369/win32/shellext/ContextMenu.cpp#cl-445 >>>> >>>> we have >>>> >>>> STDMETHODIMP >>>> CShellExt::GetCommandString( >>>> UINT_PTR idCmd, UINT uFlags, UINT FAR *reserved, >>>> LPSTR pszName, UINT cchMax) >>>> { >>>> *pszName = 0; >>>> char *psz; >>>> >>>> TDEBUG_TRACE( >>>> "CShellExt::GetCommandString: idCmd = " << idCmd >>>> << ", uFlags = " << uFlags >>>> ); >>>> MenuIdCmdMap::iterator iter = MenuIdMap.find(static_cast<UINT>(idCmd)); >>>> if (iter != MenuIdMap.end()) >>>> { >>>> TDEBUG_TRACE( >>>> "CShellExt::GetCommandString: name = " << iter->second.name); >>>> psz = (char*)iter->second.helpText.c_str(); >>>> } >>>> else >>>> { >>>> TDEBUG_TRACE( >>>> "CShellExt::GetCommandString: can't find idCmd " << idCmd); >>>> psz = ""; >>>> } >>>> >>>> if (uFlags & GCS_UNICODE) >>>> { >>>> wcscpy((wchar_t*)pszName, _WCSTR(psz)); >>>> } >>>> else >>>> { >>>> strcpy((char*)pszName, psz); >>>> } >>>> return NOERROR; >>>> } >>>> >>>> Can someone explain what _WCSTR() does? >>>> >>>> http://msdn.microsoft.com/en-us/library/bb776083(VS.85).aspx specifies >>>> that if uFlags & GCS_UNICODE, a "unicode" string must be copied into >>>> pszName (I assume a wstring is meant with that). >>>> >>>> But how do we get that wstring? >>>> >>>> See also >>>> http://bitbucket.org/tortoisehg/crew/changeset/70f7297adda2/ >>>> >>>> Usage of _WCTSR() seems to go back to >>>> http://bitbucket.org/tortoisehg/crew/src/c3caaf07dbcc/tortoise/shellext/ContextMenu.cpp#cl-260 >>>> (TK Soh's initial commit) >>>> >>>> >>> On second look, the implementation of this function looks >>> rather broken. >>> >>> The spec wants >>> >>> uFlags >>> Flags specifying the information to return. This parameter can have one >>> of the following values. >>> >>> GCS_HELPTEXTA >>> Sets pszName to an ANSI string containing the help text for the >>> command. >>> GCS_HELPTEXTW >>> Sets pszName to a Unicode string containing the help text for the >>> command. >>> GCS_VALIDATEA >>> Returns S_OK if the menu item exists, or S_FALSE otherwise. >>> GCS_VALIDATEW >>> Returns S_OK if the menu item exists, or S_FALSE otherwise. >>> GCS_VERBA >>> Sets pszName to an ANSI string containing the language-independent >>> command name for the menu item. >>> GCS_VERBW >>> Sets pszName to a Unicode string containing the language-independent >>> command name for the menu item. >>> >>> >>> we always seem return helpText, no matter what uFlags is. >>> >>> >> Yes, I know. Actual implementation is broken and I've surprise when I >> saw it at first time. >> That patch (by me) is a just work around. It won't fix the root problem. >> >> What is the root problem? I suppose it's handling of string. >> We should use "TCHAR string system" instead of ''char" or "wchar_t" If >> you want to >> implement it proper, >> But I though we can't rewrite the entire code before 0.8 release.
We should be able to correctly implement the current ANSI string implementation. If the calling side requests a wide string, we can convert our ANSI strings we have. Seems that's what you did. Thanks for your patch. >> Using TCHAR system: >> http://bitbucket.org/kuy/sharphg/src/tip/SharpHgShell/HgContextMenu.cpp#cl-148 >> >>> we always seem return helpText, no matter what uFlags is. >> Yes and No. It's help text, but it's need different string types, >> "Unicode" and "ANSI". >> Windows explorer need "Unicode" version, but some applications >> (integrated with shell extension) need "ANSI" string. Thanks for the hint. I haven't had time to look at this yet, as I was absorbed by the overlays. The context menu is mostly still as per TK's implementation. Since it just seemed to work, I had no reason to dig into this further yet. Maybe we should at least return an error if we haven't found the string. I might play with this today a bit. There seem to be quite a couple to ways to call this function which don't seem to be really used/needed. Looks like explorer is just happy with our current implementation. I was wondering about the not-found idCmd cases in debug output: [3504] [THG] CShellExt::GetCommandString: idCmd = 0, uFlags = 4 [3504] [THG] CShellExt::GetCommandString: name = commit [3504] [THG] CShellExt::GetCommandString: idCmd = 14, uFlags = 4 [3504] [THG] CShellExt::GetCommandString: can't find idCmd 14 [3504] [THG] CShellExt::GetCommandString: idCmd = 14, uFlags = 4 [3504] [THG] CShellExt::GetCommandString: can't find idCmd 14 we currently just return an empty string in this case but don't flag an error. Maybe it would be better to return an error. Another open question I have is where do we get this unknown idCmd with value 14 from. Just smells to me like we shouldn't get this 14 in the first place. A hint for an incorrectness somewhere else. > "_WCSTR" is a macro that convert string from ANSI to Unicode: > > #define _WCSTR(str) hf_mbtowc((LPWSTR)alloca((strlen(str) + 1) * > sizeof(WCHAR)),(str),strlen(str) + 1) > > in TortoiseUtils.h header file. Ah, thanks for the pointer (I was looking too far :). So this is allocating space on the stack, which is correct. ------------------------------------------------------------------------------ Crystal Reports - New Free Runtime and 30 Day Trial Check out the new simplified licensing option that enables unlimited royalty-free distribution of the report engine for externally facing server and web deployment. http://p.sf.net/sfu/businessobjects _______________________________________________ Tortoisehg-develop mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/tortoisehg-develop
