On 19.06.2009 09:17, Yuki KODAMA wrote:
> On Thu, Jun 18, 2009 at 4:08 PM, Adrian Buehlmann<adr...@cadifra.com> wrote:
>> On 18.06.2009 03:58, Yuki KODAMA wrote:
>>> On Thu, Jun 18, 2009 at 8:49 AM, Yuki KODAMA<endflow....@gmail.com> wrote:
>>>> On Thu, Jun 18, 2009 at 8:00 AM, Adrian Buehlmann<adr...@cadifra.com>
>>>> 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?
>
Looks like idCmd = 14 is for the "TortoiseHg..." root menu entry,
which is not a command we have in our bookkeeping of Cmd id's.
We are queried for the help text by explorer for this entry as well.
This is caused by:
TDEBUG_TRACE(" CShellExt::QueryContextMenu: adding main THG menu");
InsertSubMenuItemWithIcon(hMenu, hSubMenu, indexMenu++, idCmd++,
"TortoiseHG...", "hg.ico");
in QueryContextMenu
So, maybe implemented a bit oddly but no problem here.
------------------------------------------------------------------------------
_______________________________________________
Tortoisehg-develop mailing list
Tortoisehg-develop@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tortoisehg-develop
