Re: bug#62444: [PATCH] erc: Fix "dcc get" flag parsing

"J.P." <[email protected]>
Newsgroups gmane.emacs.erc.general
Message-ID <[email protected]> 
Hi Daniel,

Daniel Pettersson <[email protected]> writes:

> Proposed patch:
> erc: Fix "dcc get" flag parsing
>
> When nick or filename starts with `?-' or filename contains the
> following string " -", "dcc get" is unable determine nick/filename and
> fails to download file.
>
> Flag parsing rules is kept as is:
> [flag] nick [flag] filename [flag]

As you've mentioned elsewhere, we should probably try to reduce the
complexity WRT these flag positions. I agree that "terminal" is best,
but since we're already nominally in bed with "initial", how 'bout we
try sticking with that for now?

> Flags have the highest priority when parsing the arguments to dcc
> get. This is not an complete fix as dcc will fail on:
>      - nicks "-s" and "-t"

AFAIK, nicknames can't normally begin with a ?-, and attempting to
procure one like that will earn you a

  :mercury.libera.chat 432 testing123 -testing123 :Erroneous nickname

or similar. Of course, if you know of any popular servers where this
isn't the case, please share.

>      - filenames starting with r"-s|t +"

In the attached changes, which iterate on yours and which I'd like your
comments on, I've tried adding a familiar "end of options" separator,
i.e., "--", to cover this case. Given the unlikelihood of such
collisions, I think it's worth the occasional inconvenience.

>      - filenames with ending  with r" -s|t"

In the interest of preserving some symmetry with DCC SEND, which quotes
its outgoing arguments, I think erc-dcc should parse its own input line
rather than rely on the treatment from `erc-extract-command-from-line'.
This approach seems to work in cursory trials, but a few complications
arise when it comes to completion (also present in ERC <5.5.), although
there are workarounds. (How's your pcomplete-fu?)

> An more robust solution and cleaner implementation would be possible
> if flag position was limited to the end of the arguments list.
>
> This would also make it easier to implement pcomplete for flags as well.

Agreed. See above.

Alas, the following are just mechanical, style-related nits. Ignore them
if you wish, but please see the attached patches for a reprise of your
initial proposal with the changes I've outlined applied atop. (The first
two patches are just thrown in for convenience but ultimately
unrelated.)

> ---
>  lisp/erc/erc-dcc.el            | 36 +++++++++++++++++++++++-----------
>  test/lisp/erc/erc-dcc-tests.el | 27 ++++++++++++++-----------
>  2 files changed, 41 insertions(+), 22 deletions(-)
>
> diff --git a/lisp/erc/erc-dcc.el b/lisp/erc/erc-dcc.el
> index 4c557e0e0f9..d7c685e9413 100644
> --- a/lisp/erc/erc-dcc.el
> +++ b/lisp/erc/erc-dcc.el
> @@ -504,18 +504,32 @@ erc-dcc-do-CLOSE-command
>             ?n (erc-extract-nick (plist-get ret :nick))))))
>      t))
>
> -(defun erc-dcc-do-GET-command (proc nick &rest file)
> -  "Do a DCC GET command.  NICK is the person who is sending the file.
> -FILE is the filename.  If FILE is split into multiple arguments,
> -re-join the arguments, separated by a space.
> +(defun erc-dcc-do-GET-command (proc &rest args)
> +  "Do a DCC GET command.
> +ARGS are expected to contain:
> +  nick     The person who is sending the file.
> +  filename The filename to be downloaded. Can be split into multiple arguments
                                            ^1                                 2^

Two spaces between sentences. Not my preference either, but them's the
rules. Also, the doc-string fill column is a parsimonious 65. I often
ignore it for preformatted, "tabular" stuff like this and thus take it
up to 70 or so, but 78 is likely pushing it.

> +           which is then joined by a space.
> +  flags    \"-t\" sets `:turbo' see `erc-dcc-list'
> +           \"-s\" sets `:secure' see `erc-dcc-list'
> +ARGS are parsed as follows:
> +  [flag] nick [flag] filename [flag]
>  PROC is the server process."
> -  (let* ((args (seq-group-by (lambda (s) (eq ?- (aref s 0))) (cons nick file)))
> -         (flags (prog1 (cdr (assq t args))
> -                  (setq args (cdr (assq nil args))
> -                        nick (pop args)
> -                        file (and args (mapconcat #'identity args " ")))))
> -         (elt (erc-dcc-member :nick nick :type 'GET :file file))
> -         (filename (or file (plist-get elt :file) "unknown")))
> +  (let ((possible-flags '("-s" "-t"))
> +        flags nick elt possible-files filename)
> +    ;; Get flags between get and nick
> +    (while (seq-contains-p possible-flags (car args) 'equal)
> +      (setq flags (cons (pop args) flags)))
> +    (setq nick (or (pop args) ""))
> +    ;; Get flags between nick and filename
> +    (while (seq-contains-p possible-flags (car args) 'equal)
> +      (setq flags (cons (pop args) flags)))
> +    ;; Get flags after filename
> +    (setq args (reverse args))
> +    (while (seq-contains-p possible-flags (car args) 'equal)
> +      (setq flags (cons (pop args) flags)))
> +    (setq filename (or (mapconcat #'identity (reverse args) " ") "")
> +          elt (erc-dcc-member :nick nick :type 'GET :file filename))

Some of the above, such as (setq foo (cons x foo)) instead of `push' and
`seq-contains-p' instead of `member', might distract a few readers. I
don't really care, personally.

>      (if elt
>          (let* ((file (read-file-name
>                        (format-prompt "Local filename"
> diff --git a/test/lisp/erc/erc-dcc-tests.el b/test/lisp/erc/erc-dcc-tests.el
> index bd8a9fc7951..f21463bb5a0 100644
> --- a/test/lisp/erc/erc-dcc-tests.el
> +++ b/test/lisp/erc/erc-dcc-tests.el
> @@ -100,7 +100,7 @@ erc-dcc-handle-ctcp-send--base
>  (ert-deftest erc-dcc-handle-ctcp-send--turbo ()
>    (erc-dcc-tests--dcc-handle-ctcp-send t))
>
[...]
> +
> +(ert-deftest erc-dcc-do-GET-command ()
> +  (erc-dcc-tests--erc-dcc-do-GET-command "foo.bin")
> +  (erc-dcc-tests--erc-dcc-do-GET-command "foo - file.bin")
> +  (erc-dcc-tests--erc-dcc-do-GET-command "foo -t file.bin"))

This is nice. I should have done this originally.

>  (defun erc-dcc-tests--pcomplete-common (test-fn)
>    (with-current-buffer (get-buffer-create "*erc-dcc-do-GET-command*")

As mentioned, I've taken a slightly different tack WRT parsing based on
the presence of pre-quoted args. Please check it out, give feedback, and
by all means iterate.

Thanks!
0000-v0-v1.diff (text/x-patch, 12.2 KB) 
From 76baf9840010ba181547bbed5d86a29171922054 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email protected]>
Date: Sun, 26 Mar 2023 19:40:58 -0700
Subject: [PATCH 0/3] *** NOT A PATCH ***

*** BLURB HERE ***

Daniel Pettersson (1):
  Fix DCC GET flag parsing in erc-dcc

F. Jason Park (2):
  Add subcommand dispatch facility to erc-cmd-HELP
  Add subcommand erc-cmd-HELP handler to erc-dcc

 lisp/erc/erc-dcc.el            | 65 ++++++++++++++++++++++++++-------
 lisp/erc/erc.el                |  7 +++-
 test/lisp/erc/erc-dcc-tests.el | 67 +++++++++++++++++++++++++++-------
 3 files changed, 110 insertions(+), 29 deletions(-)

Interdiff:
diff --git a/lisp/erc/erc-dcc.el b/lisp/erc/erc-dcc.el
index d7c685e9413..5406369c62f 100644
--- a/lisp/erc/erc-dcc.el
+++ b/lisp/erc/erc-dcc.el
@@ -43,7 +43,7 @@
 ;;  /dcc chat nick - Either accept pending chat offer from nick, or offer
 ;;                   DCC chat to nick
 ;;  /dcc close type [nick] - Close DCC connection (SEND/GET/CHAT) with nick
-;;  /dcc get [-t][-s] nick [file] - Accept DCC offer from nick
+;;  /dcc get [-t][-s] nick [--] file - Accept DCC offer from nick
 ;;  /dcc list - List all DCC offers/connections
 ;;  /dcc send nick file - Offer DCC SEND to nick
 
@@ -389,12 +389,18 @@ erc-dcc-get-default-directory
   :type '(choice (const :value nil :tag "Default directory") directory))
 
 ;;;###autoload
-(defun erc-cmd-DCC (cmd &rest args)
+(defun erc-cmd-DCC (line &rest compat-args)
   "Parser for /dcc command.
 This figures out the dcc subcommand and calls the appropriate routine to
 handle it.  The function dispatched should be named \"erc-dcc-do-FOO-command\",
 where FOO is one of CLOSE, GET, SEND, LIST, CHAT, etc."
-  (when cmd
+  (let (cmd args)
+    ;; Called as library function (i.e., not directly as /dcc)
+    (if compat-args
+        (setq cmd line
+              args compat-args)
+      (setq args (delete "" (split-string-shell-command line))
+            cmd (pop args)))
     (let ((fn (intern-soft (concat "erc-dcc-do-" (upcase cmd) "-command"))))
       (if fn
           (apply fn erc-server-process args)
@@ -404,8 +410,17 @@ erc-cmd-DCC
         (apropos "erc-dcc-do-.*-command")
         t))))
 
+(put 'erc-cmd-DCC 'do-not-parse-args t)
 (autoload 'pcomplete-erc-all-nicks "erc-pcomplete")
 
+(defun erc-dcc--cmd-help (&rest args)
+  (describe-function
+   (or (and args (intern-soft (concat "erc-dcc-do-"
+                                      (upcase (car args)) "-command")))
+       'erc-cmd-DCC)))
+
+(put 'erc-cmd-DCC 'erc--cmd-help #'erc-dcc--cmd-help)
+
 ;;;###autoload
 (defun pcomplete/erc-mode/DCC ()
   "Provide completion for the /DCC command."
@@ -430,15 +445,20 @@ pcomplete/erc-mode/DCC
                       (eq (plist-get elt :type) 'GET))
                     erc-dcc-list)))
      ('send (pcomplete-erc-all-nicks))))
+  (when (equal "get" (downcase (pcomplete-arg 'first 1)))
+    (pcomplete-opt "-"))
   (pcomplete-here
    (pcase (intern (downcase (pcomplete-arg 'first 1)))
-     ('get (mapcar (lambda (elt) (plist-get elt :file))
+     ('get (mapcar (lambda (elt)
+                     (combine-and-quote-strings (list (plist-get elt :file))))
                    (cl-remove-if-not
                     (lambda (elt)
                       (and (eq (plist-get elt :type) 'GET)
                            (erc-nick-equal-p (erc-extract-nick
                                               (plist-get elt :nick))
-                                             (pcomplete-arg 1))))
+                                             (pcase (pcomplete-arg 1)
+                                               ("--" (pcomplete-arg 2))
+                                               (v v)))))
                     erc-dcc-list)))
      ('close (mapcar #'erc-dcc-nick
                      (cl-remove-if-not
@@ -505,31 +525,34 @@ erc-dcc-do-CLOSE-command
     t))
 
 (defun erc-dcc-do-GET-command (proc &rest args)
-  "Do a DCC GET command.
-ARGS are expected to contain:
+  "Perform a DCC GET command.
+Recognize input conforming to the following usage syntax:
+
+  /DCC GET [-t|-s] nick [--] filename
+
   nick     The person who is sending the file.
-  filename The filename to be downloaded. Can be split into multiple arguments
-           which is then joined by a space.
+  filename The filename to be downloaded.  Can be split into multiple
+           arguments that are then joined by a space.
   flags    \"-t\" sets `:turbo' see `erc-dcc-list'
            \"-s\" sets `:secure' see `erc-dcc-list'
-ARGS are parsed as follows:
-  [flag] nick [flag] filename [flag]
-PROC is the server process."
-  (let ((possible-flags '("-s" "-t"))
-        flags nick elt possible-files filename)
-    ;; Get flags between get and nick
-    (while (seq-contains-p possible-flags (car args) 'equal)
-      (setq flags (cons (pop args) flags)))
-    (setq nick (or (pop args) ""))
-    ;; Get flags between nick and filename
-    (while (seq-contains-p possible-flags (car args) 'equal)
-      (setq flags (cons (pop args) flags)))
-    ;; Get flags after filename
-    (setq args (reverse args))
-    (while (seq-contains-p possible-flags (car args) 'equal)
-      (setq flags (cons (pop args) flags)))
-    (setq filename (or (mapconcat #'identity (reverse args) " ") "")
-          elt (erc-dcc-member :nick nick :type 'GET :file filename))
+           \"--\" indicates end of options
+           All of which are optional.
+
+Expect PROC to be the server process and ARGS to contain
+everything after the subcommand \"GET\" in the usage description
+above."
+  ;; Despite the advertised syntax above, we currently respect flags
+  ;; in these positions: [flag] nick [flag] filename [flag]
+  (let* ((trailing (and-let* ((trailing (member "--" args)))
+                     (setq args (butlast args (length trailing)))
+                     (cdr trailing)))
+         (args (seq-group-by (lambda (s) (eq ?- (aref s 0))) args))
+         (flags (prog1 (cdr (assq t args))
+                  (setq args (nconc (cdr (assq nil args)) trailing))))
+         (nick (pop args))
+         (file (and args (mapconcat #'identity args " ")))
+         (elt (erc-dcc-member :nick nick :type 'GET :file file))
+         (filename (or file (plist-get elt :file) "unknown")))
     (if elt
         (let* ((file (read-file-name
                       (format-prompt "Local filename"
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 69bdb5d71b1..60aa26579c5 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -3203,7 +3203,7 @@ erc-cmd-CTCP
     (erc-send-ctcp-message nick str)
     t))
 
-(defun erc-cmd-HELP (&optional func)
+(defun erc-cmd-HELP (&optional func &rest rest)
   "Popup help information.
 
 If FUNC contains a valid function or variable, help about that
@@ -3236,6 +3236,11 @@ erc-cmd-HELP
                           nil)))))
         (if sym
             (cond
+             ((progn (autoloadp (symbol-function sym))
+                     (autoload-do-load (symbol-function sym))
+                     nil))
+             ((get sym 'erc--cmd-help)
+              (apply (get sym 'erc--cmd-help) rest))
              ((boundp sym) (describe-variable sym))
              ((fboundp sym) (describe-function sym))
              (t nil))
diff --git a/test/lisp/erc/erc-dcc-tests.el b/test/lisp/erc/erc-dcc-tests.el
index f21463bb5a0..fed86eff2c5 100644
--- a/test/lisp/erc/erc-dcc-tests.el
+++ b/test/lisp/erc/erc-dcc-tests.el
@@ -100,7 +100,7 @@ erc-dcc-handle-ctcp-send--base
 (ert-deftest erc-dcc-handle-ctcp-send--turbo ()
   (erc-dcc-tests--dcc-handle-ctcp-send t))
 
-(defun erc-dcc-tests--erc-dcc-do-GET-command (file)
+(defun erc-dcc-tests--erc-dcc-do-GET-command (file &optional sep)
   (with-temp-buffer
     (let* ((proc (start-process "fake" (current-buffer) "sleep" "10"))
            (elt (list :nick "[email protected]"
@@ -134,7 +134,7 @@ erc-dcc-tests--erc-dcc-do-GET-command
         (ert-info ("No turbo")
           (should-not (plist-member elt :turbo))
           (goto-char erc-input-marker)
-          (insert "/dcc GET tester " file)
+          (insert "/dcc GET tester " (or sep "") (prin1-to-string file))
           (erc-send-current-line)
           (should-not (plist-member (car erc-dcc-list) :turbo))
           (should (equal (pop calls) (list elt file proc))))
@@ -142,7 +142,7 @@ erc-dcc-tests--erc-dcc-do-GET-command
         (ert-info ("Arg turbo in pos 2")
           (should-not (plist-member elt :turbo))
           (goto-char erc-input-marker)
-          (insert "/dcc GET -t tester " file)
+          (insert "/dcc GET -t tester " (or sep "") (prin1-to-string file))
           (erc-send-current-line)
           (should (eq t (plist-get (car erc-dcc-list) :turbo)))
           (should (equal (pop calls) (list elt file proc))))
@@ -151,7 +151,7 @@ erc-dcc-tests--erc-dcc-do-GET-command
           (setq elt (plist-put elt :turbo nil)
                 erc-dcc-list (list elt))
           (goto-char erc-input-marker)
-          (insert "/dcc GET tester -t " file)
+          (insert "/dcc GET tester -t " (or sep "") (prin1-to-string file))
           (erc-send-current-line)
           (should (eq t (plist-get (car erc-dcc-list) :turbo)))
           (should (equal (pop calls) (list elt file proc))))
@@ -160,17 +160,18 @@ erc-dcc-tests--erc-dcc-do-GET-command
           (setq elt (plist-put elt :turbo nil)
                 erc-dcc-list (list elt))
           (goto-char erc-input-marker)
-          (insert "/dcc GET tester " file " -t")
+          (insert "/dcc GET tester " (prin1-to-string file) " -t" (or sep ""))
           (erc-send-current-line)
-          (should (eq t (plist-get (car erc-dcc-list) :turbo)))
-          (should (equal (pop calls) (list elt file proc))))))))
+          (should (eq (if sep nil t) (plist-get (car erc-dcc-list) :turbo)))
+          (should (equal (pop calls) (if sep nil (list elt file proc)))))))))
 
 (ert-deftest erc-dcc-do-GET-command ()
   (erc-dcc-tests--erc-dcc-do-GET-command "foo.bin")
   (erc-dcc-tests--erc-dcc-do-GET-command "foo - file.bin")
-  (erc-dcc-tests--erc-dcc-do-GET-command "foo -t file.bin"))
+  (erc-dcc-tests--erc-dcc-do-GET-command "foo -t file.bin")
+  (erc-dcc-tests--erc-dcc-do-GET-command "-t" "-- "))
 
-(defun erc-dcc-tests--pcomplete-common (test-fn)
+(defun erc-dcc-tests--pcomplete-common (test-fn &optional file)
   (with-current-buffer (get-buffer-create "*erc-dcc-do-GET-command*")
     (let* ((inhibit-message noninteractive)
            (proc (start-process "fake" (current-buffer) "sleep" "10"))
@@ -180,7 +181,7 @@ erc-dcc-tests--pcomplete-common
                       :parent proc
                       :ip "127.0.0.1"
                       :port "9899"
-                      :file "foo.bin"
+                      :file (or file "foo.bin")
                       :size 1405135128))
            ;;
            erc-accidental-paste-threshold-seconds
@@ -216,6 +217,20 @@ pcomplete/erc-mode/DCC--get-basic
        (beginning-of-line)
        (should (search-forward "/dcc get tester foo.bin" nil t))))))
 
+(ert-deftest pcomplete/erc-mode/DCC--get-quoted ()
+  (erc-dcc-tests--pcomplete-common
+   (lambda ()
+     (insert "/dcc get ")
+     (call-interactively #'completion-at-point)
+     (save-excursion
+       (beginning-of-line)
+       (should (search-forward "/dcc get tester" nil t)))
+     (call-interactively #'completion-at-point)
+     (save-excursion
+       (beginning-of-line)
+       (should (search-forward "/dcc get tester \"foo bar.bin\"" nil t))))
+   "foo bar.bin"))
+
 (ert-deftest pcomplete/erc-mode/DCC--get-1flag ()
   (erc-dcc-tests--pcomplete-common
    (lambda ()
@@ -287,4 +302,23 @@ pcomplete/erc-mode/DCC--get-2flags-reverse
        (beginning-of-line)
        (should (search-forward "/dcc get -t -s tester foo.bin" nil t))))))
 
+(ert-deftest pcomplete/erc-mode/DCC--get-sep ()
+  (erc-dcc-tests--pcomplete-common
+   (lambda ()
+     (insert "/dcc get ")
+     (call-interactively #'completion-at-point)
+     (save-excursion
+       (beginning-of-line)
+       (should (search-forward "/dcc get tester" nil t)))
+     (insert "-")
+     (call-interactively #'completion-at-point)
+     (save-excursion
+       (beginning-of-line)
+       (should (search-forward "/dcc get tester -- " nil t)))
+     (call-interactively #'completion-at-point)
+     (save-excursion
+       (beginning-of-line)
+       (should (search-forward "/dcc get tester -- -t" nil t))))
+   "-t"))
+
 ;;; erc-dcc-tests.el ends here
-- 
2.39.2
0001-Add-subcommand-dispatch-facility-to-erc-cmd-HELP.patch (text/x-patch, 1.4 KB) 
From e4aa6d281e492edb1ca8a6f3c50586fdffe7867e Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email protected]>
Date: Sat, 25 Mar 2023 09:13:40 -0700
Subject: [PATCH 1/3] Add subcommand dispatch facility to erc-cmd-HELP

* lisp/erc/erc.el (erc-cmd-HELP): Change signature by adding &rest
parameter.  Attempt to autoload symbol.  Look for symbol property
`erc--cmd-help' and, if found, assume it's a function and call it with
&rest args.
---
 lisp/erc/erc.el | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 69bdb5d71b1..60aa26579c5 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -3203,7 +3203,7 @@ erc-cmd-CTCP
     (erc-send-ctcp-message nick str)
     t))
 
-(defun erc-cmd-HELP (&optional func)
+(defun erc-cmd-HELP (&optional func &rest rest)
   "Popup help information.
 
 If FUNC contains a valid function or variable, help about that
@@ -3236,6 +3236,11 @@ erc-cmd-HELP
                           nil)))))
         (if sym
             (cond
+             ((progn (autoloadp (symbol-function sym))
+                     (autoload-do-load (symbol-function sym))
+                     nil))
+             ((get sym 'erc--cmd-help)
+              (apply (get sym 'erc--cmd-help) rest))
              ((boundp sym) (describe-variable sym))
              ((fboundp sym) (describe-function sym))
              (t nil))
-- 
2.39.2
0002-Add-subcommand-erc-cmd-HELP-handler-to-erc-dcc.patch (text/x-patch, 1.1 KB) 
From c287eeaa4dd5f6ec592fdc4138492a034723d398 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email protected]>
Date: Sat, 25 Mar 2023 09:13:40 -0700
Subject: [PATCH 2/3] Add subcommand erc-cmd-HELP handler to erc-dcc

* lisp/erc/erc-dcc.el (erc-cmd-DCC): Elect to tokenize line specially.
(erc-dcc--cmd-help): Add help handler.
---
 lisp/erc/erc-dcc.el | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lisp/erc/erc-dcc.el b/lisp/erc/erc-dcc.el
index 4c557e0e0f9..05eced6a391 100644
--- a/lisp/erc/erc-dcc.el
+++ b/lisp/erc/erc-dcc.el
@@ -404,8 +404,17 @@ erc-cmd-DCC
         (apropos "erc-dcc-do-.*-command")
         t))))
 
+(put 'erc-cmd-DCC 'do-not-parse-args t)
 (autoload 'pcomplete-erc-all-nicks "erc-pcomplete")
 
+(defun erc-dcc--cmd-help (&rest args)
+  (describe-function
+   (or (and args (intern-soft (concat "erc-dcc-do-"
+                                      (upcase (car args)) "-command")))
+       'erc-cmd-DCC)))
+
+(put 'erc-cmd-DCC 'erc--cmd-help #'erc-dcc--cmd-help)
+
 ;;;###autoload
 (defun pcomplete/erc-mode/DCC ()
   "Provide completion for the /DCC command."
-- 
2.39.2
0003-Fix-DCC-GET-flag-parsing-in-erc-dcc.patch (text/x-patch, 11.5 KB) 
From 76baf9840010ba181547bbed5d86a29171922054 Mon Sep 17 00:00:00 2001
From: Daniel Pettersson <[email protected]>
Date: Sat, 25 Mar 2023 16:25:48 +0100
Subject: [PATCH 3/3] Fix DCC GET flag parsing in erc-dcc

* lisp/erc/erc-dcc.el (erc-cmd-DCC): Elect to tokenize line specially.
(pcomplete/erc-mode/DCC): Quote file names when suggesting.  Account
for double-hyphen end-of-options separator.
(erc-dcc-do-GET-command): Simplify signature, subsuming NICK in
variadic args, now ARGS instead of FILE, which changes the arity
from (2 . many) to (1 . many).  Explain usage in doc string.  Honor an
optional separator, "--", if present.  (Bug#62444)
* test/lisp/erc/erc-dcc-tests.el (erc-dcc-do-GET-command): Call new
parameterized helper with various flags/file combinations.
(erc-dcc-tests--erc-dcc-do-GET-command): New fixture function.
(pcomplete/erc-mode/DCC--get-quoted,
pcomplete/erc-mode/DCC--get-sep): New tests.
---
 lisp/erc/erc-dcc.el            | 56 +++++++++++++++++++++-------
 test/lisp/erc/erc-dcc-tests.el | 67 +++++++++++++++++++++++++++-------
 2 files changed, 95 insertions(+), 28 deletions(-)

diff --git a/lisp/erc/erc-dcc.el b/lisp/erc/erc-dcc.el
index 05eced6a391..5406369c62f 100644
--- a/lisp/erc/erc-dcc.el
+++ b/lisp/erc/erc-dcc.el
@@ -43,7 +43,7 @@
 ;;  /dcc chat nick - Either accept pending chat offer from nick, or offer
 ;;                   DCC chat to nick
 ;;  /dcc close type [nick] - Close DCC connection (SEND/GET/CHAT) with nick
-;;  /dcc get [-t][-s] nick [file] - Accept DCC offer from nick
+;;  /dcc get [-t][-s] nick [--] file - Accept DCC offer from nick
 ;;  /dcc list - List all DCC offers/connections
 ;;  /dcc send nick file - Offer DCC SEND to nick
 
@@ -389,12 +389,18 @@ erc-dcc-get-default-directory
   :type '(choice (const :value nil :tag "Default directory") directory))
 
 ;;;###autoload
-(defun erc-cmd-DCC (cmd &rest args)
+(defun erc-cmd-DCC (line &rest compat-args)
   "Parser for /dcc command.
 This figures out the dcc subcommand and calls the appropriate routine to
 handle it.  The function dispatched should be named \"erc-dcc-do-FOO-command\",
 where FOO is one of CLOSE, GET, SEND, LIST, CHAT, etc."
-  (when cmd
+  (let (cmd args)
+    ;; Called as library function (i.e., not directly as /dcc)
+    (if compat-args
+        (setq cmd line
+              args compat-args)
+      (setq args (delete "" (split-string-shell-command line))
+            cmd (pop args)))
     (let ((fn (intern-soft (concat "erc-dcc-do-" (upcase cmd) "-command"))))
       (if fn
           (apply fn erc-server-process args)
@@ -439,15 +445,20 @@ pcomplete/erc-mode/DCC
                       (eq (plist-get elt :type) 'GET))
                     erc-dcc-list)))
      ('send (pcomplete-erc-all-nicks))))
+  (when (equal "get" (downcase (pcomplete-arg 'first 1)))
+    (pcomplete-opt "-"))
   (pcomplete-here
    (pcase (intern (downcase (pcomplete-arg 'first 1)))
-     ('get (mapcar (lambda (elt) (plist-get elt :file))
+     ('get (mapcar (lambda (elt)
+                     (combine-and-quote-strings (list (plist-get elt :file))))
                    (cl-remove-if-not
                     (lambda (elt)
                       (and (eq (plist-get elt :type) 'GET)
                            (erc-nick-equal-p (erc-extract-nick
                                               (plist-get elt :nick))
-                                             (pcomplete-arg 1))))
+                                             (pcase (pcomplete-arg 1)
+                                               ("--" (pcomplete-arg 2))
+                                               (v v)))))
                     erc-dcc-list)))
      ('close (mapcar #'erc-dcc-nick
                      (cl-remove-if-not
@@ -513,16 +524,33 @@ erc-dcc-do-CLOSE-command
            ?n (erc-extract-nick (plist-get ret :nick))))))
     t))
 
-(defun erc-dcc-do-GET-command (proc nick &rest file)
-  "Do a DCC GET command.  NICK is the person who is sending the file.
-FILE is the filename.  If FILE is split into multiple arguments,
-re-join the arguments, separated by a space.
-PROC is the server process."
-  (let* ((args (seq-group-by (lambda (s) (eq ?- (aref s 0))) (cons nick file)))
+(defun erc-dcc-do-GET-command (proc &rest args)
+  "Perform a DCC GET command.
+Recognize input conforming to the following usage syntax:
+
+  /DCC GET [-t|-s] nick [--] filename
+
+  nick     The person who is sending the file.
+  filename The filename to be downloaded.  Can be split into multiple
+           arguments that are then joined by a space.
+  flags    \"-t\" sets `:turbo' see `erc-dcc-list'
+           \"-s\" sets `:secure' see `erc-dcc-list'
+           \"--\" indicates end of options
+           All of which are optional.
+
+Expect PROC to be the server process and ARGS to contain
+everything after the subcommand \"GET\" in the usage description
+above."
+  ;; Despite the advertised syntax above, we currently respect flags
+  ;; in these positions: [flag] nick [flag] filename [flag]
+  (let* ((trailing (and-let* ((trailing (member "--" args)))
+                     (setq args (butlast args (length trailing)))
+                     (cdr trailing)))
+         (args (seq-group-by (lambda (s) (eq ?- (aref s 0))) args))
          (flags (prog1 (cdr (assq t args))
-                  (setq args (cdr (assq nil args))
-                        nick (pop args)
-                        file (and args (mapconcat #'identity args " ")))))
+                  (setq args (nconc (cdr (assq nil args)) trailing))))
+         (nick (pop args))
+         (file (and args (mapconcat #'identity args " ")))
          (elt (erc-dcc-member :nick nick :type 'GET :file file))
          (filename (or file (plist-get elt :file) "unknown")))
     (if elt
diff --git a/test/lisp/erc/erc-dcc-tests.el b/test/lisp/erc/erc-dcc-tests.el
index bd8a9fc7951..fed86eff2c5 100644
--- a/test/lisp/erc/erc-dcc-tests.el
+++ b/test/lisp/erc/erc-dcc-tests.el
@@ -100,7 +100,7 @@ erc-dcc-handle-ctcp-send--base
 (ert-deftest erc-dcc-handle-ctcp-send--turbo ()
   (erc-dcc-tests--dcc-handle-ctcp-send t))
 
-(ert-deftest erc-dcc-do-GET-command ()
+(defun erc-dcc-tests--erc-dcc-do-GET-command (file &optional sep)
   (with-temp-buffer
     (let* ((proc (start-process "fake" (current-buffer) "sleep" "10"))
            (elt (list :nick "[email protected]"
@@ -109,7 +109,7 @@ erc-dcc-do-GET-command
                       :parent proc
                       :ip "127.0.0.1"
                       :port "9899"
-                      :file "foo.bin"
+                      :file file
                       :size 1405135128))
            (erc-dcc-list (list elt))
            ;;
@@ -124,7 +124,7 @@ erc-dcc-do-GET-command
             erc-server-current-nick "dummy")
       (set-process-query-on-exit-flag proc nil)
       (cl-letf (((symbol-function 'read-file-name)
-                 (lambda (&rest _) "foo.bin"))
+                 (lambda (&rest _) file))
                 ((symbol-function 'erc-dcc-get-file)
                  (lambda (&rest r) (push r calls))))
         (goto-char (point-max))
@@ -134,38 +134,44 @@ erc-dcc-do-GET-command
         (ert-info ("No turbo")
           (should-not (plist-member elt :turbo))
           (goto-char erc-input-marker)
-          (insert "/dcc GET tester foo.bin")
+          (insert "/dcc GET tester " (or sep "") (prin1-to-string file))
           (erc-send-current-line)
           (should-not (plist-member (car erc-dcc-list) :turbo))
-          (should (equal (pop calls) (list elt "foo.bin" proc))))
+          (should (equal (pop calls) (list elt file proc))))
 
         (ert-info ("Arg turbo in pos 2")
           (should-not (plist-member elt :turbo))
           (goto-char erc-input-marker)
-          (insert "/dcc GET -t tester foo.bin")
+          (insert "/dcc GET -t tester " (or sep "") (prin1-to-string file))
           (erc-send-current-line)
           (should (eq t (plist-get (car erc-dcc-list) :turbo)))
-          (should (equal (pop calls) (list elt "foo.bin" proc))))
+          (should (equal (pop calls) (list elt file proc))))
 
         (ert-info ("Arg turbo in pos 4")
           (setq elt (plist-put elt :turbo nil)
                 erc-dcc-list (list elt))
           (goto-char erc-input-marker)
-          (insert "/dcc GET tester -t foo.bin")
+          (insert "/dcc GET tester -t " (or sep "") (prin1-to-string file))
           (erc-send-current-line)
           (should (eq t (plist-get (car erc-dcc-list) :turbo)))
-          (should (equal (pop calls) (list elt "foo.bin" proc))))
+          (should (equal (pop calls) (list elt file proc))))
 
         (ert-info ("Arg turbo in pos 6")
           (setq elt (plist-put elt :turbo nil)
                 erc-dcc-list (list elt))
           (goto-char erc-input-marker)
-          (insert "/dcc GET tester foo.bin -t")
+          (insert "/dcc GET tester " (prin1-to-string file) " -t" (or sep ""))
           (erc-send-current-line)
-          (should (eq t (plist-get (car erc-dcc-list) :turbo)))
-          (should (equal (pop calls) (list elt "foo.bin" proc))))))))
+          (should (eq (if sep nil t) (plist-get (car erc-dcc-list) :turbo)))
+          (should (equal (pop calls) (if sep nil (list elt file proc)))))))))
+
+(ert-deftest erc-dcc-do-GET-command ()
+  (erc-dcc-tests--erc-dcc-do-GET-command "foo.bin")
+  (erc-dcc-tests--erc-dcc-do-GET-command "foo - file.bin")
+  (erc-dcc-tests--erc-dcc-do-GET-command "foo -t file.bin")
+  (erc-dcc-tests--erc-dcc-do-GET-command "-t" "-- "))
 
-(defun erc-dcc-tests--pcomplete-common (test-fn)
+(defun erc-dcc-tests--pcomplete-common (test-fn &optional file)
   (with-current-buffer (get-buffer-create "*erc-dcc-do-GET-command*")
     (let* ((inhibit-message noninteractive)
            (proc (start-process "fake" (current-buffer) "sleep" "10"))
@@ -175,7 +181,7 @@ erc-dcc-tests--pcomplete-common
                       :parent proc
                       :ip "127.0.0.1"
                       :port "9899"
-                      :file "foo.bin"
+                      :file (or file "foo.bin")
                       :size 1405135128))
            ;;
            erc-accidental-paste-threshold-seconds
@@ -211,6 +217,20 @@ pcomplete/erc-mode/DCC--get-basic
        (beginning-of-line)
        (should (search-forward "/dcc get tester foo.bin" nil t))))))
 
+(ert-deftest pcomplete/erc-mode/DCC--get-quoted ()
+  (erc-dcc-tests--pcomplete-common
+   (lambda ()
+     (insert "/dcc get ")
+     (call-interactively #'completion-at-point)
+     (save-excursion
+       (beginning-of-line)
+       (should (search-forward "/dcc get tester" nil t)))
+     (call-interactively #'completion-at-point)
+     (save-excursion
+       (beginning-of-line)
+       (should (search-forward "/dcc get tester \"foo bar.bin\"" nil t))))
+   "foo bar.bin"))
+
 (ert-deftest pcomplete/erc-mode/DCC--get-1flag ()
   (erc-dcc-tests--pcomplete-common
    (lambda ()
@@ -282,4 +302,23 @@ pcomplete/erc-mode/DCC--get-2flags-reverse
        (beginning-of-line)
        (should (search-forward "/dcc get -t -s tester foo.bin" nil t))))))
 
+(ert-deftest pcomplete/erc-mode/DCC--get-sep ()
+  (erc-dcc-tests--pcomplete-common
+   (lambda ()
+     (insert "/dcc get ")
+     (call-interactively #'completion-at-point)
+     (save-excursion
+       (beginning-of-line)
+       (should (search-forward "/dcc get tester" nil t)))
+     (insert "-")
+     (call-interactively #'completion-at-point)
+     (save-excursion
+       (beginning-of-line)
+       (should (search-forward "/dcc get tester -- " nil t)))
+     (call-interactively #'completion-at-point)
+     (save-excursion
+       (beginning-of-line)
+       (should (search-forward "/dcc get tester -- -t" nil t))))
+   "-t"))
+
 ;;; erc-dcc-tests.el ends here
-- 
2.39.2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.