patch: create default error output function

patch: create default error output function

[Jarek Czekalski]

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

As suggested by Stefan in [1] I prepared a patch that creates a new 
elisp function which makes a default value for command-error-function. 
This makes it possible to use this variable by packages. Patch is 
against r114884.

Please review and comment my first Emacs patch. I couldn't find any 
guide for submitting patches, so I based it on my intuition and 
experience from previous projects, mainly jEdit.

Sometimes it's better to have a ticket in the tracker for a patch for 
easy reference to a discussion from a commit log message. But in gnu 
they say about posting a patch to the list, so I post.

Thanks,
Jarek

[1] http://lists.gnu.org/archive/html/emacs-devel/2013-10/msg00807.html


[-- Attachment #2: cmd_err_1_04.txt --]
[-- Type: text/plain, Size: 5133 bytes --]

=== modified file 'ChangeLog'
--- ChangeLog	2013-10-31 21:39:10 +0000
+++ ChangeLog	2013-11-01 19:36:58 +0000
@@ -1,3 +1,8 @@
+2013-11-01  Jarek Czekalski  <jarekczek@poczta.onet.pl>
+
+	* keyboard.c: new function default-error-output which becomes
+	a default value for command-error-function.
+
 2013-10-31  Glenn Morris  <rgm@gnu.org>
 
 	* configure.ac: Use [!...] rather than [^...], for ksh.  (Bug#15769)

=== modified file 'src/keyboard.c'
--- src/keyboard.c	2013-10-29 21:05:35 +0000
+++ src/keyboard.c	2013-11-01 19:46:51 +0000
@@ -241,6 +241,8 @@
 
 static Lisp_Object Qecho_area_clear_hook;
 
+static Lisp_Object Qdefault_error_output;
+
 /* Hooks to run before and after each command.  */
 static Lisp_Object Qpre_command_hook;
 static Lisp_Object Qpost_command_hook;
@@ -1053,35 +1055,21 @@
   return make_number (0);
 }
 
-/* Take actions on handling an error.  DATA is the data that describes
-   the error.
-
-   CONTEXT is a C-string containing ASCII characters only which
-   describes the context in which the error happened.  If we need to
-   generalize CONTEXT to allow multibyte characters, make it a Lisp
-   string.  */
-
-void
-cmd_error_internal (Lisp_Object data, const char *context)
+DEFUN ("default-error-output", Fdefault_error_output,
+       Sdefault_error_output, 3, 3, 0,
+       doc: /* Produce default output for the error message,
+into the messages buffer and the echo area.  */)
+  (Lisp_Object data, Lisp_Object context, Lisp_Object signal)
 {
   struct frame *sf = SELECTED_FRAME ();
-
-  /* The immediate context is not interesting for Quits,
-     since they are asynchronous.  */
-  if (EQ (XCAR (data), Qquit))
-    Vsignaling_function = Qnil;
-
-  Vquit_flag = Qnil;
-  Vinhibit_quit = Qt;
-
-  /* Use user's specified output function if any.  */
-  if (!NILP (Vcommand_error_function))
-    call3 (Vcommand_error_function, data,
-	   context ? build_string (context) : empty_unibyte_string,
-	   Vsignaling_function);
+  const char *sz_context;
+
+  CHECK_STRING(context);
+  sz_context = XSTRING(context)->data;
+
   /* If the window system or terminal frame hasn't been initialized
      yet, or we're not interactive, write the message to stderr and exit.  */
-  else if (!sf->glyphs_initialized_p
+  if (!sf->glyphs_initialized_p
 	   /* The initial frame is a special non-displaying frame. It
 	      will be current in daemon mode when there are no frames
 	      to display, and in non-daemon mode before the real frame
@@ -1096,7 +1084,7 @@
 	   || noninteractive)
     {
       print_error_message (data, Qexternal_debugging_output,
-			   context, Vsignaling_function);
+			   sz_context, signal);
       Fterpri (Qexternal_debugging_output);
       Fkill_emacs (make_number (-1));
     }
@@ -1107,8 +1095,35 @@
       message_log_maybe_newline ();
       bitch_at_user ();
 
-      print_error_message (data, Qt, context, Vsignaling_function);
+      print_error_message (data, Qt, sz_context, signal);
     }
+}
+
+/* Take actions on handling an error.  DATA is the data that describes
+   the error.
+
+   CONTEXT is a C-string containing ASCII characters only which
+   describes the context in which the error happened.  If we need to
+   generalize CONTEXT to allow multibyte characters, make it a Lisp
+   string.  */
+
+void
+cmd_error_internal (Lisp_Object data, const char *context)
+{
+  Lisp_Object Vcontext;
+  /* The immediate context is not interesting for Quits,
+     since they are asynchronous.  */
+  if (EQ (XCAR (data), Qquit))
+    Vsignaling_function = Qnil;
+
+  Vquit_flag = Qnil;
+  Vinhibit_quit = Qt;
+  Vcontext = context ? build_string (context) : empty_unibyte_string;
+
+  /* Use user's specified output function,
+     initially it is our Fdefault_error_output.  */
+  if (!NILP (Vcommand_error_function))
+    call3 (Vcommand_error_function, data, Vcontext, Vsignaling_function);
 
   Vsignaling_function = Qnil;
 }
@@ -11108,6 +11123,7 @@
   defsubr (&Sabort_recursive_edit);
   defsubr (&Sexit_recursive_edit);
   defsubr (&Srecursion_depth);
+  defsubr (&Sdefault_error_output);
   defsubr (&Stop_level);
   defsubr (&Sdiscard_input);
   defsubr (&Sopen_dribble_file);
@@ -11586,14 +11602,18 @@
 peculiar kind of quitting.  */);
   Vthrow_on_input = Qnil;
 
+  DEFSYM (Qdefault_error_output, "default-error-output");
   DEFVAR_LISP ("command-error-function", Vcommand_error_function,
 	       doc: /* If non-nil, function to output error messages.
 The arguments are the error data, a list of the form
  (SIGNALED-CONDITIONS . SIGNAL-DATA)
 such as just as `condition-case' would bind its variable to,
 the context (a string which normally goes at the start of the message),
-and the Lisp function within which the error was signaled.  */);
-  Vcommand_error_function = Qnil;
+and the Lisp function within which the error was signaled.
+
+This variable is initialized with Emacs default error output function.
+It is suggested that user functions are added using add-function.  */);
+  Vcommand_error_function = Qdefault_error_output;
 
   DEFVAR_LISP ("enable-disabled-menus-and-buttons",
 	       Venable_disabled_menus_and_buttons,

Re: patch: create default error output function

[Xue Fuqiao]

On Sat, Nov 2, 2013 at 4:18 AM, Jarek Czekalski
<jarekczek@poczta.onet.pl> wrote:
> As suggested by Stefan in [1] I prepared a patch that creates a new elisp
> function which makes a default value for command-error-function. This makes
> it possible to use this variable by packages. Patch is against r114884.

Thank you.

> Please review and comment my first Emacs patch. I couldn't find any guide
> for submitting patches, so I based it on my intuition and experience from
> previous projects, mainly jEdit.

See (info "(emacs) Sending Patches").

-- 
Best regards, Xue Fuqiao.
http://www.gnu.org/software/emacs/

Re: patch: create default error output function

[Xue Fuqiao]

On Sat, Nov 2, 2013 at 6:41 PM, Xue Fuqiao <xfq.free@gmail.com> wrote:
>> Please review and comment my first Emacs patch. I couldn't find any guide
>> for submitting patches, so I based it on my intuition and experience from
>> previous projects, mainly jEdit.
>
> See (info "(emacs) Sending Patches").

Although Stefan much prefer "diff -u" than "diff -c" ("diff -c" was
Richard's preference).

BTW you should modify the ChangeLog in src/ instead of the root one.
You can use `add-change-log-entry-other-window' (`C-x 4 a' by default)
to achieve it.

(Anyway, for the patch, `M-x diff-context->unified' or `M-x
diff-unified->context' takes care of converting from one to the other,
so we can adapt.)

-- 
Best regards, Xue Fuqiao.
http://www.gnu.org/software/emacs/

Re: patch: create default error output function

[Stefan Monnier]

> As suggested by Stefan in [1] I prepared a patch that creates a new elisp
> function which makes a default value for command-error-function.  This makes
> it possible to use this variable by packages. Patch is against r114884.

Thanks.  Installed (after moving the code a bit, to reduce the size of
the patch).  Also, I installed the patch below on top of it to better
follow the conventions.

> Sometimes it's better to have a ticket in the tracker for a patch for easy
> reference to a discussion from a commit log message. But in gnu they say
> about posting a patch to the list, so I post.

That's OK, but M-x report-emacs-bug is also a good way to do that,
indeed, so it gets a bug number.


        Stefan


PS: My email to jarekczek@poczta.onet.pl seems to have trouble
reaching you.  E.g. I get warnings like:

   ... while talking to mx.poczta.onet.pl.:
   >>> DATA
   <<< 450 4.7.1 <jarekczek@poczta.onet.pl>: Recipient address rejected: Spf check: softfail
   <jarekczek@poczta.onet.pl>... Deferred: 450 4.7.1 <jarekczek@poczta.onet.pl>: Recipient address rejected: Spf check: softfail
   <<< 554 5.5.1 Error: no valid recipients
   Warning: message still undelivered after 4 hours
   Will keep trying until message is 2 days old

PPS: I installed your patch as a "tiny change".  We won't be able to
accept many more contributions from you until you sign some
copyright paperwork.  If you want to sign such paperwork to get it out
of the way, then please send me an email.


=== modified file 'src/ChangeLog'
--- src/ChangeLog	2013-11-05 20:12:39 +0000
+++ src/ChangeLog	2013-11-05 20:13:34 +0000
@@ -1,7 +1,8 @@
 2013-11-05  Jarek Czekalski  <jarekczek@poczta.onet.pl>  (tiny change)
 
-	* keyboard.c: new function default-error-output which becomes
-	a default value for command-error-function.
+	* keyboard.c (Fdefault_error_output): New function, extracted from
+	cmd_error_internal.
+	(syms_of_keyboard): Use it for Vcommand_error_function.
 
 2013-11-05  Stefan Monnier  <monnier@iro.umontreal.ca>
 

=== modified file 'src/keyboard.c'
--- src/keyboard.c	2013-11-05 20:12:39 +0000
+++ src/keyboard.c	2013-11-05 20:45:53 +0000
@@ -241,8 +241,6 @@
 
 static Lisp_Object Qecho_area_clear_hook;
 
-static Lisp_Object Qdefault_error_output;
-
 /* Hooks to run before and after each command.  */
 static Lisp_Object Qpre_command_hook;
 static Lisp_Object Qpost_command_hook;
@@ -1074,8 +1072,7 @@
   Vquit_flag = Qnil;
   Vinhibit_quit = Qt;
 
-  /* Use user's specified output function,
-     initially it is our Fdefault_error_output.  */
+  /* Use user's specified output function if any.  */
   if (!NILP (Vcommand_error_function))
     call3 (Vcommand_error_function, data,
 	   context ? build_string (context) : empty_unibyte_string,
@@ -1084,17 +1081,17 @@
   Vsignaling_function = Qnil;
 }
 
-DEFUN ("default-error-output", Fdefault_error_output,
-       Sdefault_error_output, 3, 3, 0,
-       doc: /* Produce default output for the error message,
-into the messages buffer and the echo area.  */)
+DEFUN ("command-error-default-function", Fcommand_error_default_function,
+       Scommand_error_default_function, 3, 3, 0,
+       doc: /* Produce default output for unhandled error message.
+Default value of `command-error-function'.  */)
   (Lisp_Object data, Lisp_Object context, Lisp_Object signal)
 {
   struct frame *sf = SELECTED_FRAME ();
   const char *sz_context;
 
-  CHECK_STRING(context);
-  sz_context = XSTRING(context)->data;
+  CHECK_STRING (context);
+  sz_context = XSTRING (context)->data;
 
   /* If the window system or terminal frame hasn't been initialized
      yet, or we're not interactive, write the message to stderr and exit.  */
@@ -1126,6 +1123,7 @@
 
       print_error_message (data, Qt, sz_context, signal);
     }
+  return Qnil;
 }
 
 static Lisp_Object command_loop_2 (Lisp_Object);
@@ -11123,7 +11121,7 @@
   defsubr (&Sabort_recursive_edit);
   defsubr (&Sexit_recursive_edit);
   defsubr (&Srecursion_depth);
-  defsubr (&Sdefault_error_output);
+  defsubr (&Scommand_error_default_function);
   defsubr (&Stop_level);
   defsubr (&Sdiscard_input);
   defsubr (&Sopen_dribble_file);
@@ -11602,18 +11600,14 @@
 peculiar kind of quitting.  */);
   Vthrow_on_input = Qnil;
 
-  DEFSYM (Qdefault_error_output, "default-error-output");
   DEFVAR_LISP ("command-error-function", Vcommand_error_function,
-	       doc: /* If non-nil, function to output error messages.
-The arguments are the error data, a list of the form
- (SIGNALED-CONDITIONS . SIGNAL-DATA)
-such as just as `condition-case' would bind its variable to,
-the context (a string which normally goes at the start of the message),
-and the Lisp function within which the error was signaled.
-
-This variable is initialized with Emacs default error output function.
-It is suggested that user functions are added using add-function.  */);
-  Vcommand_error_function = Qdefault_error_output;
+	       doc: /* Function to output error messages.
+Called with three arguments:
+- the error data, a list of the form (SIGNALED-CONDITION . SIGNAL-DATA)
+  such as what `condition-case' would bind its variable to,
+- the context (a string which normally goes at the start of the message),
+- the Lisp function within which the error was signaled.  */);
+  Vcommand_error_function = intern ("command-error-default-function");
 
   DEFVAR_LISP ("enable-disabled-menus-and-buttons",
 	       Venable_disabled_menus_and_buttons,

Re: patch: create default error output function

[Jarek Czekalski]

Stefan, thank you for the review and the application.

> PS: My email to jarekczek@poczta.onet.pl seems to have trouble 
> reaching you. E.g. I get warnings like:

Too bad we can't contact directly. I also sent an email or two to you, 
probably they were dropped. My server is sometimes blocked by spam 
killers. They don't do the right job. The message from your server does 
not contain the spam filter server responsible for dropping the message, 
so I can't even react to that.

I use only 1 email address. If you have another account that we may use 
to establish the connection, please try a priv. Other option is my 
account on Google, which should be easy to locate by my email address. 
If someone has better ideas, I'm listening.

I sent the electronic papers you gave me to the "assign" mail box at 
gnu.org, but got no answer from them. I think I did it the same day that 
I sent the patch, but can't confirm this at the moment.

Thank you,
Jarek

Re: patch: create default error output function

[Stefan Monnier]

>> PS: My email to jarekczek@poczta.onet.pl seems to have trouble reaching
>> you. E.g. I get warnings like:
> Too bad we can't contact directly. I also sent an email or two to you,
> probably they were dropped.

I haven't noticed missing email, but of course, it's hard to see what is
absent.  This said, the problems I get seem to indicate a problem in "me
to you" direction rather than the other way around.

> I sent the electronic papers you gave me to the "assign" mail box at
> gnu.org, but got no answer from them. I think I did it the same day that
> I sent the patch, but can't confirm this at the moment.

Oh, sorry, I forgot that you had already started the process.
You might want to send a "ping" message to assign@gnu.org if you haven't
heard from them yet.


        Stefan