GNU bug report logs - #20481
24.5; Newlines in message-box output don't work on Windows

Previous Next

Full log

Message #19 received at 20481 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Cecilio Pardo <cpardo <at> imayhem.com>
Cc: 20481 <at> debbugs.gnu.org
Subject: Re: bug#20481: 24.5;
 , Newlines in message-box output don't work on Windows
Date: Mon, 19 Aug 2024 20:44:50 +0300

> Date: Mon, 19 Aug 2024 18:13:31 +0200
> From: Cecilio Pardo <cpardo <at> imayhem.com>
> 
> This patch adds support on Windows Vista an later for dialog boxes using 
> TaskDialog.

Thanks.

First, to accept a contribution of this size we'll need a
copyright-assignment paperwork from you.  Should I send you the form
to fill with instructions to go with it, so you could start the
paperwork rolling?

A few comments about the patch:

> +  void *task_dialog_indirect =
> +    get_proc_addr (GetModuleHandle ("comctl32.dll"), "TaskDialogIndirect");
> +
> +  if (task_dialog_indirect)

A minor optimization is to call get_proc_addr only once and save the
result in a static variable.  We use this technique in many places in
Emacs, and I see no reason not to do that here.

> +      /* Get the title as a UTF-16 string. */
> +      CHECK_STRING (XCAR (contents));
> +      char *title =  SSDATA (XCAR (contents));
> +      int wide_length = sizeof(WCHAR) *
> +	pMultiByteToWideChar (CP_UTF8, 0, title, -1, NULL, 0);
> +      WCHAR *title_text_wide = alloca (wide_length);
> +      pMultiByteToWideChar (CP_UTF8, 0, title, -1,
> +			    title_text_wide, wide_length);

The text of Lisp strings is stored by Emacs in a super-set of UTF-8,
so it cannot be safely passed to MultiByteToWideChar.  You need to
encode it in UTF-8 first (use ENCODE_UTF_8).

> +	    CHECK_STRING (item_name);
> +
> +	    int wide_length = sizeof(WCHAR) *
> +	      pMultiByteToWideChar (CP_UTF8, 0, SSDATA (item_name), -1,
> +				    NULL, 0);
> +	    buttons[button_count].pszButtonText = alloca (wide_length);
> +	    pMultiByteToWideChar (CP_UTF8, 0, SSDATA (item_name), -1,
> +				  (LPWSTR)
> +				  buttons[button_count].pszButtonText,
> +				  wide_length);

Same here.

> +	else if (NILP (item))
> +	  {
> +	    /* A nil item means to put all following items on the
> +	       right. We ignore this. */
                    ^^              ^^
Our convention is to leave two spaces between sentences in
documentation, comments, and strings.  We also leave two spaces at the
end of C comments, before the closing "*/" (here and elsewhere in the
patch).

> +	else if (STRINGP(item))
                       ^^
Another stylistic nit: please leave one space between the name of a
function/macro and the opening parenthesis that follows it (here and
elsewhere in the patch).

> +	    /* A string item means an unselectable button. We add a
> +	       button, an then need to disable it on the callback.
> +	       We use ids based on 2000 to mark these buttons */
> +	    int wide_length = sizeof(WCHAR) *
> +	      pMultiByteToWideChar (CP_UTF8, 0, SSDATA (item), -1, NULL, 0);
> +	    buttons[button_count].pszButtonText = alloca (wide_length);
> +	    pMultiByteToWideChar (CP_UTF8, 0, SSDATA (item), -1,
> +				  (LPWSTR)
> +				  buttons[button_count].pszButtonText,
> +				  wide_length);

UTF-8 encoding again.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.