On Thu, Jun 18, 2009 at 4:08 PM, Adrian Buehlmann<[email protected]> wrote: > 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. >
I also noticed unknown idCmd. But I didn't track any more in back then. Are there some finding of you? BTW, I created new page in crew Wiki: http://bitbucket.org/tortoisehg/crew/wiki/NewShellExtensionPlan This is just memorize, so please feel free add/remove you want. And this page http://bitbucket.org/tortoisehg/crew/wiki/ShellExtension is very useful for other developers. Thank you. I'll add a topic how to build debug DLL using VC9. In my environment, release DLL didn't output any debug information. regards, Yuki ------------------------------------------------------------------------------ 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
