GNU bug report logs - #29156
25.3; eshell/kill does not understand -<signal>

Previous Next

Package: emacs;

Reported by: Pierre Neidhardt <ambrevar <at> gmail.com>

Date: Sun, 5 Nov 2017 11:32:02 UTC

Severity: normal

Tags: confirmed, easy, fixed

Found in version 25.3

Fixed in version 27.1

Done: Noam Postavsky <npostavs <at> gmail.com>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Sun, 05 Nov 2017 11:32:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Pierre Neidhardt <ambrevar <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 05 Nov 2017 11:32:02 GMT) Full text and rfc822 format available.

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

From: Pierre Neidhardt <ambrevar <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.3; eshell/kill does not understand -<signal>
Date: Sun, 05 Nov 2017 12:31:38 +0100
- emacs -Q
- M-x eshell

~ $ kill -9 emacs
kill: bad pid: -9


In GNU Emacs 25.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.16)
 of 2017-11-05 built on dhiov23k
Windowing system distributor 'The X.Org Foundation', version 11.0.11905000
System Description:	Gentoo Base System release 2.4.1

Configured using:
 'configure --prefix=/usr --build=x86_64-pc-linux-gnu
 --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
 --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
 --localstatedir=/var/lib --disable-dependency-tracking
 --disable-silent-rules --docdir=/usr/share/doc/emacs-25.3
 --htmldir=/usr/share/doc/emacs-25.3/html --libdir=/usr/lib64
 --program-suffix=-emacs-25 --infodir=/usr/share/info/emacs-25
 --localstatedir=/var
 --enable-locallisppath=/etc/emacs:/usr/share/emacs/site-lisp
 --with-gameuser=:gamestat --without-compress-install
 --with-file-notification=inotify --enable-acl --without-dbus
 --without-modules --without-gpm --without-hesiod --without-kerberos
 --without-kerberos5 --with-xml2 --without-selinux --with-gnutls
 --without-wide-int --with-zlib --with-sound=alsa --with-x --without-ns
 --without-gconf --without-gsettings --without-toolkit-scroll-bars
 --with-gif --with-jpeg --with-png --with-rsvg --with-tiff --with-xpm
 --with-imagemagick --with-xft --without-cairo --without-libotf
 --without-m17n-flt --with-x-toolkit=gtk3 --without-xwidgets
 GENTOO_PACKAGE=app-editors/emacs-25.3 'CFLAGS=-march=ivybridge -O2
 -pipe' CPPFLAGS= 'LDFLAGS=-Wl,-O1 -Wl,--as-needed''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND NOTIFY ACL GNUTLS LIBXML2
FREETYPE XFT ZLIB GTK3 X11

Important settings:
  value of $LANG: en_US.utf8
  locale-coding-system: utf-8-unix





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Sun, 05 Nov 2017 12:39:01 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Pierre Neidhardt <ambrevar <at> gmail.com>
Cc: 29156 <at> debbugs.gnu.org
Subject: Re: bug#29156: 25.3; eshell/kill does not understand -<signal>
Date: Sun, 05 Nov 2017 07:38:37 -0500
tags 29156 + confirmed
quit

Pierre Neidhardt <ambrevar <at> gmail.com> writes:

> - emacs -Q
> - M-x eshell
>
> ~ $ kill -9 emacs
> kill: bad pid: -9

This bit converts the "-9" into an integer:

(defun eshell-lisp-command (object &optional args)
   ...
		  ;; if any of the arguments are flagged as numbers
		  ;; waiting for conversion, convert them now
		  (unless (get object 'eshell-no-numeric-conversions)
		    (while args
		      (let ((arg (car args)))
			(if (and (stringp arg)
				 (> (length arg) 0)
				 (not (text-property-not-all
				       0 (length arg) 'number t arg)))
			    (setcar args (string-to-number arg))))
		      (setq args (cdr args))))
  ...

Whereas this bit expects "-9" as a string:

(defun eshell/kill (&rest args)
  ...
      (when (stringp arg)
        (cond
         ((string-match "\\`-[[:digit:]]+\\'" arg)
          (setq signum (abs (string-to-number arg))))
         ((string-match "\\`-\\([[:upper:]]+\\|[[:lower:]]+\\)\\'" arg)
          (setq signum (abs (string-to-number arg)))))
  ...

Using "-SIGKILL" doesn't work either, because eshell/kill calls
`string-to-number' on it, giving 0.  Apparently nobody ever actually
used this feature before.




Added tag(s) confirmed. Request was from Noam Postavsky <npostavs <at> users.sourceforge.net> to control <at> debbugs.gnu.org. (Sun, 05 Nov 2017 12:39:02 GMT) Full text and rfc822 format available.

Added tag(s) easy. Request was from Noam Postavsky <npostavs <at> users.sourceforge.net> to control <at> debbugs.gnu.org. (Thu, 23 Nov 2017 02:41:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Sat, 17 Mar 2018 00:22:02 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: 29156 <at> debbugs.gnu.org
Cc: Eric Skoglund <eric <at> pagefault.se>, Pierre Neidhardt <ambrevar <at> gmail.com>
Subject: Re: bug#29156: 25.3;
 eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill
 handle -<signal> and -<SIGNALNAME>
Date: Fri, 16 Mar 2018 20:21:46 -0400
[Message part 1 (text/plain, inline)]
[forwarding to list]

[Message part 2 (message/rfc822, inline)]
From: Eric Skoglund <eric <at> pagefault.se>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Subject: Re: bug#29156: 25.3; eshell/kill does not understand -<signal>
Date: Fri, 16 Mar 2018 15:01:32 +0100
--=-=-=
Content-Type: text/plain


First time contributor. Here's a patch that allows eshell/kill to handle
both the -9 case and the -SIGKILL case.

--=-=-=
Content-Type: text/x-patch
Content-Disposition: inline; filename=0001-Make-eshell-kill-handle-signal-and-SIGNALNAME.patch
Content-Description: eshell/kill patch

From 2789c82b27cfac175d6d04260db78a54a2f26b01 Mon Sep 17 00:00:00 2001
From: Eric Skoglund <eric <at> pagefault.se>
Date: Fri, 16 Mar 2018 14:49:56 +0100
Subject: [PATCH] Make eshell/kill handle -<signal> and -<SIGNALNAME>

     * lisp/eshell/esh-proc.el: Handle -<signal> and -<SIGNALNAME>
---
 lisp/eshell/esh-proc.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index b3bd7a7245..6dab6636b0 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -178,12 +178,12 @@ eshell/kill
          ((string-match "\\`-[[:digit:]]+\\'" arg)
           (setq signum (abs (string-to-number arg))))
          ((string-match "\\`-\\([[:upper:]]+\\|[[:lower:]]+\\)\\'" arg)
-          (setq signum (abs (string-to-number arg)))))
+          (setq signum (make-symbol (substring arg 1 (length arg))))))
         (setq args (cdr args))))
     (while args
       (let ((arg (if (eshell-processp (car args))
                      (process-id (car args))
-                   (car args))))
+                   (string-to-number (car args)))))
         (when arg
           (cond
            ((null arg)
-- 
2.13.6


--=-=-=--

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Sat, 17 Mar 2018 00:35:02 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: 29156 <at> debbugs.gnu.org
Cc: Eric Skoglund <eric <at> pagefault.se>, Pierre Neidhardt <ambrevar <at> gmail.com>
Subject: Re: bug#29156: 25.3;
 eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill
 handle -<signal> and -<SIGNALNAME>
Date: Fri, 16 Mar 2018 20:34:25 -0400
> From: Eric Skoglund <eric <at> pagefault.se>

> First time contributor. Here's a patch that allows eshell/kill to handle
> both the -9 case and the -SIGKILL case.

Thanks!

>      * lisp/eshell/esh-proc.el: Handle -<signal> and -<SIGNALNAME>

Minor formatting nitpick, you should have the function name here:

    * lisp/eshell/esh-proc.el (eshell-kill): ...

More importantly, could you explain a bit how your change works/why it's
correct?

>           ((string-match "\\`-\\([[:upper:]]+\\|[[:lower:]]+\\)\\'" arg)
> -          (setq signum (abs (string-to-number arg)))))
> +          (setq signum (make-symbol (substring arg 1 (length arg))))))

Not sure this `make-symbol' call, should it rather be `intern'?  (Maybe
we should update signal-process take a string as well a symbol.)

>          (setq args (cdr args))))
>      (while args
>        (let ((arg (if (eshell-processp (car args))
>                       (process-id (car args))
> -                   (car args))))
> +                   (string-to-number (car args)))))

I think the args have already been converted to numbers, or did you mean
to also add a (put 'eshell/kill 'eshell-no-numeric-conversions t)?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Sat, 17 Mar 2018 08:59:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 29156 <at> debbugs.gnu.org, eric <at> pagefault.se, ambrevar <at> gmail.com
Subject: Re: bug#29156: 25.3;
 eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill
 handle -<signal> and -<SIGNALNAME>
Date: Sat, 17 Mar 2018 10:58:32 +0200
> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Fri, 16 Mar 2018 20:34:25 -0400
> Cc: Eric Skoglund <eric <at> pagefault.se>, Pierre Neidhardt <ambrevar <at> gmail.com>
> 
> >           ((string-match "\\`-\\([[:upper:]]+\\|[[:lower:]]+\\)\\'" arg)
> > -          (setq signum (abs (string-to-number arg)))))
> > +          (setq signum (make-symbol (substring arg 1 (length arg))))))
> 
> Not sure this `make-symbol' call, should it rather be `intern'?

Yes, I think intern is better here.

> (Maybe we should update signal-process take a string as well a
> symbol.)

Possibly.

Btw, the doc string of eshell/kill should be updated to reflect the
fact we now support symbolic names of Unix signals.  Also, NEWS and
the Eshell manual should be updated.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Sat, 17 Mar 2018 14:44:01 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: 29156 <at> debbugs.gnu.org
Cc: Eric Skoglund <eric <at> pagefault.se>, Pierre Neidhardt <ambrevar <at> gmail.com>
Subject: Re: bug#29156: 25.3;
 eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill
 handle -<signal> and -<SIGNALNAME>
Date: Sat, 17 Mar 2018 10:43:04 -0400
[Message part 1 (text/plain, inline)]
[forwarding to list, please use "Reply All" to keep 29156 <at> debbugs.gnu.org on Cc]

[Message part 2 (message/rfc822, inline)]
From: Eric Skoglund <eric <at> pagefault.se>
To: Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#29156: 25.3; eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill handle -<signal> and -<SIGNALNAME>
Date: Sat, 17 Mar 2018 15:11:41 +0100
Thanks for the review!

> More importantly, could you explain a bit how your change works/why it's
> correct?

I'm guessing you mean in the commit message and not here in the bug
report right?

> Not sure this `make-symbol' call, should it rather be `intern'?  (Maybe
> we should update signal-process take a string as well a symbol.)

Saw that Eli also agreed, for future reference when do we want to use
`make-symbol` vs `intern`?

> I think the args have already been converted to numbers, or did you mean
> to also add a (put 'eshell/kill 'eshell-no-numeric-conversions t)?

Yes indeed. I'm unsure how I managed to lose that in git...

// Eric

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Sat, 17 Mar 2018 14:55:01 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: 29156 <at> debbugs.gnu.org
Cc: Eric Skoglund <eric <at> pagefault.se>, Pierre Neidhardt <ambrevar <at> gmail.com>
Subject: Re: bug#29156: 25.3;
 eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill
 handle -<signal> and -<SIGNALNAME>
Date: Sat, 17 Mar 2018 10:54:46 -0400
> From: Eric Skoglund <eric <at> pagefault.se>

>> More importantly, could you explain a bit how your change works/why it's
>> correct?
>
> I'm guessing you mean in the commit message and not here in the bug
> report right?

Either, both :)

(If you send a new patch with the explanation in the commit message,
it's automatically in the bug report as well anyway)

>> Not sure [about] this `make-symbol' call, should it rather be
>> `intern'?  (Maybe we should update signal-process take a string as
>> well a symbol.)
>
> Saw that Eli also agreed, for future reference when do we want to use
> `make-symbol` vs `intern`?

Usually, `make-symbol' is only for macros, where you want a symbol that
is not `eq' to any other.  Sometimes it's handy for making a unique
object at run-time.  In this case, since we only care about the symbol
name, it doesn't really matter, it's just a bit surprising to see it
because it's usually a mistake in a non-macro context.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Sat, 17 Mar 2018 18:09:01 GMT) Full text and rfc822 format available.

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

From: Eric Skoglund <eric <at> pagefault.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29156 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> gmail.com>,
 ambrevar <at> gmail.com
Subject: Re: bug#29156: 25.3;
 eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill
 handle -<signal> and -<SIGNALNAME>
Date: Sat, 17 Mar 2018 18:43:29 +0100
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Noam Postavsky <npostavs <at> gmail.com>
>> Date: Fri, 16 Mar 2018 20:34:25 -0400
>> Cc: Eric Skoglund <eric <at> pagefault.se>, Pierre Neidhardt <ambrevar <at> gmail.com>
>> 
>> >           ((string-match "\\`-\\([[:upper:]]+\\|[[:lower:]]+\\)\\'" arg)
>> > -          (setq signum (abs (string-to-number arg)))))
>> > +          (setq signum (make-symbol (substring arg 1 (length arg))))))
>> 
>> Not sure this `make-symbol' call, should it rather be `intern'?
>
> Yes, I think intern is better here.
>
>> (Maybe we should update signal-process take a string as well a
>> symbol.)
>
> Possibly.
>
> Btw, the doc string of eshell/kill should be updated to reflect the
> fact we now support symbolic names of Unix signals.  Also, NEWS and
> the Eshell manual should be updated.

Here is an updated patch which hopefully fixes all the issues with the
previous.

[0001-Make-eshell-kill-handle-signal-and-SIGNALNAME.patch (text/x-patch, inline)]
From 54bca6f5420d80abeaf9c1da5b73d50112eaab71 Mon Sep 17 00:00:00 2001
From: Eric Skoglund <eric <at> pagefault.se>
Date: Fri, 16 Mar 2018 14:49:56 +0100
Subject: [PATCH] Make eshell/kill handle -<signal> and -<SIGNALNAME>

     * lisp/eshell/esh-proc.el (eshell/kill):
     Handle the argument parsing and numeric conversion in function
     in order to parse -signal and -SIGNALNAME correctly.
---
 doc/misc/eshell.texi    | 2 +-
 etc/NEWS                | 5 +++++
 lisp/eshell/esh-proc.el | 9 ++++++---
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 80077e5ccd..bda6159488 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -330,7 +330,7 @@ Built-ins
 @item kill
 @cmindex kill
 Kill processes.  Takes a PID or a process object and an optional
-signal specifier.
+signal specifier which can either be a number or a signal name.
 
 @item listify
 @cmindex listify
diff --git a/etc/NEWS b/etc/NEWS
index b6c4157384..bfd9a33040 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -273,6 +273,11 @@ To restore the old behavior, use
 *** The function 'shell-uniquify-list' has been renamed from
 'eshell-uniqify-list'.
 
+*** The function eshell/kill is now able to handle signal switches.
+Previously eshell/kill would fail if provided a kill signal to send to the
+process. It now handles both numeric signals and the string version of
+the signals.
+
 ** Pcomplete
 *** The function 'pcomplete-uniquify-list' has been renamed from
 'pcomplete-uniqify-list'.
diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index b3bd7a7245..1fe4b324a6 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -167,7 +167,8 @@ eshell/jobs
 (defun eshell/kill (&rest args)
   "Kill processes.
 Usage: kill [-<signal>] <pid>|<process> ...
-Accepts PIDs and process objects."
+Accepts PIDs and process objects.  Optionally accept signals
+and signal names."
   ;; If the first argument starts with a dash, treat it as the signal
   ;; specifier.
   (let ((signum 'SIGINT))
@@ -178,12 +179,12 @@ eshell/kill
          ((string-match "\\`-[[:digit:]]+\\'" arg)
           (setq signum (abs (string-to-number arg))))
          ((string-match "\\`-\\([[:upper:]]+\\|[[:lower:]]+\\)\\'" arg)
-          (setq signum (abs (string-to-number arg)))))
+          (setq signum (intern (substring arg 1 (length arg))))))
         (setq args (cdr args))))
     (while args
       (let ((arg (if (eshell-processp (car args))
                      (process-id (car args))
-                   (car args))))
+                   (string-to-number (car args)))))
         (when arg
           (cond
            ((null arg)
@@ -198,6 +199,8 @@ eshell/kill
       (setq args (cdr args))))
   nil)
 
+(put 'eshell/kill 'eshell-no-numeric-conversions t)
+
 (defun eshell-read-process-name (prompt)
   "Read the name of a process from the minibuffer, using completion.
 The prompt will be set to PROMPT."
-- 
2.13.6

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

// Eric

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Sat, 17 Mar 2018 19:20:02 GMT) Full text and rfc822 format available.

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

From: Eric Skoglund <eric <at> pagefault.se>
To: 29156 <at> debbugs.gnu.org,  29156 <at> https:
Cc: Noam Postavsky <npostavs <at> gmail.com>, Pierre Neidhardt <ambrevar <at> gmail.com>
Subject: Re: bug#29156: 25.3;
 eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill
 handle -<signal> and -<SIGNALNAME>
Date: Sat, 17 Mar 2018 20:19:27 +0100
Noam Postavsky <npostavs <at> gmail.com> writes:
>>
>> Saw that Eli also agreed, for future reference when do we want to use
>> `make-symbol` vs `intern`?
>
> Usually, `make-symbol' is only for macros, where you want a symbol that
> is not `eq' to any other.  Sometimes it's handy for making a unique
> object at run-time.  In this case, since we only care about the symbol
> name, it doesn't really matter, it's just a bit surprising to see it
> because it's usually a mistake in a non-macro context.

Thanks for the clarification!

PS. Not sure why I'm not seeing my replies in debbugs-gnu so sorry for the
extra work in forwarding them. Tried changing the To header to the bug
mail adress, let's se if that works.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Sat, 17 Mar 2018 19:34:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eric Skoglund <eric <at> pagefault.se>
Cc: 29156 <at> debbugs.gnu.org, npostavs <at> gmail.com, ambrevar <at> gmail.com
Subject: Re: bug#29156: 25.3;
 eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill
 handle -<signal> and -<SIGNALNAME>
Date: Sat, 17 Mar 2018 21:33:48 +0200
> From: Eric Skoglund <eric <at> pagefault.se>
> Cc: Noam Postavsky <npostavs <at> gmail.com>, 29156 <at> debbugs.gnu.org,  ambrevar <at> gmail.com
> Date: Sat, 17 Mar 2018 18:43:29 +0100
> 
>      * lisp/eshell/esh-proc.el (eshell/kill):
>      Handle the argument parsing and numeric conversion in function
>      in order to parse -signal and -SIGNALNAME correctly.
> ---
>  doc/misc/eshell.texi    | 2 +-
>  etc/NEWS                | 5 +++++
>  lisp/eshell/esh-proc.el | 9 ++++++---
>  3 files changed, 12 insertions(+), 4 deletions(-)

The commit log should mention changes to all the files you are
changing.

> +*** The function eshell/kill is now able to handle signal switches.
> +Previously eshell/kill would fail if provided a kill signal to send to the
> +process. It now handles both numeric signals and the string version of
          ^^
Two spaces between sentences, please.  Also, I'd rephrase the last
sentence:

  It now accepts signals specified either by name or by its number.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Sat, 17 Mar 2018 21:21:02 GMT) Full text and rfc822 format available.

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

From: Eric Skoglund <eric <at> pagefault.se>
To: Eli Zaretskii <eliz <at> gnu.org>, 29156 <at> https:
Cc: 29156 <at> debbugs.gnu.org, npostavs <at> gmail.com, ambrevar <at> gmail.com
Subject: Re: bug#29156: 25.3;
 eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill
 handle -<signal> and -<SIGNALNAME>
Date: Sat, 17 Mar 2018 22:20:34 +0100
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:

> Two spaces between sentences, please.  Also, I'd rephrase the last
> sentence:
>
>   It now accepts signals specified either by name or by its number.
>
> Thanks.

Here'sthe latest patch, thanks for the patience!

[0001-Make-eshell-kill-handle-signal-and-SIGNALNAME.patch (text/x-patch, inline)]
From a02f03aa683afc3c1b41eaaaa8bf274a7727747f Mon Sep 17 00:00:00 2001
From: Eric Skoglund <eric <at> pagefault.se>
Date: Fri, 16 Mar 2018 14:49:56 +0100
Subject: [PATCH] Make eshell/kill handle -<signal> and -<SIGNALNAME>

     * lisp/eshell/esh-proc.el (eshell/kill):
     Handle the argument parsing and numeric conversion in function
     in order to parse -signal and -SIGNALNAME correctly.
     * doc/misc/eshell.texi (kill): Update docs to reflect new
     function behaviour
     * etc/NEWS: Mention new eshell/kill behaviour
---
 doc/misc/eshell.texi    | 2 +-
 etc/NEWS                | 5 +++++
 lisp/eshell/esh-proc.el | 9 ++++++---
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 80077e5ccd..bda6159488 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -330,7 +330,7 @@ Built-ins
 @item kill
 @cmindex kill
 Kill processes.  Takes a PID or a process object and an optional
-signal specifier.
+signal specifier which can either be a number or a signal name.
 
 @item listify
 @cmindex listify
diff --git a/etc/NEWS b/etc/NEWS
index b6c4157384..7df6eb4387 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -273,6 +273,11 @@ To restore the old behavior, use
 *** The function 'shell-uniquify-list' has been renamed from
 'eshell-uniqify-list'.
 
+*** The function eshell/kill is now able to handle signal switches.
+Previously eshell/kill would fail if provided a kill signal to send to the
+process.  It now accepts signals specified either by name or by its number.
+
+
 ** Pcomplete
 *** The function 'pcomplete-uniquify-list' has been renamed from
 'pcomplete-uniqify-list'.
diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index b3bd7a7245..1fe4b324a6 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -167,7 +167,8 @@ eshell/jobs
 (defun eshell/kill (&rest args)
   "Kill processes.
 Usage: kill [-<signal>] <pid>|<process> ...
-Accepts PIDs and process objects."
+Accepts PIDs and process objects.  Optionally accept signals
+and signal names."
   ;; If the first argument starts with a dash, treat it as the signal
   ;; specifier.
   (let ((signum 'SIGINT))
@@ -178,12 +179,12 @@ eshell/kill
          ((string-match "\\`-[[:digit:]]+\\'" arg)
           (setq signum (abs (string-to-number arg))))
          ((string-match "\\`-\\([[:upper:]]+\\|[[:lower:]]+\\)\\'" arg)
-          (setq signum (abs (string-to-number arg)))))
+          (setq signum (intern (substring arg 1 (length arg))))))
         (setq args (cdr args))))
     (while args
       (let ((arg (if (eshell-processp (car args))
                      (process-id (car args))
-                   (car args))))
+                   (string-to-number (car args)))))
         (when arg
           (cond
            ((null arg)
@@ -198,6 +199,8 @@ eshell/kill
       (setq args (cdr args))))
   nil)
 
+(put 'eshell/kill 'eshell-no-numeric-conversions t)
+
 (defun eshell-read-process-name (prompt)
   "Read the name of a process from the minibuffer, using completion.
 The prompt will be set to PROMPT."
-- 
2.13.6

[Message part 3 (text/plain, inline)]
// Eric

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Fri, 23 Mar 2018 01:06:01 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eric Skoglund <eric <at> pagefault.se>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29156 <at> debbugs.gnu.org, ambrevar <at> gmail.com
Subject: Re: bug#29156: 25.3;
 eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill
 handle -<signal> and -<SIGNALNAME>
Date: Thu, 22 Mar 2018 21:05:33 -0400
Thanks, do you have copyright assignment for Emacs? (the change is small
enough to install regardless, I'm asking just to know if it should be
marked as a tiny change).

A couple more minor nitpicks:

>      * doc/misc/eshell.texi (kill): Update docs to reflect new
>      function behaviour
>      * etc/NEWS: Mention new eshell/kill behaviour

Missing periods here.

> +          (setq signum (intern (substring arg 1 (length arg))))))

That could be just (substring arg 1)

P.S. don't worry about the forwarding thing, it happens often enough
that I've written an Emacs command to mostly automate it.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Fri, 23 Mar 2018 06:04:02 GMT) Full text and rfc822 format available.

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

From: Eric Skoglund <eric <at> pagefault.se>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29156 <at> debbugs.gnu.org, ambrevar <at> gmail.com
Subject: Re: bug#29156: 25.3;
 eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill
 handle -<signal> and -<SIGNALNAME>
Date: Fri, 23 Mar 2018 07:03:55 +0100
[Message part 1 (text/plain, inline)]
Noam Postavsky <npostavs <at> gmail.com> writes:

> Thanks, do you have copyright assignment for Emacs? (the change is small
> enough to install regardless, I'm asking just to know if it should be
> marked as a tiny change).

Yes it was completed on 2016-12-05 when I had grand plans to contribute
to emacs (maybe thoose plans can be fullfilled now :)).

>
> A couple more minor nitpicks:
>
>>      * doc/misc/eshell.texi (kill): Update docs to reflect new
>>      function behaviour
>>      * etc/NEWS: Mention new eshell/kill behaviour
>
> Missing periods here.
>
>> +          (setq signum (intern (substring arg 1 (length arg))))))
>
> That could be just (substring arg 1)
>

Thanks!

> P.S. don't worry about the forwarding thing, it happens often enough
> that I've written an Emacs command to mostly automate it.

Ah great :)

Here is the latest patch version. Thanks for all the help for a first
time contributor!

[0001-Make-eshell-kill-handle-signal-and-SIGNALNAME.patch (text/x-patch, inline)]
From 7588028d623a8312a7a359f2be4ba84426d0619e Mon Sep 17 00:00:00 2001
From: Eric Skoglund <eric <at> pagefault.se>
Date: Fri, 16 Mar 2018 14:49:56 +0100
Subject: [PATCH] Make eshell/kill handle -<signal> and -<SIGNALNAME>

     * lisp/eshell/esh-proc.el (eshell/kill):
     Handle the argument parsing and numeric conversion in function
     in order to parse -signal and -SIGNALNAME correctly.
     * doc/misc/eshell.texi (kill): Update docs to reflect new
     function behaviour.
     * etc/NEWS: Mention new eshell/kill behaviour.
---
 doc/misc/eshell.texi    | 2 +-
 etc/NEWS                | 5 +++++
 lisp/eshell/esh-proc.el | 9 ++++++---
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 80077e5ccd..bda6159488 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -330,7 +330,7 @@ Built-ins
 @item kill
 @cmindex kill
 Kill processes.  Takes a PID or a process object and an optional
-signal specifier.
+signal specifier which can either be a number or a signal name.
 
 @item listify
 @cmindex listify
diff --git a/etc/NEWS b/etc/NEWS
index b6c4157384..7df6eb4387 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -273,6 +273,11 @@ To restore the old behavior, use
 *** The function 'shell-uniquify-list' has been renamed from
 'eshell-uniqify-list'.
 
+*** The function eshell/kill is now able to handle signal switches.
+Previously eshell/kill would fail if provided a kill signal to send to the
+process.  It now accepts signals specified either by name or by its number.
+
+
 ** Pcomplete
 *** The function 'pcomplete-uniquify-list' has been renamed from
 'pcomplete-uniqify-list'.
diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index b3bd7a7245..a7855d81db 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -167,7 +167,8 @@ eshell/jobs
 (defun eshell/kill (&rest args)
   "Kill processes.
 Usage: kill [-<signal>] <pid>|<process> ...
-Accepts PIDs and process objects."
+Accepts PIDs and process objects.  Optionally accept signals
+and signal names."
   ;; If the first argument starts with a dash, treat it as the signal
   ;; specifier.
   (let ((signum 'SIGINT))
@@ -178,12 +179,12 @@ eshell/kill
          ((string-match "\\`-[[:digit:]]+\\'" arg)
           (setq signum (abs (string-to-number arg))))
          ((string-match "\\`-\\([[:upper:]]+\\|[[:lower:]]+\\)\\'" arg)
-          (setq signum (abs (string-to-number arg)))))
+          (setq signum (intern (substring arg 1)))))
         (setq args (cdr args))))
     (while args
       (let ((arg (if (eshell-processp (car args))
                      (process-id (car args))
-                   (car args))))
+                   (string-to-number (car args)))))
         (when arg
           (cond
            ((null arg)
@@ -198,6 +199,8 @@ eshell/kill
       (setq args (cdr args))))
   nil)
 
+(put 'eshell/kill 'eshell-no-numeric-conversions t)
+
 (defun eshell-read-process-name (prompt)
   "Read the name of a process from the minibuffer, using completion.
 The prompt will be set to PROMPT."
-- 
2.13.6


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29156; Package emacs. (Sun, 25 Mar 2018 15:35:01 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eric Skoglund <eric <at> pagefault.se>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29156 <at> debbugs.gnu.org, ambrevar <at> gmail.com
Subject: Re: bug#29156: 25.3;
 eshell/kill does not understand -<signal>, [PATCH] Make eshell/kill
 handle -<signal> and -<SIGNALNAME>
Date: Sun, 25 Mar 2018 11:34:34 -0400
tags 29156 fixed
close 29156 27.1
quit

Eric Skoglund <eric <at> pagefault.se> writes:

> Noam Postavsky <npostavs <at> gmail.com> writes:
>
>> Thanks, do you have copyright assignment for Emacs?

> Yes it was completed on 2016-12-05 when I had grand plans to contribute
> to emacs (maybe thoose plans can be fullfilled now :)).

Plans fullfilled :) [1: 1be6a21fd8] 

> Here is the latest patch version. Thanks for all the help for a first
> time contributor!

Thanks for your contribution!

[1: 1be6a21fd8]: 2018-03-25 11:20:20 -0400
  Make eshell/kill handle -<signal> and -<SIGNALNAME> (Bug#29156)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=1be6a21fd8b5ade67f7f69f964331aa570623683




Added tag(s) fixed. Request was from Noam Postavsky <npostavs <at> gmail.com> to control <at> debbugs.gnu.org. (Sun, 25 Mar 2018 15:35:02 GMT) Full text and rfc822 format available.

bug marked as fixed in version 27.1, send any further explanations to 29156 <at> debbugs.gnu.org and Pierre Neidhardt <ambrevar <at> gmail.com> Request was from Noam Postavsky <npostavs <at> gmail.com> to control <at> debbugs.gnu.org. (Sun, 25 Mar 2018 15:35:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 23 Apr 2018 11:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 6 years and 3 days ago.

Previous Next


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