bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#71909: 30.0.60;


From: Eli Zaretskii
Subject: bug#71909: 30.0.60;
Date: Sat, 02 Nov 2024 12:44:32 +0200

> Date: Sat, 2 Nov 2024 01:23:11 +0100
> Cc: 71909@debbugs.gnu.org
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> Here is a new version, with the discussed corrections.
> 
> - Now the test for gdiplus is done calling w32_gdiplus_startup.
>    Will use it if its available, even if native image functions
>    are disabled.
> - Don't use cl-seq.
> - Use Fexpand_file_name on file name.
> - Send unibyte string only for binary data.
> - Use unwind-protect to ensure temp file is deleted.
> - Fixed docs, changelog, NEWS and manual.

Thanks, there are a few minor nits left.

> Adds the capacity to handle types different from strings to the
> clipboard management functions on MS-Windows, and some logic
> required to convert media types names and content to be what
> yank-media and the modes that use it expect.

Please mention the bug number somwhere in the log message.

> * lisp/term/w32-win.el (w32--selection-target-translations): New
> variable that holds the name translations for media tytpes.
                                                      ^^^^^^
Typo.

> (w32--translate-reverse-selection-target): New function, Reverse
> translation.                                             ^^^^^^^

"reverse", not capitalized.

> * src/w32select.c (stdfmt_name): Made global, was function
> static.                                       ^^^^^^^^^^^^
  ^^^^^^
"was static function"

> (convert_dibv5_to_png): New function to convert DIBV5 clipboard
> format to PNG.
> (get_clipboard_format_name): New function get the name of a
> format given its index.
> (Fw32__get_clipboard_data_media): New function, retrieves and
> converts media content.
> (syms_of_w32select): Export new lisp functions
> * src/w32gdiplus.h: New file, for definitions in w32image.c
> * doc/lispref/frames.texi: Updated with MS-Windows support.

etc/NEWS not mentioned.

> ++++
> +** Emacs on MS-Windows now supports 'yank-media'.
> +This command inserts clipboard data of different formats into the
> +current buffer, if the active mode supports it.
                          ^^^^^^^^^^^
"major mode"

> +(defun w32--translate-selection-target(target)
                                        ^^
We leave one space between the function's name and the opening
parenthesis.  Same issue with other functions you added.

> +  (let ((str (symbol-name mime-type)))
> +    (or
> +     (eq mime-type 'application/xml)
> +     (eq mime-type 'application/json)
> +     (eq mime-type 'application/yaml)
> +     (eq mime-type 'application/json-seq)
> +     (string-match-p "\\`text/" str)
> +     (string-match-p "+xml\\'" str)
> +     (string-match-p "+json\\'" str)
> +     (string-match-p "+yaml\\'" str)
> +     (string-match-p "+json-seq\\'" str))))

This begs for 2 variables and using memq and seq-contains-p.  Or am I
missing something?

> +Elements in FORMATS are symbols naming a format, such a image/png, or
> +image/jpeg.  They don't need to be MIME types, any format available can
> +be retrieved.  For compatibility with X systems, some conventional
> +format names are translated to equivalent MIME types.

Should this mention 'w32--selection-target-translations'?

And I don't understand what you mean by the second sentence above.
Surely, "any format" can be retrieved only if there's a handler for
it?

> +If it returns t, then the file contains the data.

I guess we should add "and the caller should read the file to fetch
the data"?

> +If it returns a string, then that is the data and the file is not used.
> +
> +When returning a string, it can be unibyte if the format is not known to
> +be text.  */)
> +  (Lisp_Object formats, Lisp_Object temp_file_in, Lisp_Object is_textual)

This doc string doesn't say anything about the IS-TEXTUAL argument.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]