Re: [comctl32] implement header callback support (HDN_GETDISPINFO notification)

[Vitaliy Margolen]

Monday, November 14, 2005, 6:31:29 PM, Raphael wrote:
> Changelog:
>  - implement implement header callback support (HDN_GETDISPINFO notification):

Patch comments:
Please use the commonly accepted coding style in wine:
- Indents are 4 spaces (and spaces, not tabs)
- Curly brackets go to their own line:
  if (somethin)
  {
      do_this;
  }
  else
  {
      do_something_else;
  }
> @@ -601,6 +601,40 @@
> Please add "-p" to diff so we can see what function this changes are to.
> +HEADER_SendHeaderDispInfoNotify(HWND hwnd, INT iItem, INT mask, NMHDDISPINFOW* pDispInfo)
> You can't do this. Please respect unicodiness of the control. See other controls
> (listvew, treeview for examples).
> +    /*FIXME("HEADER_SendHeaderDispInfoNotify\n");*/
> Don't comment, delete.
> +    FIXME("SendMessage returns(mask:0x%x,str:%s,lParam:%p)\n",
> +         pDispInfo->mask,
> +         debugstr_w(pDispInfo->pszText),
> +         (void*) pDispInfo->lParam
> +         );
> Why is this a FIXME? Don't add fixmes for no good reason. Use TRACE instead.
> +       HEADER_SendHeaderDispInfoNotify(hwnd, nItem, HDI_IMAGE, &dispInfo);
> +       if (dispInfo.mask & HDI_DI_SETITEM) {
> +         FIXME("NMHDDISPINFO returns with flags HDI_DI_SETITEM\n");
> +           lpItem->>iImage = dispInfo.iImage;
> +       }
> There is no reason to print same fixme two times (once in
> HEADER_SendHeaderDispInfoNotify an second time here).
> -        else if (lpItem->pszText == LPSTR_TEXTCALLBACKW) /* covers == TEXTCALLBACKA too */
> -            phdi->>pszText = LPSTR_TEXTCALLBACKW;
> +        else if (lpItem->pszText == LPSTR_TEXTCALLBACKW) { /* covers == TEXTCALLBACKA too */
> +         /*phdi->>pszText = LPSTR_TEXTCALLBACKW;*/
> +       NMHDDISPINFOW dispInfo; 
> +       HEADER_SendHeaderDispInfoNotify(hwnd, nItem, HDI_TEXT, &dispInfo);
> This requires a test. I don't think native sends notify to the app for
> HDM_GETITEM.

Vitaliy

[comctl32] implement header callback support (HDN_GETDISPINFO notification)

[Dimi Paun]

On Mon, 2005-11-14 at 19:02 -0700, Vitaliy Margolen wrote:
> Monday, November 14, 2005, 6:31:29 PM, Raphael wrote:
> > Changelog:
> >  - implement implement header callback support (HDN_GETDISPINFO notification):
>
> Patch comments:
> Please use the commonly accepted coding style in wine:
> - Indents are 4 spaces (and spaces, not tabs)
> - Curly brackets go to their own line:
>   if (somethin)
>   {
>       do_this;
>   }
>   else
>   {
>       do_something_else;
>   }

That file uses the  K&R style of brackets, so he should use the same
style for consistency. But yes, you are correct that he should use
4 space indents for the same reason.
> > -        else if (lpItem->pszText == LPSTR_TEXTCALLBACKW) /* covers == TEXTCALLBACKA too */
> > -            phdi->>pszText = LPSTR_TEXTCALLBACKW;
> > +        else if (lpItem->pszText == LPSTR_TEXTCALLBACKW) { /* covers == TEXTCALLBACKA too */
> > +         /*phdi->>pszText = LPSTR_TEXTCALLBACKW;*/
> > +       NMHDDISPINFOW dispInfo; 
> > +       HEADER_SendHeaderDispInfoNotify(hwnd, nItem, HDI_TEXT, &dispInfo);
> > This requires a test. I don't think native sends notify to the app for
> > HDM_GETITEM.

It does, this is the very meaning of LPSTR_TEXTCALLBACK.
It works the same in all common controls.

-- 
Dimi Paun <dimi(a)lattica.com>
Lattica, Inc.

[comctl32] implement header callback support (HDN_GETDISPINFO notification)

[Raphael]

Hi,

Try 2 

Changelog:
 - implement implement header callback support (HDN_GETDISPINFO notification):
 - better factorisation
 - unicodiness fixes

On Tuesday 15 November 2005 03:02, Vitaliy Margolen wrote:
> Monday, November 14, 2005, 6:31:29 PM, Raphael wrote:
> > +HEADER_SendHeaderDispInfoNotify(HWND hwnd, INT iItem, INT mask,
> > NMHDDISPINFOW* pDispInfo)
>
> You can't do this. Please respect unicodiness of the control. See other
> controls (listvew, treeview for examples).

expect this patch is correct for unicodiness 
(i haven't see the problem as headers never used NotifyFormat switches before)
> > -        else if (lpItem->pszText == LPSTR_TEXTCALLBACKW) /* covers ==
> > TEXTCALLBACKA too */ -            phdi->>pszText = LPSTR_TEXTCALLBACKW;
> > +        else if (lpItem->pszText == LPSTR_TEXTCALLBACKW) { /* covers ==
> > TEXTCALLBACKA too */ +         /*phdi->>pszText = LPSTR_TEXTCALLBACKW;*/
> > +       NMHDDISPINFOW dispInfo;
> > +       HEADER_SendHeaderDispInfoNotify(hwnd, nItem, HDI_TEXT,
> > &dispInfo);
>
> This requires a test. I don't think native sends notify to the app for
> HDM_GETITEM.

This is the meaning of GETDISPINFO header callbacks as seen in documentation:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/pla...
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/pla...
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/pla...

Thx for comments
> Vitaliy

Regards,
Raphael

[comctl32] implement header callback support (HDN_GETDISPINFO notification)

[Michael Stefaniuc]

Vitaliy Margolen wrote:
> Monday, November 14, 2005, 6:31:29 PM, Raphael wrote:
> > Changelog:
> > - implement implement header callback support (HDN_GETDISPINFO notification):
>
> Patch comments:
> Please use the commonly accepted coding style in wine:
> - Indents are 4 spaces (and spaces, not tabs)
> - Curly brackets go to their own line:
> The style of bracketing was never part of the "commonly accepted coding 
> style in wine". In use are K&R and Gnu style. Though K&R style is The 
> Best and Gnu style an abomination. We didn't have a coding style flame 
> war in a while ;)
> if (somethin)
>   {
>       do_this;
>   }
>   else
>   {
>       do_something_else;
>   }

bye
	michael
-- 
Michael Stefaniuc               Tel.: [phone]
Sr. Network Engineer            Fax.: [phone]
Red Hat GmbH                    Email: mstefani(a)redhat.com
Hauptstaetterstr. 58            http://www.redhat.de/
D-70178 Stuttgart