bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Stefan Kangas <stefankangas@gmail.com>]

Robert Pluim <rpluim@gmail.com> writes:

> So this simplifies the code considerably, and in fact removes the
> whole retry thing completely.

We could do that, yes.  It looks good to me.

I had a different idea in mind though, which lets us keep the diagnostic
message, but only if given a --verbose flag.  We could extend --verbose
to include more diagnostic output in the future.  Please see the
attached patch.

I'm happy with either your patch or mine, depending on what people
prefer.

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index 88800b9b2e..cd3069d7a8 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -66,7 +66,9 @@ Copyright (C) 1986-2022 Free Software Foundation, Inc.

 #endif /* !WINDOWSNT */

-#define DEFAULT_TIMEOUT (30)
+/* Seconds to wait before warning that the server hasn't responded.
+   Only applicable with --verbose and without any --timeout flag.  */
+#define DEFAULT_WARN_AFTER (30)

 #include <ctype.h>
 #include <errno.h>
@@ -110,6 +112,9 @@ #define DEFAULT_TIMEOUT (30)
 /* True means don't print messages for successful operations.  --quiet.  */
 static bool quiet;

+/* True means print verbose messages.  --verbose.  */
+static bool verbose;
+
 /* True means don't print values returned from emacs. --suppress-output.  */
 static bool suppress_output;

@@ -166,6 +171,7 @@ #define DEFAULT_TIMEOUT (30)
 static struct option const longopts[] =
 {
   { "no-wait",	no_argument,	   NULL, 'n' },
+  { "verbose",	no_argument,	   NULL, 'v' },
   { "quiet",	no_argument,	   NULL, 'q' },
   { "suppress-output", no_argument, NULL, 'u' },
   { "eval",	no_argument,	   NULL, 'e' },
@@ -191,7 +197,7 @@ #define DEFAULT_TIMEOUT (30)
 /* Short options, in the same order as the corresponding long options.
    There is no '-p' short option.  */
 static char const shortopts[] =
-  "nqueHVtca:F:w:"
+  "nqueHVtcva:F:w:"
 #ifdef SOCKETS_IN_FILE_SYSTEM
   "s:"
 #endif
@@ -488,6 +494,23 @@ message (bool is_error, const char *format, ...)
   va_end (args);
 }

+/* Print message only if we got the --verbose flag.  */
+static void print_verbose (const char *, ...) ATTRIBUTE_FORMAT_PRINTF (1, 2);
+static void
+print_verbose (const char *format, ...)
+{
+  va_list args;
+  va_start (args, format);
+
+  if (verbose)
+    {
+      vfprintf (stderr, format, args);
+      fflush (stderr);
+    }
+
+  va_end (args);
+}
+
 /* Decode the options from argv and argc.
    The global variable 'optind' will say how many arguments we used up.  */

@@ -556,6 +579,10 @@ decode_options (int argc, char **argv)
 	  quiet = true;
 	  break;

+	case 'v':
+	  verbose = true;
+	  break;
+
 	case 'u':
 	  suppress_output = true;
 	  break;
@@ -2144,40 +2171,49 @@ main (int argc, char **argv)
     }
   fflush (stdout);

-  set_socket_timeout (emacs_socket, timeout > 0 ? timeout : DEFAULT_TIMEOUT);
-  bool saw_response = false;
+  bool should_timeout = timeout > 0 || verbose;
   /* Now, wait for an answer and print any messages.  */
   while (exit_status == EXIT_SUCCESS)
     {
-      bool retry = true;
-      bool msg_showed = quiet;
+      if (should_timeout)
+	set_socket_timeout (emacs_socket,
+			    timeout > 0 ? timeout : DEFAULT_WARN_AFTER);
       do
 	{
+	retry_recv:
 	  act_on_signals (emacs_socket);
 	  rl = recv (emacs_socket, string, BUFSIZ, 0);
-	  retry = check_socket_timeout (rl);
-	  if (retry && !saw_response)
+
+	  /* Handle --timeout and --verbose.  */
+	  if (should_timeout
+	      /* Just retry if we got EINTR; it's not a timeout.  */
+	      && errno != EINTR)
 	    {
-	      if (timeout > 0)
-		{
-		  /* Don't retry if we were given a --timeout flag.  */
-		  fprintf (stderr, "\nServer not responding; timed out after %lu seconds",
-			   timeout);
-		  retry = false;
-		}
-	      else if (!msg_showed)
+	      should_timeout = false;
+	      set_socket_timeout (emacs_socket, 0);
+
+	      if (check_socket_timeout (rl))
 		{
-		  msg_showed = true;
-		  fprintf (stderr, "\nServer not responding; use Ctrl+C to break");
+		  if (timeout > 0)
+		    {
+		      fprintf (stderr,
+			       "\nServer not responding; timed out after %lu seconds",
+			       timeout);
+		      break;
+		    }
+		  else /* if (verbose) */
+		    {
+		      print_verbose ("\nServer not responding; use Ctrl+C to break");
+		      goto retry_recv;
+		    }
 		}
 	    }
 	}
-      while ((rl < 0 && errno == EINTR) || retry);
+      while (rl < 0 && errno == EINTR);

       if (rl <= 0)
         break;

-      saw_response = true;
       string[rl] = '\0';

       /* Loop over all NL-terminated messages.  */