[RFA/commit/doco] move handing of "set interactive-mode" to gdb_has_a_terminal

[RFA/commit/doco] move handing of "set interactive-mode" to gdb_has_a_terminal

[Joel Brobecker]

Hello,

This is a change spawned by some observations made by Doug, which led
me to rethink the way "set/show iteractive-mode" is implemented. I think
that the new implementation makes things much clearer, and much closer
to what the original intent was.

I feel that the name of the setting isn't great, and I've been trying
to find better names, but not much luck (the best I have, I feel, is
"set/show terminal-mode [on|off|auto]).  However, this can be discussed
separately if some useful suggestion makes the change worthwhile.
In the meantime, I have adjusted the documentation in an effort to,
hopefully, make it clearer what this setting does.

Patch follows:
-------------------------------------------------------------------------

The real purpose of this setting is really to override what the debugger
would otherwise guess from checking the stdin settings.  So it seems
more natural to see this setting being handled inside gdb_has_a_terminal
rather than input_is_terminal (which checks for other things, such as
whether the input is stdin, for instance).

This patch also adjust the command help and the associated section in
the GDB Manual to be a little clearer about that.

gdb/ChangeLog:

        * inflow.c: Include "gdbcmd.h".
        (interactive_mode): New static global, moved here from top.c.
        (show_interactive_mode): New function, moved here from top.c.
        use gdb_has_a_terminal instead of input_from_terminal_p to
        determine the current mode.
        (gdb_has_a_terminal): Add handling of the "iteractive-mode"
        setting.
        (_initialize_inflow): Add the "set/show interactive-mode"
        commands.  Moved here from top.c, after having adjusted slightly
        the help text.
        * top.c (interactive_mode, show_interactive_mode): Delete, moved
        to inflow.c.
        (input_from_terminal_p): Remove handling of "interactive-mode"
        setting, moved to infow.c.
        (init_main): Remove creation of the "set/show interactive-mode"
        commands, moved to inflow.c.

gdb/doc/ChangeLog:

        * gdb.texinfo (Other Misc Settings): Rework part of the
        documentation of the "set interactive mode" command.

Tested on x86_64-linux.  Also manually tested on x86-windows, with
MinGW gdb running inside a cygwin window.

OK to commit?

---
 gdb/doc/gdb.texinfo |    9 ++++++---
 gdb/inflow.c        |   36 ++++++++++++++++++++++++++++++++++++
 gdb/top.c           |   38 --------------------------------------
 3 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d48d95c..e33bb4a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20022,10 +20022,13 @@ Displays the current state of XML debugging messages.
 @table @code
 @kindex set interactive-mode
 @item set interactive-mode
-If @code{on}, forces @value{GDBN} to operate interactively.
-If @code{off}, forces @value{GDBN} to operate non-interactively,
+If @code{on}, forces @value{GDBN} to assume that GDB was started
+in a terminal.  In practice, this means that @value{GDBN} should wait
+for the user to answer queries generated by commands entered at
+the command prompt.  If @code{off}, forces @value{GDBN} to operate
+in the opposite mode, and it uses the default answers to all queries.
 If @code{auto} (the default), @value{GDBN} guesses which mode to use,
-based on whether the debugger was started in a terminal or not.
+based on standard input settings.
 
 In the vast majority of cases, the debugger should be able to guess
 correctly which mode should be used.  But this setting can be useful
diff --git a/gdb/inflow.c b/gdb/inflow.c
index 65f48ba..859c74e 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -34,6 +34,7 @@
 #include "gdb_select.h"
 
 #include "inflow.h"
+#include "gdbcmd.h"
 
 #ifdef HAVE_SYS_IOCTL_H
 #include <sys/ioctl.h>
@@ -141,10 +142,31 @@ enum
   }
 gdb_has_a_terminal_flag = have_not_checked;
 
+/* The value of the "interactive-mode" setting.  */
+static enum auto_boolean interactive_mode = AUTO_BOOLEAN_AUTO;
+
+/* Implement the "show interactive-mode" option.  */
+
+static void
+show_interactive_mode (struct ui_file *file, int from_tty,
+                       struct cmd_list_element *c,
+                       const char *value)
+{
+  if (interactive_mode == AUTO_BOOLEAN_AUTO)
+    fprintf_filtered (file, "Debugger's interactive mode "
+		            "is %s (currently %s).\n",
+                      value, gdb_has_a_terminal () ? "on" : "off");
+  else
+    fprintf_filtered (file, "Debugger's interactive mode is %s.\n", value);
+}
+
 /* Does GDB have a terminal (on stdin)?  */
 int
 gdb_has_a_terminal (void)
 {
+  if (interactive_mode != AUTO_BOOLEAN_AUTO)
+    return interactive_mode = AUTO_BOOLEAN_TRUE;
+
   switch (gdb_has_a_terminal_flag)
     {
     case yes:
@@ -853,6 +875,20 @@ _initialize_inflow (void)
   add_info ("terminal", term_info,
 	    _("Print inferior's saved terminal status."));
 
+  add_setshow_auto_boolean_cmd ("interactive-mode", class_support,
+                                &interactive_mode, _("\
+Set whether GDB's standard input is a terminal."), _("\
+Show whether GDB's standard input is a terminal."), _("\
+If on, GDB assumes that standard input is a terminal.  In practice, it\n\
+means that GDB should wait for the user to answer queries associated to\n\
+commands entered at the command prompt.  If off, GDB assumes that standard\n\
+input is not a terminal, and uses the default answer to all queries.\n\
+If auto (the default), determine which mode to use based on the standard\n\
+input settings."),
+                        NULL,
+                        show_interactive_mode,
+                        &setlist, &showlist);
+
   terminal_is_ours = 1;
 
   /* OK, figure out whether we have job control.  If neither termios nor
diff --git a/gdb/top.c b/gdb/top.c
index bba1a2d..d14f308 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1263,38 +1263,12 @@ quit_force (char *args, int from_tty)
   exit (exit_code);
 }
 
-/* If OFF, the debugger will run in non-interactive mode, which means
-   that it will automatically select the default answer to all the
-   queries made to the user.  If ON, gdb will wait for the user to
-   answer all queries.  If AUTO, gdb will determine whether to run
-   in interactive mode or not depending on whether stdin is a terminal
-   or not.  */
-static enum auto_boolean interactive_mode = AUTO_BOOLEAN_AUTO;
-
-/* Implement the "show interactive-mode" option.  */
-
-static void
-show_interactive_mode (struct ui_file *file, int from_tty,
-                       struct cmd_list_element *c,
-                       const char *value)
-{
-  if (interactive_mode == AUTO_BOOLEAN_AUTO)
-    fprintf_filtered (file, "Debugger's interactive mode "
-		      "is %s (currently %s).\n",
-                      value, input_from_terminal_p () ? "on" : "off");
-  else
-    fprintf_filtered (file, "Debugger's interactive mode is %s.\n", value);
-}
-
 /* Returns whether GDB is running on a terminal and input is
    currently coming from that terminal.  */
 
 int
 input_from_terminal_p (void)
 {
-  if (interactive_mode != AUTO_BOOLEAN_AUTO && instream == stdin)
-    return interactive_mode == AUTO_BOOLEAN_TRUE;
-
   if (batch_flag)
     return 0;
 
@@ -1627,18 +1601,6 @@ Use \"on\" to enable the notification, and \"off\" to disable it."),
 			   show_exec_done_display_p,
 			   &setlist, &showlist);
 
-  add_setshow_auto_boolean_cmd ("interactive-mode", class_support,
-                                &interactive_mode, _("\
-Set whether GDB should run in interactive mode or not"), _("\
-Show whether GDB runs in interactive mode"), _("\
-If on, run in interactive mode and wait for the user to answer\n\
-all queries.  If off, run in non-interactive mode and automatically\n\
-assume the default answer to all queries.  If auto (the default),\n\
-determine which mode to use based on the standard input settings"),
-                        NULL,
-                        show_interactive_mode,
-                        &setlist, &showlist);
-
   add_setshow_filename_cmd ("data-directory", class_maintenance,
                            &gdb_datadir, _("Set GDB's data directory."),
                            _("Show GDB's data directory."),
-- 
1.7.1

Re: [RFA/commit/doco] move handing of "set interactive-mode" to gdb_has_a_terminal

[Eli Zaretskii]

> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Joel Brobecker <brobecker@adacore.com>
> Date: Tue, 18 Jan 2011 18:42:21 -0500
> 
> I feel that the name of the setting isn't great, and I've been trying
> to find better names, but not much luck (the best I have, I feel, is
> "set/show terminal-mode [on|off|auto]).

I see nothing wrong with "set interactive-mode", maybe because I'm
accustomed to this term in Emacs.

> This patch also adjust the command help and the associated section in
> the GDB Manual to be a little clearer about that.

Thanks.

>  If @code{auto} (the default), @value{GDBN} guesses which mode to use,
> -based on whether the debugger was started in a terminal or not.
> +based on standard input settings.

"Based on standard input settings" sounds vague and mysterious.  What
exactly does this mean?

Thanks.

Re: [RFA/commit/doco] move handing of "set interactive-mode" to gdb_has_a_terminal

[Joel Brobecker]

> > I feel that the name of the setting isn't great, and I've been trying
> > to find better names, but not much luck (the best I have, I feel, is
> > "set/show terminal-mode [on|off|auto]).
> 
> I see nothing wrong with "set interactive-mode", maybe because I'm
> accustomed to this term in Emacs.

So maybe the name isn't so bad, after all.  Changing it is a bit of
a pain in terms of transitioning from one name to the next, so that's
another argument in favor of keeping the current one.

> >  If @code{auto} (the default), @value{GDBN} guesses which mode to use,
> > -based on whether the debugger was started in a terminal or not.
> > +based on standard input settings.
> 
> "Based on standard input settings" sounds vague and mysterious.  What
> exactly does this mean?

Basically, we test whether stdin is a terminal or not.  How about:

    If @code{auto} (the default), @value{GDBN} guesses which mode to use,
    based on whether @value{GDBN}'s standard input is a terminal.

?

Thanks,
-- 
Joel

Re: [RFA/commit/doco] move handing of "set interactive-mode" to gdb_has_a_terminal

[Eli Zaretskii]

> Date: Wed, 19 Jan 2011 12:12:23 -0500
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > "Based on standard input settings" sounds vague and mysterious.  What
> > exactly does this mean?
> 
> Basically, we test whether stdin is a terminal or not.  How about:
> 
>     If @code{auto} (the default), @value{GDBN} guesses which mode to use,
>     based on whether @value{GDBN}'s standard input is a terminal.
> 
> ?

I would suggest

  If @code{auto} (the default), @value{GDBN} tries to determine
  whether its standard input is a terminal, and works in
  interactive-mode if it is, non-interactively otherwise.

Okay with that change.

Re: [RFA/commit/doco] move handing of "set interactive-mode" to gdb_has_a_terminal

[Joel Brobecker]

> gdb/ChangeLog:
> 
>         * inflow.c: Include "gdbcmd.h".
>         (interactive_mode): New static global, moved here from top.c.
>         (show_interactive_mode): New function, moved here from top.c.
>         use gdb_has_a_terminal instead of input_from_terminal_p to
>         determine the current mode.
>         (gdb_has_a_terminal): Add handling of the "iteractive-mode"
>         setting.
>         (_initialize_inflow): Add the "set/show interactive-mode"
>         commands.  Moved here from top.c, after having adjusted slightly
>         the help text.
>         * top.c (interactive_mode, show_interactive_mode): Delete, moved
>         to inflow.c.
>         (input_from_terminal_p): Remove handling of "interactive-mode"
>         setting, moved to infow.c.
>         (init_main): Remove creation of the "set/show interactive-mode"
>         commands, moved to inflow.c.
> 
> gdb/doc/ChangeLog:
> 
>         * gdb.texinfo (Other Misc Settings): Rework part of the
>         documentation of the "set interactive mode" command.

Patch checked in with Eli's suggestions (thanks!).

-- 
Joel

RE: [RFA/commit/doco] move handing of "set interactive-mode" to gdb_has_a_terminal

[Pierre Muller]

  Hi Joel,

  I think your patch is good, but
it has an error:
in gdb_has_a_terminal function:

+  if (interactive_mode != AUTO_BOOLEAN_AUTO)
+    return interactive_mode = AUTO_BOOLEAN_TRUE;
+
  The current code sets interactive_mode variable
to AUTO_BOOLEAN_TRUE, which happens to be zero,
and thus the function returns 0...

  I suppose that the correct code should be:
+  if (interactive_mode != AUTO_BOOLEAN_AUTO)
+    return (interactive_mode == AUTO_BOOLEAN_TRUE);
+

There is also a small error in the ChangeLog:
> >         (gdb_has_a_terminal): Add handling of the "iteractive-mode"
This should have "interactive-mode" instead of "iteractive-mode"


Pierre Muller
GDB pascal language maintainer

Re: [RFA/commit/doco] move handing of "set interactive-mode" to gdb_has_a_terminal

[Joel Brobecker]

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

>   I suppose that the correct code should be:
> +  if (interactive_mode != AUTO_BOOLEAN_AUTO)
> +    return (interactive_mode == AUTO_BOOLEAN_TRUE);

This is absolutely correct - horrible typo.

I just checked in the following patch to correct the problem, together
with added tests that demonstrate the effect of this bug.  In the
future, don't hesitate to fix the problem yourself too.

-- 
Joel

[-- Attachment #2: interact.diff --]
[-- Type: text/x-diff, Size: 3835 bytes --]

commit 9610bd83ed7d0c5bfd28fbc46c4f8828ab10c473
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Mon Jan 31 06:28:48 2011 +0400

    fix typo during interactive_mode check in gdb_has_a_terminal
    
    Discovered by Pierre Muller.
    
    gdb/ChangeLog:
    
            * inflow.c (gdb_has_a_terminal): Fix typo in interactive_mode
            value test.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.base/interact.exp: Add extra tests that verify that
            the value of the interactive-mode setting does not change
            after the script is sourced.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 373ccfc..9c18a83 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2011-01-31  Joel Brobecker  <brobecker@adacore.com>
+
+	* inflow.c (gdb_has_a_terminal): Fix typo in interactive_mode
+	value test.
+
 2011-01-31  Yao Qi  <yao@codesourcery.com>
 
 	* arm-linux-nat.c: Update calls to regcache_register_status
diff --git a/gdb/inflow.c b/gdb/inflow.c
index 859c74e..9975904 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -165,7 +165,7 @@ int
 gdb_has_a_terminal (void)
 {
   if (interactive_mode != AUTO_BOOLEAN_AUTO)
-    return interactive_mode = AUTO_BOOLEAN_TRUE;
+    return interactive_mode == AUTO_BOOLEAN_TRUE;
 
   switch (gdb_has_a_terminal_flag)
     {
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 1b63bff..d05b6e6 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2011-01-31  Joel Brobecker  <brobecker@adacore.com>
+
+	* gdb.base/interact.exp: Add extra tests that verify that
+	the value of the interactive-mode setting does not change
+	after the script is sourced.
+
 2011-01-29  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* gdb.cp/noparam.exp: New file.
diff --git a/gdb/testsuite/gdb.base/interact.exp b/gdb/testsuite/gdb.base/interact.exp
index 1f15fd8..59caf1b 100644
--- a/gdb/testsuite/gdb.base/interact.exp
+++ b/gdb/testsuite/gdb.base/interact.exp
@@ -28,21 +28,36 @@ set script_output "\\$\[0-9\]+ = 1\[\r\n\]+\\$\[0-9\]+ = 2.*"
 gdb_exit
 gdb_start
 
-# Test sourcing of the script with interactive mode `auto'
+# Test sourcing of the script with interactive mode `auto'.
+# Verify that evaluating the script does not cause an unexpected
+# change of the interactive-mode setting.
 gdb_test_no_output "set interactive-mode auto"
 gdb_test "source zzz-gdbscript" "$script_output" \
          "source script with interactive-mode auto"
 gdb_test "print 3" "= 3" "sanity check with interactive-mode auto"
+gdb_test "show interactive-mode" \
+         "Debugger's interactive mode is auto \\(currently .*\\)\\." \
+         "show interactive-mode (auto)"
 
-# Test sourcing of the script with interactive mode `on'
+# Test sourcing of the script with interactive mode `on'.
+# Verify that evaluating the script does not cause an unexpected
+# change of the interactive-mode setting.
 gdb_test_no_output "set interactive-mode on"
 gdb_test "source zzz-gdbscript" "$script_output" \
          "source script with interactive-mode on"
 gdb_test "print 4" "= 4" "sanity check with interactive-mode on"
+gdb_test "show interactive-mode" \
+         "Debugger's interactive mode is on\\." \
+         "show interactive-mode (on)"
 
-# Test sourcing of the script with interactive mode `of'
+# Test sourcing of the script with interactive mode `off'.
+# Verify that evaluating the script does not cause an unexpected
+# change of the interactive-mode setting.
 gdb_test_no_output "set interactive-mode off"
 gdb_test "source zzz-gdbscript" "$script_output" \
          "source script with interactive-mode off"
 gdb_test "print 5" "= 5" "sanity check with interactive-mode off"
+gdb_test "show interactive-mode" \
+         "Debugger's interactive mode is off\\." \
+         "show interactive-mode (off)"