bug#73258: 31.0.50; w32 drag-n-dropping multiple files is broken

bug#73258: 31.0.50; w32 drag-n-dropping multiple files is broken

[Cecilio Pardo]

[-- Attachment #1: Type: text/plain, Size: 264 bytes --]

dnd-handle-multiple-urls is called once for each file. This brings 
problems, such that when dropping two directories, emacs open the first 
one on dired, then tries to copy the contents of the second to the 
first. The attached patch fixes this. -- Cecilio Pardo

[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 1677 bytes --]

diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index 3c0acf368f4..29629c9072c 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -100,7 +100,7 @@ w32-color-map
 ;;   (interactive "e")
 ;;   (princ event))
 
-(defun w32-handle-dropped-file (window file-name)
+(defun w32-dropped-file-to-url (file-name)
   (let ((f (if (eq system-type 'cygwin)
                (cygwin-convert-file-name-from-windows file-name t)
              (subst-char-in-string ?\\ ?/ file-name)))
@@ -117,14 +117,12 @@ w32-handle-dropped-file
                      (split-string (encode-coding-string f coding)
                                    "/")
                      "/")))
-  ;; FIXME: is the W32 build capable only of receiving a single file
-  ;; from each drop?
-  (dnd-handle-multiple-urls window (list (concat
-			                  (if (eq system-type 'cygwin)
-				              "file://"
-			                    "file:")
-			                  file-name))
-                            'private))
+
+  (concat
+   (if (eq system-type 'cygwin)
+       "file://"
+     "file:")
+   file-name))
 
 (defun w32-drag-n-drop (event &optional new-frame)
   "Edit the files listed in the drag-n-drop EVENT.
@@ -146,8 +144,11 @@ w32-drag-n-drop
       (raise-frame)
       (setq window (selected-window))
 
-      (mapc (apply-partially #'w32-handle-dropped-file window)
-            (car (cdr (cdr event)))))))
+      (dnd-handle-multiple-urls
+       window 
+       (mapcar #'w32-dropped-file-to-url 
+               (car (cdr (cdr event))))
+       'private))))
 
 (defun w32-drag-n-drop-other-frame (event)
   "Edit the files listed in the drag-n-drop EVENT, in other frames.

bug#73258: 31.0.50; w32 drag-n-dropping multiple files is broken

[Eli Zaretskii]

> Date: Sat, 14 Sep 2024 21:33:00 +0200
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> dnd-handle-multiple-urls is called once for each file. This brings 
> problems, such that when dropping two directories, emacs open the first 
> one on dired, then tries to copy the contents of the second to the 
> first. The attached patch fixes this. -- Cecilio Pardo

Thanks.

We cannot remove or make backward-incompatible changes in a public
API.  So removing/renaming w32-handle-dropped-file and/or changing its
signature is out of the question.  Can you rewrite the patch such that
it keeps this function and its arguments, and just change the
implementation to fix the problem?

Also, please accompany your changes with a ChangeLog-style description
(see CONTRIBUTE for the details), to make the job of installing the
changes easier.

Thanks again for working on this.

bug#73258: 31.0.50; w32 drag-n-dropping multiple files is broken

[Cecilio Pardo]

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]


On 15/09/2024 10:25, Eli Zaretskii wrote:
> We cannot remove or make backward-incompatible changes in a public
> API.  So removing/renaming w32-handle-dropped-file and/or changing its
> signature is out of the question.  Can you rewrite the patch such that
> it keeps this function and its arguments, and just change the
> implementation to fix the problem?
>
> Also, please accompany your changes with a ChangeLog-style description
> (see CONTRIBUTE for the details), to make the job of installing the
> changes easier.

See the attached patch. Hope I did it right, let me know.

Thanks.

[-- Attachment #2: 0001-Fix-multifile-drag-n-drop-on-win32.patch --]
[-- Type: text/plain, Size: 1716 bytes --]

From 3fece1ca8d89258c0a02897110e2f9d4c31eb36a Mon Sep 17 00:00:00 2001
From: Cecilio Pardo <cpardo@imayhem.com>
Date: Sun, 15 Sep 2024 21:06:00 +0200
Subject: [PATCH] Fix multifile drag-n-drop on win32

Pass all dropped files to dnd-handle-multiple-urls
on one call instead on doing multiple calls (Bug#73258).
* lisp/term/w32-win.el (w32-dropped-file-to-url): New function to
convert file name to a url for dnd
(w32-handle-dropped-file): Changed to use w32-dropped-file-to-url
(w32-drag-n-drop): Changed to pass al files to
dnd-handle-multiple-urls
---
 lisp/term/w32-win.el | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index 3c0acf368f4..95c2b357666 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -101,6 +101,13 @@ w32-color-map
 ;;   (princ event))
 
 (defun w32-handle-dropped-file (window file-name)
+  (dnd-handle-multiple-urls
+   window
+   (list
+    (w32-dropped-file-to-url file-name))
+   'private))
+
+(defun w32-dropped-file-to-url (file-name)
   (let ((f (if (eq system-type 'cygwin)
                (cygwin-convert-file-name-from-windows file-name t)
              (subst-char-in-string ?\\ ?/ file-name)))
@@ -146,8 +153,11 @@ w32-drag-n-drop
       (raise-frame)
       (setq window (selected-window))
 
-      (mapc (apply-partially #'w32-handle-dropped-file window)
-            (car (cdr (cdr event)))))))
+      (dnd-handle-multiple-urls
+       window
+       (mapcar #'w32-dropped-file-to-url
+               (car (cdr (cdr event))))
+       'private))))
 
 (defun w32-drag-n-drop-other-frame (event)
   "Edit the files listed in the drag-n-drop EVENT, in other frames.
-- 
2.35.1.windows.2

bug#73258: 31.0.50; w32 drag-n-dropping multiple files is broken

[Cecilio Pardo]

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

Did you get a chance to review this?

On 15/09/2024 21:22, Cecilio Pardo wrote:
>
> On 15/09/2024 10:25, Eli Zaretskii wrote:
>> We cannot remove or make backward-incompatible changes in a public
>> API.  So removing/renaming w32-handle-dropped-file and/or changing its
>> signature is out of the question.  Can you rewrite the patch such that
>> it keeps this function and its arguments, and just change the
>> implementation to fix the problem?
>>
>> Also, please accompany your changes with a ChangeLog-style description
>> (see CONTRIBUTE for the details), to make the job of installing the
>> changes easier.
>
> See the attached patch. Hope I did it right, let me know.
>
> Thanks.

[-- Attachment #2: Type: text/html, Size: 1348 bytes --]

bug#73258: 31.0.50; w32 drag-n-dropping multiple files is broken

[Eli Zaretskii]

> Date: Fri, 20 Sep 2024 13:20:05 +0200
> From: Cecilio Pardo <cpardo@imayhem.com>
> Cc: 73258@debbugs.gnu.org
> 
> Did you get a chance to review this? 

Not yet.  Will do soon.

bug#73258: 31.0.50; w32 drag-n-dropping multiple files is broken

[Eli Zaretskii]

> Date: Sun, 15 Sep 2024 21:22:32 +0200
> Cc: 73258@debbugs.gnu.org
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> On 15/09/2024 10:25, Eli Zaretskii wrote:
> > We cannot remove or make backward-incompatible changes in a public
> > API.  So removing/renaming w32-handle-dropped-file and/or changing its
> > signature is out of the question.  Can you rewrite the patch such that
> > it keeps this function and its arguments, and just change the
> > implementation to fix the problem?
> >
> > Also, please accompany your changes with a ChangeLog-style description
> > (see CONTRIBUTE for the details), to make the job of installing the
> > changes easier.
> 
> See the attached patch. Hope I did it right, let me know.

This causes the following warning while byte-compiling:

  In w32-dropped-file-to-url:
  term/w32-win.el:129:29: Warning: reference to free variable `window'

And indeed, 'window' is not bound to any value here, and
dnd-handle-multiple-urls does need it to be a valid window.  If I try
drag-and-drop with your patch installed, Emacs signals an error about
'window' being void.  I don't understand how it worked for you.

Also, what about this comment:

  ;; FIXME: is the W32 build capable only of receiving a single file
  ;; from each drop?

I guess it is no longer pertinent and should be deleted?

bug#73258: 31.0.50; w32 drag-n-dropping multiple files is broken

[Cecilio Pardo]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

On 21/09/2024 12:11, Eli Zaretskii wrote:
> This causes the following warning while byte-compiling:
>
>    In w32-dropped-file-to-url:
>    term/w32-win.el:129:29: Warning: reference to free variable `window'
>
> And indeed, 'window' is not bound to any value here, and
> dnd-handle-multiple-urls does need it to be a valid window.  If I try
> drag-and-drop with your patch installed, Emacs signals an error about
> 'window' being void.  I don't understand how it worked for you.
>
> Also, what about this comment:
>
>    ;; FIXME: is the W32 build capable only of receiving a single file
>    ;; from each drop?
>
> I guess it is no longer pertinent and should be deleted?

I messed up when preparing the patch file, I'm sorry.

Here is the correct one.

[-- Attachment #2: 0001-Fix-multifile-drag-n-drop-on-win32.patch --]
[-- Type: text/plain, Size: 2443 bytes --]

From 3c5af14b5c0d956153d83fe9a5041ae897e5b9eb Mon Sep 17 00:00:00 2001
From: Cecilio Pardo <cpardo@imayhem.com>
Date: Sat, 21 Sep 2024 15:30:27 +0200
Subject: [PATCH] Fix multifile drag-n-drop on win32

Pass all dropped files to dnd-handle-multiple-urls
on one call instead on doing multiple calls (Bug#73258).
* lisp/term/w32-win.el (w32-dropped-file-to-url): New function to
convert file name to a url for dnd
(w32-handle-dropped-file): Changed to use w32-dropped-file-to-url
(w32-drag-n-drop): Changed to pass all files to
dnd-handle-multiple-urls
---
 lisp/term/w32-win.el | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index 3c0acf368f4..b57b3dd3bef 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -101,6 +101,13 @@ w32-color-map
 ;;   (princ event))
 
 (defun w32-handle-dropped-file (window file-name)
+  (dnd-handle-multiple-urls
+   window
+   (list
+    (w32-dropped-file-to-url file-name))
+   'private))
+
+(defun w32-dropped-file-to-url (file-name)
   (let ((f (if (eq system-type 'cygwin)
                (cygwin-convert-file-name-from-windows file-name t)
              (subst-char-in-string ?\\ ?/ file-name)))
@@ -117,14 +124,11 @@ w32-handle-dropped-file
                      (split-string (encode-coding-string f coding)
                                    "/")
                      "/")))
-  ;; FIXME: is the W32 build capable only of receiving a single file
-  ;; from each drop?
-  (dnd-handle-multiple-urls window (list (concat
-			                  (if (eq system-type 'cygwin)
-				              "file://"
-			                    "file:")
-			                  file-name))
-                            'private))
+  (concat
+   (if (eq system-type 'cygwin)
+       "file://"
+     "file:")
+   file-name))
 
 (defun w32-drag-n-drop (event &optional new-frame)
   "Edit the files listed in the drag-n-drop EVENT.
@@ -146,8 +150,11 @@ w32-drag-n-drop
       (raise-frame)
       (setq window (selected-window))
 
-      (mapc (apply-partially #'w32-handle-dropped-file window)
-            (car (cdr (cdr event)))))))
+      (dnd-handle-multiple-urls
+       window
+       (mapcar #'w32-dropped-file-to-url
+               (car (cdr (cdr event))))
+       'private))))
 
 (defun w32-drag-n-drop-other-frame (event)
   "Edit the files listed in the drag-n-drop EVENT, in other frames.
-- 
2.35.1.windows.2

bug#73258: 31.0.50; w32 drag-n-dropping multiple files is broken

[Eli Zaretskii]

> Date: Sat, 21 Sep 2024 15:41:03 +0200
> Cc: 73258@debbugs.gnu.org
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> I messed up when preparing the patch file, I'm sorry.
> 
> Here is the correct one.

Thanks, installed on the master branch, and closing the bug.