GNU bug report logs - #62444
[PATCH] erc: Fix "dcc get" flag parsing

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 62444 in the body.
You can then email your comments to 62444 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Daniel Pettersson <daniel <at> dpettersson.net>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] erc: Fix "dcc get" flag parsing
Date: Sat, 25 Mar 2023 16:25:48 +0100

In erc mode when receiving a file with "/dcc get" if the nick or
filename starts with a dash or the filename contains the following
string " -", "/dcc get" is unable to download the file.

Reproduce:
As this is a bit cumbersome to reproduce without mocking files. I
included a patch of erc-dcc-tests where the file name contains a the
string " - ".
Apply the following patch for erc-dcc-tests and run lisp-erc tests.
---
diff --git a/test/lisp/erc/erc-dcc-tests.el b/test/lisp/erc/erc-dcc-tests.el
index bd8a9fc7951..a487f9067cd 100644
--- a/test/lisp/erc/erc-dcc-tests.el
+++ b/test/lisp/erc/erc-dcc-tests.el
@@ -109,7 +109,7 @@ erc-dcc-do-GET-command
                       :parent proc
                       :ip "127.0.0.1"
                       :port "9899"
-                      :file "foo.bin"
+                      :file "foo - .bin"
                       :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 _) "foo - .bin"))
                 ((symbol-function 'erc-dcc-get-file)
                  (lambda (&rest r) (push r calls))))
         (goto-char (point-max))
@@ -134,36 +134,36 @@ 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 foo - .bin")
           (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 "foo - .bin" 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 foo - .bin")
           (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 "foo - .bin" 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 foo - .bin")
           (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 "foo - .bin" 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 foo - .bin -t")
           (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 "foo - .bin" proc))))))))

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

---

Issue present since:
df1e553688b * Accommodate nonstandard turbo file senders in erc-dcc

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]

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"
     - filenames starting with r"-s|t +"
     - filenames with ending  with r" -s|t"

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.
---
 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
+           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))
     (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 ()
+(defun erc-dcc-tests--erc-dcc-do-GET-command (file)
   (with-temp-buffer
     (let* ((proc (start-process "fake" (current-buffer) "sleep" "10"))
            (elt (list :nick "tester!~tester <at> fake.irc"
@@ -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,36 +134,41 @@ 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 " 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 " 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 " 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 " file " -t")
           (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-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"))

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

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

From: "J.P." <jp <at> neverwas.me>
To: Daniel Pettersson <daniel <at> dpettersson.net>
Cc: 62444 <at> debbugs.gnu.org, emacs-erc <at> gnu.org
Subject: Re: bug#62444: [PATCH] erc: Fix "dcc get" flag parsing
Date: Sat, 25 Mar 2023 21:10:26 -0700

[Message part 1 (text/plain, inline)]

Hi Daniel,

Thanks for submitting this report. I haven't gotten around to reviewing
your proposed changes properly but definitely will in the coming days.
For now, all I can offer are a few boring administrative notes.

Daniel Pettersson <daniel <at> dpettersson.net> writes:

> In erc mode when receiving a file with "/dcc get" if the nick or
> filename starts with a dash or the filename contains the following
> string " -", "/dcc get" is unable to download the file.

Indeed, sorry for any inconvenience this might have caused.

> Reproduce:
> As this is a bit cumbersome to reproduce without mocking files. I
> included a patch of erc-dcc-tests where the file name contains a the
> string " - ".
> Apply the following patch for erc-dcc-tests and run lisp-erc tests.
> ---
>
> diff --git a/test/lisp/erc/erc-dcc-tests.el b/test/lisp/erc/erc-dcc-tests.el
> index bd8a9fc7951..a487f9067cd 100644
> --- a/test/lisp/erc/erc-dcc-tests.el
> +++ b/test/lisp/erc/erc-dcc-tests.el
> @@ -109,7 +109,7 @@ erc-dcc-do-GET-command
[...]
>
> ---

This example makes sense, thanks. BTW, in the future, can you save out
your patches with git-format-patch and attach them somehow? This and
other conventions worth noting, such as formatting change-log style
commit messages, are detailed in CONTRIBUTE. As an example, I've
re-attached your patch with a reformatted commit message more along the
lines of what's expected (if you wouldn't mind taking a look).

Also, if you can remember, please add the header:

  X-Debbugs-CC: emacs-erc <at> gnu.org

to any future bug reports (see also `erc-bug').

> Issue present since:
> df1e553688b * Accommodate nonstandard turbo file senders in erc-dcc
>
> 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]
>
> 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"
>      - filenames starting with r"-s|t +"
>      - filenames with ending  with r" -s|t"

Guessing r"..." just means regexp? If not, please clarify.

> 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.

I would tend to agree. Perhaps we ought to go ahead and only make these
-s and -t flags valid in the terminal position. Normally, that'd be a
hassle, likely involving the introduction of a "compat" user option. But
we've deliberately refrained from announcing these DCC features (other
than in the "usage" portion of the Commentary section atop the library
file). So I think a trivially breaking change in a patch version, like
5.5.1, might be doable.

Lastly, I have to mention the dreaded copyright thing because I couldn't
tell from the discussion for bug#57905 whether you ended up filing. If
not and we go with changes resembling those you've proposed, you'll
probably want to do so.

Thanks,
J.P.

[0001-POC-Demo-broken-flags-parsing-in-erc-dcc-do-GET-comm.patch (text/x-patch, attachment)]

[0002-Fix-DCC-GET-flag-parsing-in-erc-dcc.patch (text/x-patch, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: Daniel Pettersson <daniel <at> dpettersson.net>
Cc: 62444 <at> debbugs.gnu.org, emacs-erc <at> gnu.org
Subject: Re: bug#62444: [PATCH] erc: Fix "dcc get" flag parsing
Date: Sun, 26 Mar 2023 20:50:00 -0700

[Message part 1 (text/plain, inline)]

Hi Daniel,

Daniel Pettersson <daniel <at> dpettersson.net> 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, attachment)]

[0001-Add-subcommand-dispatch-facility-to-erc-cmd-HELP.patch (text/x-patch, attachment)]

[0002-Add-subcommand-erc-cmd-HELP-handler-to-erc-dcc.patch (text/x-patch, attachment)]

[0003-Fix-DCC-GET-flag-parsing-in-erc-dcc.patch (text/x-patch, attachment)]

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

From: Daniel Pettersson <daniel <at> dpettersson.net>
To: 62444 <at> debbugs.gnu.org
Subject: Re: bug#62444: [PATCH] erc: Fix "dcc get" flag parsing
Date: Wed, 5 Apr 2023 20:27:33 +0200

[Message part 1 (text/plain, inline)]

Hi J.P.,

Sorry for the delay for some reason the responses bounced my mailbox,
first and foremost I would like to thank you for your patience and
clarity in your response.

> Lastly, I have to mention the dreaded copyright thing because I couldn't
> tell from the discussion for bug#57905 whether you ended up filing. If
> not and we go with changes resembling those you've proposed, you'll
> probably want to do so.

Attached my assignment.

> 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.

I didn't manage to find a case where ?- is allowed, don't really know
where I got that idea from.

> 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?)

I can't say that I am one of the enlightened ones ;).

> 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.)

This is great, reminds me that I have some homework here; coding
standard, reading some more elisp in emacs packages and the bug
reporting.

Sorry about that.

> 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.

I'll also add that I was not really happy with my implementation as it
was far from elegant.

> 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.

I don't have any feedback, looks great to me!

Thanks!

[Pettersson.GNU.EMACS.1875138.pdf (application/pdf, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: Daniel Pettersson <daniel <at> dpettersson.net>
Cc: emacs-erc <at> gnu.org, 62444-done <at> debbugs.gnu.org
Subject: Re: bug#62444: [PATCH] erc: Fix "dcc get" flag parsing
Date: Sat, 08 Apr 2023 15:53:23 -0700

Hi Daniel,

Daniel Pettersson <daniel <at> dpettersson.net> writes:

> Hi J.P.,
>
> Sorry for the delay for some reason the responses bounced my mailbox,
> first and foremost I would like to thank you for your patience and
> clarity in your response.

Not at all, and you're welcome.

>> Lastly, I have to mention the dreaded copyright thing because I couldn't
>> tell from the discussion for bug#57905 whether you ended up filing. If
>> not and we go with changes resembling those you've proposed, you'll
>> probably want to do so.
>
> Attached my assignment.

Thanks. (I would have taken your word for it).

>> 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.)
>
> This is great, reminds me that I have some homework here; coding
> standard, reading some more elisp in emacs packages and the bug
> reporting.
>
> Sorry about that.

No worries. I'm quite the serial offender myself.

>> 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.
>
> I don't have any feedback, looks great to me!

Cool. I've added your changes to master and will probably backport the
bug-fixing portion to 29.1 (or 29.2) once we figure out what to do with
ERC's version number on the release branch.

Thanks, and please consider contributing to ERC again in the future!

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

From: Fernando de Morais <fernandodemorais.jf <at> gmail.com>
To: 62444 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org, daniel <at> dpettersson.net, jp <at> neverwas.me
Subject: Re: bug#62444: [PATCH] erc: Fix "dcc get" flag parsing
Date: Sat, 08 Jul 2023 00:22:54 -0300

Hello J.P. and Daniel,

I don't know if this behaviour is related to the changes discussed in
this bug report, but I can no longer receive files from senders whose
nicknames contains a "|" pipe character.

Example:

	/DCC GET EXAMPLE|Nick file_name.txt

ERC returns in its buffer (followed by an *Apropos* buffer showing up in
another window):

	*** DCC: Nick undefined subcommand. GET, CHAT and LIST are defined.

Thanks in advance for any help.

-- 
Regards,
Fernando de Morais.

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

From: "J.P." <jp <at> neverwas.me>
To: Fernando de Morais <fernandodemorais.jf <at> gmail.com>
Cc: 62444 <at> debbugs.gnu.org, emacs-erc <at> gnu.org, daniel <at> dpettersson.net
Subject: Re: bug#62444: [PATCH] erc: Fix "dcc get" flag parsing
Date: Fri, 07 Jul 2023 21:24:38 -0700

[Message part 1 (text/plain, inline)]

Hi Fernando,

Fernando de Morais <fernandodemorais.jf <at> gmail.com> writes:

> I can no longer receive files from senders whose
> nicknames contains a "|" pipe character.
>
> Example:
>
> 	/DCC GET EXAMPLE|Nick file_name.txt
>
> ERC returns in its buffer (followed by an *Apropos* buffer showing up in
> another window):
>
> 	*** DCC: Nick undefined subcommand. GET, CHAT and LIST are defined.

Thanks for the heads up. This looks like my bad. Would something like
the attached (untried) maybe suffice as a temporary workaround?

[fernando.diff (text/x-patch, attachment)]

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

From: Fernando de Morais <fernandodemorais.jf <at> gmail.com>
To: "J.P." <jp <at> neverwas.me>
Cc: 62444 <at> debbugs.gnu.org, emacs-erc <at> gnu.org, daniel <at> dpettersson.net
Subject: Re: bug#62444: [PATCH] erc: Fix "dcc get" flag parsing
Date: Sat, 08 Jul 2023 09:56:34 -0300

Hello J.P.,

"J.P." <jp <at> neverwas.me> writes:

> Thanks for the heads up. This looks like my bad. Would something like
> the attached (untried) maybe suffice as a temporary workaround?

You're welcome!  I've applied the patch but, unfortunately, the reported
behaviour persists...

Not sure if that's why the changes didn't work, as the patch is related
to a `compat' code for Emacs 28, but I've been following the Emacs
master branch (I should have informed in the previous message).

Thanks!

-- 
Regards,
Fernando de Morais.

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

From: "J.P." <jp <at> neverwas.me>
To: Fernando de Morais <fernandodemorais.jf <at> gmail.com>
Cc: 62444 <at> debbugs.gnu.org, emacs-erc <at> gnu.org, daniel <at> dpettersson.net
Subject: Re: bug#62444: [PATCH] erc: Fix "dcc get" flag parsing
Date: Sat, 08 Jul 2023 07:18:46 -0700

[Message part 1 (text/plain, inline)]

Fernando de Morais <fernandodemorais.jf <at> gmail.com> writes:

> Hello J.P.,
>
> "J.P." <jp <at> neverwas.me> writes:
>
>> Thanks for the heads up. This looks like my bad. Would something like
>> the attached (untried) maybe suffice as a temporary workaround?
>
> You're welcome!  I've applied the patch but, unfortunately, the reported
> behaviour persists...

Gah, right, that was a dumb hack anyway. I shouldn't have suggested it.

> Not sure if that's why the changes didn't work, as the patch is related
> to a `compat' code for Emacs 28, but I've been following the Emacs
> master branch (I should have informed in the previous message).

This one is more like a proper patch, but it's based on the same
(mis?)understanding of the problem, which is slightly worrying. It may
very well be that I'll need to step back and reassess. But if you're
still willing at this juncture, please give this one a go. And if you're
not already doing something like

  find lisp/erc -name \*.elc -delete

before rerunning make, please do, so we can rule out some corner-case
weirdness (if you wouldn't mind). Thanks again.

[0001-5.6-Fix-command-line-parsing-regression-in-erc-cmd-D.patch (text/x-patch, attachment)]

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

From: Fernando de Morais <fernandodemorais.jf <at> gmail.com>
To: "J.P." <jp <at> neverwas.me>
Cc: 62444 <at> debbugs.gnu.org, emacs-erc <at> gnu.org, daniel <at> dpettersson.net
Subject: Re: bug#62444: [PATCH] erc: Fix "dcc get" flag parsing
Date: Sun, 09 Jul 2023 15:02:21 -0300

Hi J.P.,

"J.P." <jp <at> neverwas.me> writes:

> Gah, right, that was a dumb hack anyway. I shouldn't have suggested it.

It's always nice to help test ERC solutions, so don't worry about it!  😄

> But if you're still willing at this juncture, please give this one a
> go. And if you're not already doing something like
>
>   find lisp/erc -name \*.elc -delete
>
> before rerunning make, please do, so we can rule out some corner-case
> weirdness (if you wouldn't mind). Thanks again.

I've applied the patch and tested it: the problem was solved!

Thanks for all the work on ERC!

-- 
Regards,
Fernando de Morais.

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

From: "J.P." <jp <at> neverwas.me>
To: Fernando de Morais <fernandodemorais.jf <at> gmail.com>
Cc: 62444 <at> debbugs.gnu.org, emacs-erc <at> gnu.org, daniel <at> dpettersson.net
Subject: Re: bug#62444: [PATCH] erc: Fix "dcc get" flag parsing
Date: Thu, 13 Jul 2023 19:22:04 -0700

Hi Fernando,

Fernando de Morais <fernandodemorais.jf <at> gmail.com> writes:

> I've applied the patch and tested it: the problem was solved!

Awesome! I've just now installed the patch [1]. Really appreciate the
feedback, as always.

> Thanks for all the work on ERC!

You bet. (Not sure I'm owed much gratitude for cleaning up my own
messes, but I'll take it!)

Cheers,
J.P.

[1] https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b95bb644

Previous Next

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