all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#24555: [PATCH] Remove unused variable `command-debug-status'
@ 2016-09-28 12:39 Philippe Vaucher
  2016-09-28 14:58 ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Vaucher @ 2016-09-28 12:39 UTC (permalink / raw
  To: 24555


[-- Attachment #1.1: Type: text/plain, Size: 273 bytes --]

Hello,

For information, I did the procedure for copyright assignment to Emacs and
it is complete.

This patch removes the variable `command-debug-status', which really seems
to be unused since a long time to me (that's what "git -G
Vcommand-debug-status" says).

Philippe

[-- Attachment #1.2: Type: text/html, Size: 372 bytes --]

[-- Attachment #2: 0001-Remove-unused-variable-command-debug-status.patch --]
[-- Type: text/x-patch, Size: 2021 bytes --]

From eae6374e5a3f052c55e4e9e8b821539b6f1900aa Mon Sep 17 00:00:00 2001
From: Philippe Vaucher <philippe.vaucher@gmail.com>
Date: Wed, 28 Sep 2016 12:14:28 +0200
Subject: [PATCH] Remove unused variable `command-debug-status'

---
 doc/lispref/debugging.texi | 12 ------------
 src/callint.c              |  6 ------
 2 files changed, 18 deletions(-)

diff --git a/doc/lispref/debugging.texi b/doc/lispref/debugging.texi
index 2f83b40..f4e63ae 100644
--- a/doc/lispref/debugging.texi
+++ b/doc/lispref/debugging.texi
@@ -644,18 +644,6 @@ the debugger.
 This function is used only by the debugger.
 @end defun
 
-@defvar command-debug-status
-This variable records the debugging status of the current interactive
-command.  Each time a command is called interactively, this variable is
-bound to @code{nil}.  The debugger can set this variable to leave
-information for future debugger invocations during the same command
-invocation.
-
-The advantage of using this variable rather than an ordinary global
-variable is that the data will never carry over to a subsequent command
-invocation.
-@end defvar
-
 @defun backtrace-frame frame-number
 The function @code{backtrace-frame} is intended for use in Lisp
 debuggers.  It returns information about what computation is happening
diff --git a/src/callint.c b/src/callint.c
index 053ee6c..ccb5c6a 100644
--- a/src/callint.c
+++ b/src/callint.c
@@ -928,12 +928,6 @@ Maximum length of the history list is determined by the value
 of `history-length', which see.  */);
   Vcommand_history = Qnil;
 
-  DEFVAR_LISP ("command-debug-status", Vcommand_debug_status,
-	       doc: /* Debugging status of current interactive command.
-Bound each time `call-interactively' is called;
-may be set by the debugger as a reminder for itself.  */);
-  Vcommand_debug_status = Qnil;
-
   DEFVAR_LISP ("mark-even-if-inactive", Vmark_even_if_inactive,
 	       doc: /* Non-nil means you can use the mark even when inactive.
 This option makes a difference in Transient Mark mode.
-- 
2.10.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-09-28 12:39 bug#24555: [PATCH] Remove unused variable `command-debug-status' Philippe Vaucher
@ 2016-09-28 14:58 ` Eli Zaretskii
  2016-09-28 16:14   ` Stefan Monnier
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2016-09-28 14:58 UTC (permalink / raw
  To: Philippe Vaucher; +Cc: Stefan Monnier, 24555

> From: Philippe Vaucher <philippe.vaucher@gmail.com>
> Date: Wed, 28 Sep 2016 14:39:02 +0200
> 
> For information, I did the procedure for copyright assignment to Emacs and it is complete.

Indeed, your assignment is on file.

> This patch removes the variable `command-debug-status', which really seems to be unused since a long time
> to me (that's what "git -G Vcommand-debug-status" says).

Thanks for bringing this up.

I looked into the history of this variable, and found that it was
still supported in Emacs 24.5, and was removed during development of
Emacs 25 by this commit:

  commit 0e4857b7d84f958f66e726ed57b824427b272681
  Author: Stefan Monnier <monnier@iro.umontreal.ca>
  Date:   Tue May 27 20:09:14 2014 -0400

      * src/callint.c (Ffuncall_interactively): New function.
      (Qfuncall_interactively): New var.
      (Qcall_interactively): Remove.
      (Fcall_interactively): Use it.
      (syms_of_callint): Defsubr it.
      * lisp/subr.el (internal--funcall-interactively): New.
      (internal--call-interactively): Remove.
      (called-interactively-p): Detect funcall-interactively instead of
      call-interactively.
      * lisp/simple.el (repeat-complex-command): Use funcall-interactively.
      (repeat-complex-command--called-interactively-skip): Remove.

As you see, the commit log doesn't mention the removal of the
variable.  Stefan, was it removed on purpose?  If so, can you tell
why?

Thanks.





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-09-28 14:58 ` Eli Zaretskii
@ 2016-09-28 16:14   ` Stefan Monnier
  2016-09-28 17:24     ` Philippe Vaucher
  2016-09-28 20:05     ` Eli Zaretskii
  0 siblings, 2 replies; 26+ messages in thread
From: Stefan Monnier @ 2016-09-28 16:14 UTC (permalink / raw
  To: Eli Zaretskii; +Cc: Philippe Vaucher, 24555

> As you see, the commit log doesn't mention the removal of the
> variable.  Stefan, was it removed on purpose?  If so, can you tell
> why?

Hmm... I can't remember making such a conscious choice, no.
Must have been an oversight on my part.


        Stefan





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-09-28 16:14   ` Stefan Monnier
@ 2016-09-28 17:24     ` Philippe Vaucher
  2016-09-28 20:11       ` Eli Zaretskii
  2016-09-28 20:05     ` Eli Zaretskii
  1 sibling, 1 reply; 26+ messages in thread
From: Philippe Vaucher @ 2016-09-28 17:24 UTC (permalink / raw
  To: Stefan Monnier; +Cc: 24555

> > As you see, the commit log doesn't mention the removal of the
> > variable.  Stefan, was it removed on purpose?  If so, can you tell
> > why?
>
> Hmm... I can't remember making such a conscious choice, no.
> Must have been an oversight on my part.

What is/was the use of `command-debug-status' ? its docstring does not
really help figure much what data was stored there and what it was
used for.

Philippe





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-09-28 16:14   ` Stefan Monnier
  2016-09-28 17:24     ` Philippe Vaucher
@ 2016-09-28 20:05     ` Eli Zaretskii
  2016-09-29  0:12       ` Stefan Monnier
  1 sibling, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2016-09-28 20:05 UTC (permalink / raw
  To: Stefan Monnier; +Cc: philippe.vaucher, 24555

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Philippe Vaucher <philippe.vaucher@gmail.com>,  24555@debbugs.gnu.org
> Date: Wed, 28 Sep 2016 12:14:24 -0400
> 
> > As you see, the commit log doesn't mention the removal of the
> > variable.  Stefan, was it removed on purpose?  If so, can you tell
> > why?
> 
> Hmm... I can't remember making such a conscious choice, no.
> Must have been an oversight on my part.

That's what I thought.

So restoring the specbind call that was lost in that commit is TRT, do
you agree?





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-09-28 17:24     ` Philippe Vaucher
@ 2016-09-28 20:11       ` Eli Zaretskii
  2016-09-29  0:16         ` Stefan Monnier
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2016-09-28 20:11 UTC (permalink / raw
  To: Philippe Vaucher; +Cc: monnier, 24555

> From: Philippe Vaucher <philippe.vaucher@gmail.com>
> Date: Wed, 28 Sep 2016 19:24:41 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, 24555@debbugs.gnu.org
> 
> What is/was the use of `command-debug-status' ? its docstring does not
> really help figure much what data was stored there and what it was
> used for.

Which parts of the documentation are not clear?

In a nutshell, it's a variable provided for storing arbitrary data
that persists for the duration of the current interactive command.





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-09-28 20:05     ` Eli Zaretskii
@ 2016-09-29  0:12       ` Stefan Monnier
  0 siblings, 0 replies; 26+ messages in thread
From: Stefan Monnier @ 2016-09-29  0:12 UTC (permalink / raw
  To: Eli Zaretskii; +Cc: philippe.vaucher, 24555

> So restoring the specbind call that was lost in that commit is TRT, do
> you agree?

Could be.  At the same time, I have no recollection of ever noticing
this variable anywhere, and reading its docstring doesn't trigger any
recollection of a scenario where I could make use of it, so maybe it can
just be dropped.

What's the use case?


        Stefan





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-09-28 20:11       ` Eli Zaretskii
@ 2016-09-29  0:16         ` Stefan Monnier
  2016-09-29  6:50           ` Philippe Vaucher
  0 siblings, 1 reply; 26+ messages in thread
From: Stefan Monnier @ 2016-09-29  0:16 UTC (permalink / raw
  To: Eli Zaretskii; +Cc: Philippe Vaucher, 24555

> In a nutshell, it's a variable provided for storing arbitrary data
> that persists for the duration of the current interactive command.

Can't think of any situation where I'd want/need that.  The name seems
to indicate it could be used that way in the context of debugging, but
even in that kind of scenario I'm unable to come up with a use-case.


        Stefan





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-09-29  0:16         ` Stefan Monnier
@ 2016-09-29  6:50           ` Philippe Vaucher
  2016-09-29 15:04             ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Vaucher @ 2016-09-29  6:50 UTC (permalink / raw
  To: Stefan Monnier; +Cc: 24555

>> In a nutshell, it's a variable provided for storing arbitrary data
>> that persists for the duration of the current interactive command.
>
> Can't think of any situation where I'd want/need that.  The name seems
> to indicate it could be used that way in the context of debugging, but
> even in that kind of scenario I'm unable to come up with a use-case.

That's why I asked what was the use of such variable... even if we
restore the code that sets it, it is not used anywhere else.

The commit history suggest it might have been useful near the start of
the development of Emacs, but I don't think it is anymore.

I vote for dropping it.

Philippe





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-09-29  6:50           ` Philippe Vaucher
@ 2016-09-29 15:04             ` Eli Zaretskii
  2016-09-30  9:08               ` Philippe Vaucher
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2016-09-29 15:04 UTC (permalink / raw
  To: Philippe Vaucher; +Cc: monnier, 24555

> From: Philippe Vaucher <philippe.vaucher@gmail.com>
> Date: Thu, 29 Sep 2016 08:50:08 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, 24555@debbugs.gnu.org
> 
> > Can't think of any situation where I'd want/need that.  The name seems
> > to indicate it could be used that way in the context of debugging, but
> > even in that kind of scenario I'm unable to come up with a use-case.
> 
> That's why I asked what was the use of such variable... even if we
> restore the code that sets it, it is not used anywhere else.
> 
> The commit history suggest it might have been useful near the start of
> the development of Emacs, but I don't think it is anymore.
> 
> I vote for dropping it.

I don't think we can drop features just because no use case presents
itself for the moment.  IME, a debugging aid could be of no use until
you are in a situation where it is something to kill for.

But we don't need to argue about this now, because the way we remove
features from Emacs is by first declaring them obsolete, and keeping
them like that for a few releases; we don't just drop them without a
previous notice.

So I'm okay with declaring this variable obsolete, and even removing
its documentation from the manual, but we do need to restore the
specbind call that was deleted.

Philippe, can you do this, please?  It should be done on the emacs-25
branch, since the specbind was deleted in 25.1.

Thanks.





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-09-29 15:04             ` Eli Zaretskii
@ 2016-09-30  9:08               ` Philippe Vaucher
  2016-10-01 13:09                 ` Philippe Vaucher
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Vaucher @ 2016-09-30  9:08 UTC (permalink / raw
  To: Eli Zaretskii; +Cc: Stefan Monnier, 24555

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

>
> So I'm okay with declaring this variable obsolete, and even removing
> its documentation from the manual, but we do need to restore the
> specbind call that was deleted.
>
> Philippe, can you do this, please?  It should be done on the emacs-25
> branch, since the specbind was deleted in 25.1.
>

Okay, I'll start working on this.

Philippe

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-09-30  9:08               ` Philippe Vaucher
@ 2016-10-01 13:09                 ` Philippe Vaucher
  2016-10-01 15:50                   ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Vaucher @ 2016-10-01 13:09 UTC (permalink / raw
  To: Eli Zaretskii; +Cc: Stefan Monnier, 24555

>> Philippe, can you do this, please?  It should be done on the emacs-25
>> branch, since the specbind was deleted in 25.1.

Okay, here we go.

https://github.com/Silex/emacs/compare/emacs-25~2...Silex:emacs-25

https://github.com/Silex/emacs/commit/c8566ff77a347e7efc4cb2819cd7f58b68876e6f.patch
https://github.com/Silex/emacs/commit/e34b40dc4d12a0d806e59ccbf682b0980480ff88.patch

What is your prefered way to receive patches?

---
 src/callint.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/callint.c b/src/callint.c
index 053ee6c..4652151 100644
--- a/src/callint.c
+++ b/src/callint.c
@@ -837,7 +837,10 @@ invoke it.  If KEYS is omitted or nil, the return value of
   kset_last_command (current_kboard, save_last_command);

   {
-    Lisp_Object val = Ffuncall (nargs, args);
+    Lisp_Object val;
+    specbind (Vcommand_debug_status, Qnil);
+
+    val = Ffuncall (nargs, args);
     val = unbind_to (speccount, val);
     SAFE_FREE ();
     return val;

---
 doc/lispref/debugging.texi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/doc/lispref/debugging.texi b/doc/lispref/debugging.texi
index 2f83b40..322acd0 100644
--- a/doc/lispref/debugging.texi
+++ b/doc/lispref/debugging.texi
@@ -654,6 +654,8 @@ invocation.
 The advantage of using this variable rather than an ordinary global
 variable is that the data will never carry over to a subsequent command
 invocation.
+
+This variable is obsolete and should be removed in future versions.
 @end defvar

 @defun backtrace-frame frame-number





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-01 13:09                 ` Philippe Vaucher
@ 2016-10-01 15:50                   ` Eli Zaretskii
  2016-10-01 16:12                     ` Philippe Vaucher
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2016-10-01 15:50 UTC (permalink / raw
  To: Philippe Vaucher; +Cc: monnier, 24555

> From: Philippe Vaucher <philippe.vaucher@gmail.com>
> Date: Sat, 1 Oct 2016 15:09:33 +0200
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 24555@debbugs.gnu.org
> 
> >> Philippe, can you do this, please?  It should be done on the emacs-25
> >> branch, since the specbind was deleted in 25.1.
> 
> Okay, here we go.
> 
> https://github.com/Silex/emacs/compare/emacs-25~2...Silex:emacs-25
> 
> https://github.com/Silex/emacs/commit/c8566ff77a347e7efc4cb2819cd7f58b68876e6f.patch
> https://github.com/Silex/emacs/commit/e34b40dc4d12a0d806e59ccbf682b0980480ff88.patch
> 
> What is your prefered way to receive patches?

Either "git diff" with the commit log message prepended, or
"git format-patch", whatever works best for you.

I think your patch lacks the call to make-obsolete-variable.  We need
that to warn users of this variable about it being obsoleted.

Thanks.





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-01 15:50                   ` Eli Zaretskii
@ 2016-10-01 16:12                     ` Philippe Vaucher
  2016-10-01 17:39                       ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Vaucher @ 2016-10-01 16:12 UTC (permalink / raw
  To: Eli Zaretskii; +Cc: Stefan Monnier, 24555

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

> Either "git diff" with the commit log message prepended, or
> "git format-patch", whatever works best for you.

Alright.

> I think your patch lacks the call to make-obsolete-variable.  We need
> that to warn users of this variable about it being obsoleted.

Well, I thought about it but was not sure about various things, so I didn't
do it. Here are the things I'm unsure about:

(make-obsolete-variable OBSOLETE-NAME CURRENT-NAME WHEN
&optional ACCESS-TYPE)

Make the byte-compiler warn that OBSOLETE-NAME is obsolete.
The warning will say that CURRENT-NAME should be used instead.
If CURRENT-NAME is a string, that is the ‘use instead’ message.
WHEN should be a string indicating when the variable
was first made obsolete, for example a date or a release number.



   1. Where should I make the `make-obsolete-variable` call (in which
   file)? Given it's defined in callint.c it's not really obvious in which .el
   it should go.
   2. I don't have a "current name". Should I make CURRENT-NAME something
   like "This variable will be removed in the future"? Should I pass "nil"?
   3. What WHEN release number should I use? 25.2 ?

Thanks,
Philippe

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-01 16:12                     ` Philippe Vaucher
@ 2016-10-01 17:39                       ` Eli Zaretskii
  2016-10-02 17:08                         ` Philippe Vaucher
  2016-10-02 19:41                         ` Philippe Vaucher
  0 siblings, 2 replies; 26+ messages in thread
From: Eli Zaretskii @ 2016-10-01 17:39 UTC (permalink / raw
  To: Philippe Vaucher; +Cc: monnier, 24555

> From: Philippe Vaucher <philippe.vaucher@gmail.com>
> Date: Sat, 1 Oct 2016 18:12:06 +0200
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 24555@debbugs.gnu.org
> 
> > I think your patch lacks the call to make-obsolete-variable. We need
> > that to warn users of this variable about it being obsoleted.
> 
> Well, I thought about it but was not sure about various things, so I didn't do it. Here are the things I'm unsure
> about:
> 
>  (make-obsolete-variable OBSOLETE-NAME CURRENT-NAME WHEN &optional ACCESS-TYPE)
> 
>  Make the byte-compiler warn that OBSOLETE-NAME is obsolete.
>  The warning will say that CURRENT-NAME should be used instead.
>  If CURRENT-NAME is a string, that is the ‘use instead’ message.
>  WHEN should be a string indicating when the variable
>  was first made obsolete, for example a date or a release number.
> 
> 1 Where should I make the `make-obsolete-variable` call (in which file)? Given it's defined in callint.c it's not
>  really obvious in which .el it should go.

I'd say in subr.el; there's a bunch of those there already.

> 2 I don't have a "current name". Should I make CURRENT-NAME something like "This variable will be
>  removed in the future"? Should I pass "nil"?

The former, but I think the string should start with a lower-case
letter, as it will be displayed after the standard text saying the
variable is obsolete.  I suggest to experiment with different strings
and find the one which makes the most sense.

> 3 What WHEN release number should I use? 25.2 ?

Yes, 25.2.

Thanks.





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-01 17:39                       ` Eli Zaretskii
@ 2016-10-02 17:08                         ` Philippe Vaucher
  2016-10-02 18:32                           ` Noam Postavsky
  2016-10-02 19:41                         ` Philippe Vaucher
  1 sibling, 1 reply; 26+ messages in thread
From: Philippe Vaucher @ 2016-10-02 17:08 UTC (permalink / raw
  To: Eli Zaretskii; +Cc: Stefan Monnier, 24555

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

Hum.

I wanted to use `git send-email` and now of course it created 3 tickets.

So basically, I messed up. Sorry, is there a way I can close those 3
tickets myself?

Which brings me to a second question, can I use `git send-email` and send
it to 24555@debbugs.gnu.org instead or should I really copy-paste the
patches?

Thanks,
Philippe

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-02 17:08                         ` Philippe Vaucher
@ 2016-10-02 18:32                           ` Noam Postavsky
  2016-10-02 18:48                             ` Philippe Vaucher
  0 siblings, 1 reply; 26+ messages in thread
From: Noam Postavsky @ 2016-10-02 18:32 UTC (permalink / raw
  To: Philippe Vaucher; +Cc: Stefan Monnier, 24555

On Sun, Oct 2, 2016 at 1:08 PM, Philippe Vaucher
<philippe.vaucher@gmail.com> wrote:
> Hum.
>
> I wanted to use `git send-email` and now of course it created 3 tickets.
>
> So basically, I messed up. Sorry, is there a way I can close those 3 tickets
> myself?

Send email to <control@debbugs.gnu.org> with contents

merge xxxxx yyyyy zzzzz
quit

Where x, y, and z are the 3 bug numbers.

>
> Which brings me to a second question, can I use `git send-email` and send it
> to 24555@debbugs.gnu.org instead or should I really copy-paste the patches?

Both should work, the problem with using "git send-email" is that the
mails sometimes arrive out of order. I usually use "git format-patch"
and send as attachments.





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-02 18:32                           ` Noam Postavsky
@ 2016-10-02 18:48                             ` Philippe Vaucher
  0 siblings, 0 replies; 26+ messages in thread
From: Philippe Vaucher @ 2016-10-02 18:48 UTC (permalink / raw
  To: Noam Postavsky; +Cc: Stefan Monnier, 24555

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

>
> > So basically, I messed up. Sorry, is there a way I can close those 3
> tickets
> > myself?
>
> Send email to <control@debbugs.gnu.org> with contents
>
> merge xxxxx yyyyy zzzzz
> quit
>
> Where x, y, and z are the 3 bug numbers.


Will do, thanks!



> > Which brings me to a second question, can I use `git send-email` and
> send it
> > to 24555@debbugs.gnu.org instead or should I really copy-paste the
> patches?
>
> Both should work, the problem with using "git send-email" is that the
> mails sometimes arrive out of order. I usually use "git format-patch"
> and send as attachments.
>

Okay, I'll attach them from now on.

Thanks,
Philippe

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-01 17:39                       ` Eli Zaretskii
  2016-10-02 17:08                         ` Philippe Vaucher
@ 2016-10-02 19:41                         ` Philippe Vaucher
  2016-10-03  7:13                           ` Eli Zaretskii
  1 sibling, 1 reply; 26+ messages in thread
From: Philippe Vaucher @ 2016-10-02 19:41 UTC (permalink / raw
  To: Eli Zaretskii; +Cc: Stefan Monnier, 24555


[-- Attachment #1.1: Type: text/plain, Size: 766 bytes --]

>
> > 1 Where should I make the `make-obsolete-variable` call (in which file)?
> Given it's defined in callint.c it's not
> >  really obvious in which .el it should go.
>
> I'd say in subr.el; there's a bunch of those there already.
>
> > 2 I don't have a "current name". Should I make CURRENT-NAME something
> like "This variable will be
> >  removed in the future"? Should I pass "nil"?
>
> The former, but I think the string should start with a lower-case
> letter, as it will be displayed after the standard text saying the
> variable is obsolete.  I suggest to experiment with different strings
> and find the one which makes the most sense.
>
> > 3 What WHEN release number should I use? 25.2 ?
>
> Yes, 25.2.
>

Okay, here attached are the patches.

Philippe

[-- Attachment #1.2: Type: text/html, Size: 1184 bytes --]

[-- Attachment #2: 0001-Restore-command-debug-status-functionality.patch --]
[-- Type: text/x-patch, Size: 799 bytes --]

From 3d6ad7960a401602220e487ab20ec582f66609c0 Mon Sep 17 00:00:00 2001
From: Philippe Vaucher <philippe.vaucher@gmail.com>
Date: Sat, 1 Oct 2016 13:11:13 +0200
Subject: [PATCH 1/2] Restore command-debug-status functionality

---
 src/callint.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/callint.c b/src/callint.c
index 053ee6c..4652151 100644
--- a/src/callint.c
+++ b/src/callint.c
@@ -837,7 +837,10 @@ invoke it.  If KEYS is omitted or nil, the return value of
   kset_last_command (current_kboard, save_last_command);
 
   {
-    Lisp_Object val = Ffuncall (nargs, args);
+    Lisp_Object val;
+    specbind (Vcommand_debug_status, Qnil);
+
+    val = Ffuncall (nargs, args);
     val = unbind_to (speccount, val);
     SAFE_FREE ();
     return val;
-- 
2.10.0


[-- Attachment #3: 0002-Deprecate-variable-command-debug-status.patch --]
[-- Type: text/x-patch, Size: 1347 bytes --]

From ee9ceff89efe5e71b956d75f69cef6f50dd6b8ad Mon Sep 17 00:00:00 2001
From: Philippe Vaucher <philippe.vaucher@gmail.com>
Date: Wed, 28 Sep 2016 12:14:28 +0200
Subject: [PATCH 2/2] Deprecate variable command-debug-status

---
 doc/lispref/debugging.texi | 2 ++
 lisp/subr.el               | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/doc/lispref/debugging.texi b/doc/lispref/debugging.texi
index 2f83b40..322acd0 100644
--- a/doc/lispref/debugging.texi
+++ b/doc/lispref/debugging.texi
@@ -654,6 +654,8 @@ invocation.
 The advantage of using this variable rather than an ordinary global
 variable is that the data will never carry over to a subsequent command
 invocation.
+
+This variable is obsolete and should be removed in future versions.
 @end defvar
 
 @defun backtrace-frame frame-number
diff --git a/lisp/subr.el b/lisp/subr.el
index e9e19d3..271cd2f 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1377,6 +1377,8 @@ is converted into a string by expressing it in decimal."
 (make-obsolete 'process-filter-multibyte-p nil "23.1")
 (make-obsolete 'set-process-filter-multibyte nil "23.1")
 
+(make-obsolete-variable 'command-debug-status "should be removed in future versions" "25.2")
+
 ;; Lisp manual only updated in 22.1.
 (define-obsolete-variable-alias 'executing-macro 'executing-kbd-macro
   "before 19.34")
-- 
2.10.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-02 19:41                         ` Philippe Vaucher
@ 2016-10-03  7:13                           ` Eli Zaretskii
  2016-10-03  7:37                             ` Philippe Vaucher
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2016-10-03  7:13 UTC (permalink / raw
  To: Philippe Vaucher; +Cc: monnier, 24555

> From: Philippe Vaucher <philippe.vaucher@gmail.com>
> Date: Sun, 2 Oct 2016 21:41:18 +0200
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 24555@debbugs.gnu.org
> 
> diff --git a/doc/lispref/debugging.texi b/doc/lispref/debugging.texi
> index 2f83b40..322acd0 100644
> --- a/doc/lispref/debugging.texi
> +++ b/doc/lispref/debugging.texi
> @@ -654,6 +654,8 @@ invocation.
>  The advantage of using this variable rather than an ordinary global
>  variable is that the data will never carry over to a subsequent command
>  invocation.
> +
> +This variable is obsolete and should be removed in future versions.

I think "will be removed" is better here.

> diff --git a/lisp/subr.el b/lisp/subr.el
> index e9e19d3..271cd2f 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -1377,6 +1377,8 @@ is converted into a string by expressing it in decimal."
>  (make-obsolete 'process-filter-multibyte-p nil "23.1")
>  (make-obsolete 'set-process-filter-multibyte nil "23.1")
>  
> +(make-obsolete-variable 'command-debug-status "should be removed in future versions" "25.2")

And here I'd reword the message:

  "; expect it to be removed in a future version"

Can someone please push this to the emacs-25 branch in Philippe's
name?

Thanks.





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-03  7:13                           ` Eli Zaretskii
@ 2016-10-03  7:37                             ` Philippe Vaucher
  2016-10-03  7:42                               ` Philippe Vaucher
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Vaucher @ 2016-10-03  7:37 UTC (permalink / raw
  To: Eli Zaretskii; +Cc: Stefan Monnier, 24555


[-- Attachment #1.1: Type: text/plain, Size: 462 bytes --]

>
> > +This variable is obsolete and should be removed in future versions.
>
> I think "will be removed" is better here.
>
> > +(make-obsolete-variable 'command-debug-status "should be removed in
> future versions" "25.2")
>
> And here I'd reword the message:
>
>   "; expect it to be removed in a future version"
>

Here are the new patches with your modifications.

Also viewable at
https://github.com/Silex/emacs/compare/emacs-25~2...Silex:emacs-25

Philippe

[-- Attachment #1.2: Type: text/html, Size: 954 bytes --]

[-- Attachment #2: 0001-Restore-command-debug-status-functionality.patch --]
[-- Type: text/x-patch, Size: 799 bytes --]

From 16524476a21a09e0195d2db34e1ed4008b91d31c Mon Sep 17 00:00:00 2001
From: Philippe Vaucher <philippe.vaucher@gmail.com>
Date: Sat, 1 Oct 2016 13:11:13 +0200
Subject: [PATCH 1/2] Restore command-debug-status functionality

---
 src/callint.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/callint.c b/src/callint.c
index 053ee6c..4652151 100644
--- a/src/callint.c
+++ b/src/callint.c
@@ -837,7 +837,10 @@ invoke it.  If KEYS is omitted or nil, the return value of
   kset_last_command (current_kboard, save_last_command);
 
   {
-    Lisp_Object val = Ffuncall (nargs, args);
+    Lisp_Object val;
+    specbind (Vcommand_debug_status, Qnil);
+
+    val = Ffuncall (nargs, args);
     val = unbind_to (speccount, val);
     SAFE_FREE ();
     return val;
-- 
2.10.0


[-- Attachment #3: 0002-Deprecate-variable-command-debug-status.patch --]
[-- Type: text/x-patch, Size: 1351 bytes --]

From c666ae758ac57ce4684da2667fd912b0ae7ba428 Mon Sep 17 00:00:00 2001
From: Philippe Vaucher <philippe.vaucher@gmail.com>
Date: Wed, 28 Sep 2016 12:14:28 +0200
Subject: [PATCH 2/2] Deprecate variable command-debug-status

---
 doc/lispref/debugging.texi | 2 ++
 lisp/subr.el               | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/doc/lispref/debugging.texi b/doc/lispref/debugging.texi
index 2f83b40..c88a2fa 100644
--- a/doc/lispref/debugging.texi
+++ b/doc/lispref/debugging.texi
@@ -654,6 +654,8 @@ invocation.
 The advantage of using this variable rather than an ordinary global
 variable is that the data will never carry over to a subsequent command
 invocation.
+
+This variable is obsolete and will be removed in future versions.
 @end defvar
 
 @defun backtrace-frame frame-number
diff --git a/lisp/subr.el b/lisp/subr.el
index e9e19d3..2600901 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1377,6 +1377,8 @@ is converted into a string by expressing it in decimal."
 (make-obsolete 'process-filter-multibyte-p nil "23.1")
 (make-obsolete 'set-process-filter-multibyte nil "23.1")
 
+(make-obsolete-variable 'command-debug-status "; expect it to be removed a future version" "25.2")
+
 ;; Lisp manual only updated in 22.1.
 (define-obsolete-variable-alias 'executing-macro 'executing-kbd-macro
   "before 19.34")
-- 
2.10.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-03  7:37                             ` Philippe Vaucher
@ 2016-10-03  7:42                               ` Philippe Vaucher
  2016-10-04 14:43                                 ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Vaucher @ 2016-10-03  7:42 UTC (permalink / raw
  To: Eli Zaretskii; +Cc: Stefan Monnier, 24555


[-- Attachment #1.1: Type: text/plain, Size: 199 bytes --]

>
> Here are the new patches with your modifications.
>
> Also viewable at https://github.com/Silex/emacs/compare/emacs-25~2...
> Silex:emacs-25
>

And again new patches without a typo :-)

Philippe

[-- Attachment #1.2: Type: text/html, Size: 694 bytes --]

[-- Attachment #2: 0001-Restore-command-debug-status-functionality.patch --]
[-- Type: text/x-patch, Size: 799 bytes --]

From 16524476a21a09e0195d2db34e1ed4008b91d31c Mon Sep 17 00:00:00 2001
From: Philippe Vaucher <philippe.vaucher@gmail.com>
Date: Sat, 1 Oct 2016 13:11:13 +0200
Subject: [PATCH 1/2] Restore command-debug-status functionality

---
 src/callint.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/callint.c b/src/callint.c
index 053ee6c..4652151 100644
--- a/src/callint.c
+++ b/src/callint.c
@@ -837,7 +837,10 @@ invoke it.  If KEYS is omitted or nil, the return value of
   kset_last_command (current_kboard, save_last_command);
 
   {
-    Lisp_Object val = Ffuncall (nargs, args);
+    Lisp_Object val;
+    specbind (Vcommand_debug_status, Qnil);
+
+    val = Ffuncall (nargs, args);
     val = unbind_to (speccount, val);
     SAFE_FREE ();
     return val;
-- 
2.10.0


[-- Attachment #3: 0002-Deprecate-variable-command-debug-status.patch --]
[-- Type: text/x-patch, Size: 1354 bytes --]

From 2ac525977746173c2dd3be552960ab0a099b4dc3 Mon Sep 17 00:00:00 2001
From: Philippe Vaucher <philippe.vaucher@gmail.com>
Date: Wed, 28 Sep 2016 12:14:28 +0200
Subject: [PATCH 2/2] Deprecate variable command-debug-status

---
 doc/lispref/debugging.texi | 2 ++
 lisp/subr.el               | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/doc/lispref/debugging.texi b/doc/lispref/debugging.texi
index 2f83b40..c88a2fa 100644
--- a/doc/lispref/debugging.texi
+++ b/doc/lispref/debugging.texi
@@ -654,6 +654,8 @@ invocation.
 The advantage of using this variable rather than an ordinary global
 variable is that the data will never carry over to a subsequent command
 invocation.
+
+This variable is obsolete and will be removed in future versions.
 @end defvar
 
 @defun backtrace-frame frame-number
diff --git a/lisp/subr.el b/lisp/subr.el
index e9e19d3..5a9543d 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1377,6 +1377,8 @@ is converted into a string by expressing it in decimal."
 (make-obsolete 'process-filter-multibyte-p nil "23.1")
 (make-obsolete 'set-process-filter-multibyte nil "23.1")
 
+(make-obsolete-variable 'command-debug-status "; expect it to be removed in a future version" "25.2")
+
 ;; Lisp manual only updated in 22.1.
 (define-obsolete-variable-alias 'executing-macro 'executing-kbd-macro
   "before 19.34")
-- 
2.10.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-03  7:42                               ` Philippe Vaucher
@ 2016-10-04 14:43                                 ` Eli Zaretskii
  2016-10-04 15:18                                   ` Philippe Vaucher
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2016-10-04 14:43 UTC (permalink / raw
  To: Philippe Vaucher; +Cc: monnier, 24555-done

> From: Philippe Vaucher <philippe.vaucher@gmail.com>
> Date: Mon, 3 Oct 2016 09:42:14 +0200
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 24555@debbugs.gnu.org
> 
> And again new patches without a typo :-)

Thanks, I pushed this to the emacs-25 branch, and I'm marking this
bug done.

Please note that your patch had a fatal flaw: specbind needs a
(quoted) symbol, not its value.  Using Vcommand_debug_status there
produced a broken binary that would display an error message and
become unresponsive.  See what I actually committed for the details.

Also, the obsolescence warning needed some minor tweaks (it turns out
that my advice to prepend a semi-colon was a bad idea, as a semi-colon
and a newline are produced by Emacs automatically).

Please always test the build after you patch it, to make sure the
behavior is correct and no bugs creep in.

Finally, in the future please provide commit log messages for the
changes formatted in the ChanegLog style, as described in CONTRIBUTE.
I wrote them for this commit, please see the commit for the details of
the formatting we use.

Thanks a lot for working on this.





^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-04 14:43                                 ` Eli Zaretskii
@ 2016-10-04 15:18                                   ` Philippe Vaucher
  2016-10-04 15:46                                     ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Vaucher @ 2016-10-04 15:18 UTC (permalink / raw
  To: Eli Zaretskii; +Cc: Stefan Monnier, 24555-done

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

>
> Please note that your patch had a fatal flaw: specbind needs a
> (quoted) symbol, not its value.  Using Vcommand_debug_status there
> produced a broken binary that would display an error message and
> become unresponsive.  See what I actually committed for the details.
>

Ah, you're right I missread the patch that removed Vcommand_debug_status
and assumed "Q" variables got renamed to "V" in the emacs 25 transition. I
understand now that variables and symbols are different things :-) I should
have verified.



> Please always test the build after you patch it, to make sure the
> behavior is correct and no bugs creep in.
>

Yes, I should have. Sorry.



> Finally, in the future please provide commit log messages for the
> changes formatted in the ChanegLog style, as described in CONTRIBUTE.
> I wrote them for this commit, please see the commit for the details of
> the formatting we use.
>

Well, the commit f2144eef does not contain any ChangeLog entries... maybe
you forgot to stage these changes? But yes, I understand what you mean.

Regards,
Philippe

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-04 15:18                                   ` Philippe Vaucher
@ 2016-10-04 15:46                                     ` Eli Zaretskii
  2016-10-04 15:54                                       ` Philippe Vaucher
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2016-10-04 15:46 UTC (permalink / raw
  To: Philippe Vaucher; +Cc: monnier, 24555-done

> From: Philippe Vaucher <philippe.vaucher@gmail.com>
> Date: Tue, 4 Oct 2016 17:18:23 +0200
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 24555-done@debbugs.gnu.org
> 
>  Finally, in the future please provide commit log messages for the
>  changes formatted in the ChanegLog style, as described in CONTRIBUTE.
>  I wrote them for this commit, please see the commit for the details of
>  the formatting we use.
> 
> Well, the commit f2144eef does not contain any ChangeLog entries... maybe you forgot to stage these
> changes? But yes, I understand what you mean.

Not ChangeLog entries, but commit log message formatted as ChangeLog.
IOW, this:

    Restore 'command-debug-status' functionality

    * src/callint.c (Fcall_interactively): Bind command-debug-status
    to nil.  This restores functionality inadvertently removed in
    Emacs 25.1.  (Bug#24555)

    * lisp/subr.el (command-debug-status): Declare obsolete.

    * doc/lispref/debugging.texi (Internals of Debugger): Document
    that 'command-debug-status' is obsolete.






^ permalink raw reply	[flat|nested] 26+ messages in thread

* bug#24555: [PATCH] Remove unused variable `command-debug-status'
  2016-10-04 15:46                                     ` Eli Zaretskii
@ 2016-10-04 15:54                                       ` Philippe Vaucher
  0 siblings, 0 replies; 26+ messages in thread
From: Philippe Vaucher @ 2016-10-04 15:54 UTC (permalink / raw
  To: Eli Zaretskii; +Cc: Stefan Monnier, 24555-done

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

>
> > Well, the commit f2144eef does not contain any ChangeLog entries...
> maybe you forgot to stage these
> > changes? But yes, I understand what you mean.
>
> Not ChangeLog entries, but commit log message formatted as ChangeLog.
>

Ah, right. That is indeed a very complete change description!

Thanks,
Philippe

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2016-10-04 15:54 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-28 12:39 bug#24555: [PATCH] Remove unused variable `command-debug-status' Philippe Vaucher
2016-09-28 14:58 ` Eli Zaretskii
2016-09-28 16:14   ` Stefan Monnier
2016-09-28 17:24     ` Philippe Vaucher
2016-09-28 20:11       ` Eli Zaretskii
2016-09-29  0:16         ` Stefan Monnier
2016-09-29  6:50           ` Philippe Vaucher
2016-09-29 15:04             ` Eli Zaretskii
2016-09-30  9:08               ` Philippe Vaucher
2016-10-01 13:09                 ` Philippe Vaucher
2016-10-01 15:50                   ` Eli Zaretskii
2016-10-01 16:12                     ` Philippe Vaucher
2016-10-01 17:39                       ` Eli Zaretskii
2016-10-02 17:08                         ` Philippe Vaucher
2016-10-02 18:32                           ` Noam Postavsky
2016-10-02 18:48                             ` Philippe Vaucher
2016-10-02 19:41                         ` Philippe Vaucher
2016-10-03  7:13                           ` Eli Zaretskii
2016-10-03  7:37                             ` Philippe Vaucher
2016-10-03  7:42                               ` Philippe Vaucher
2016-10-04 14:43                                 ` Eli Zaretskii
2016-10-04 15:18                                   ` Philippe Vaucher
2016-10-04 15:46                                     ` Eli Zaretskii
2016-10-04 15:54                                       ` Philippe Vaucher
2016-09-28 20:05     ` Eli Zaretskii
2016-09-29  0:12       ` Stefan Monnier

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.