diff mbox

[1/2] Read $GDBHISTSIZE instead of $HISTSIZE

Message ID 1432293831-23599-1-git-send-email-patrick@parcs.ath.cx
State New, archived
Headers

Commit Message

Patrick Palka May 22, 2015, 11:23 a.m. UTC 
  The HISTSIZE environment variable is generally expected to be read by
shells, not by applications.  Some distros for example globally export
HISTSIZE in /etc/profile -- with the intention that it only affects
shells -- and by doing so it renders useless GDB's own mechanism for
setting the history size via .gdbinit.  Also, annoyances may arise when
HISTSIZE is not interpreted the same way by the shell and by GDB, e.g.
PR gdb/16999.  That can always be fixed on a shell-by-shell basis but it
may be impossible to be consistent with the behavior of all shells at
once.  Finally it just makes sense to not confound shell environment
variables with application environment variables.

gdb/ChangeLog:

	* NEWS: Add entry.
	* top.c (init_history): Read from GDBHISTSIZE instead of
	HISTSIZE.
	(init_main): Refer to GDBHISTSIZE instead of HISTSIZE.

gdb/doc/ChangeLog:

	* gdb.texinfo (Command History): Replace occurrences of HISTSIZE
	with GDBHISTSIZE.

gdb/testsuite/ChangeLog:

	* gdb.base/gdbinit-history.exp: Replace occurrences of HISTSIZE
	with GDBHISTSIZE.
	* gdb.base/readline.exp: Likewis.
---
 gdb/NEWS                                   |  4 ++++
 gdb/doc/gdb.texinfo                        |  4 ++--
 gdb/testsuite/gdb.base/gdbinit-history.exp |  4 ++--
 gdb/testsuite/gdb.base/readline.exp        | 12 ++++++------
 gdb/top.c                                  |  4 ++--
 5 files changed, 16 insertions(+), 12 deletions(-)

Comments

Eli Zaretskii May 22, 2015, 12:30 p.m. UTC | #1 
> From: Patrick Palka <patrick@parcs.ath.cx>
> Cc: Patrick Palka <patrick@parcs.ath.cx>
> Date: Fri, 22 May 2015 07:23:50 -0400
> 
> The HISTSIZE environment variable is generally expected to be read by
> shells, not by applications.  Some distros for example globally export
> HISTSIZE in /etc/profile -- with the intention that it only affects
> shells -- and by doing so it renders useless GDB's own mechanism for
> setting the history size via .gdbinit.  Also, annoyances may arise when
> HISTSIZE is not interpreted the same way by the shell and by GDB, e.g.
> PR gdb/16999.  That can always be fixed on a shell-by-shell basis but it
> may be impossible to be consistent with the behavior of all shells at
> once.  Finally it just makes sense to not confound shell environment
> variables with application environment variables.
> 
> gdb/ChangeLog:
> 
> 	* NEWS: Add entry.
> 	* top.c (init_history): Read from GDBHISTSIZE instead of
> 	HISTSIZE.
> 	(init_main): Refer to GDBHISTSIZE instead of HISTSIZE.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Command History): Replace occurrences of HISTSIZE
> 	with GDBHISTSIZE.

Thanks.

I think we should explain in the manual why we don't use HISTSIZE.
And I wonder why we cannot use HISTSIZE if neither GDBHISTSIZE nor
.gdbinit specify the size, but you probably already discussed that.

Otherwise, the documentation part is OK.
Pedro Alves May 29, 2015, 11:05 a.m. UTC | #2 
On 05/22/2015 12:23 PM, Patrick Palka wrote:
> The HISTSIZE environment variable is generally expected to be read by
> shells, not by applications.  Some distros for example globally export
> HISTSIZE in /etc/profile -- with the intention that it only affects
> shells -- and by doing so it renders useless GDB's own mechanism for
> setting the history size via .gdbinit.  Also, annoyances may arise when
> HISTSIZE is not interpreted the same way by the shell and by GDB, e.g.
> PR gdb/16999.  That can always be fixed on a shell-by-shell basis but it
> may be impossible to be consistent with the behavior of all shells at
> once.  Finally it just makes sense to not confound shell environment
> variables with application environment variables.

This looks good to me.

Thanks,
Pedro Alves
Patrick Palka June 4, 2015, 3:30 p.m. UTC | #3 
On Fri, May 22, 2015 at 8:30 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Patrick Palka <patrick@parcs.ath.cx>
>> Cc: Patrick Palka <patrick@parcs.ath.cx>
>> Date: Fri, 22 May 2015 07:23:50 -0400
>>
>> The HISTSIZE environment variable is generally expected to be read by
>> shells, not by applications.  Some distros for example globally export
>> HISTSIZE in /etc/profile -- with the intention that it only affects
>> shells -- and by doing so it renders useless GDB's own mechanism for
>> setting the history size via .gdbinit.  Also, annoyances may arise when
>> HISTSIZE is not interpreted the same way by the shell and by GDB, e.g.
>> PR gdb/16999.  That can always be fixed on a shell-by-shell basis but it
>> may be impossible to be consistent with the behavior of all shells at
>> once.  Finally it just makes sense to not confound shell environment
>> variables with application environment variables.
>>
>> gdb/ChangeLog:
>>
>>       * NEWS: Add entry.
>>       * top.c (init_history): Read from GDBHISTSIZE instead of
>>       HISTSIZE.
>>       (init_main): Refer to GDBHISTSIZE instead of HISTSIZE.
>>
>> gdb/doc/ChangeLog:
>>
>>       * gdb.texinfo (Command History): Replace occurrences of HISTSIZE
>>       with GDBHISTSIZE.
>
> Thanks.
>
> I think we should explain in the manual why we don't use HISTSIZE.
> And I wonder why we cannot use HISTSIZE if neither GDBHISTSIZE nor
> .gdbinit specify the size, but you probably already discussed that.

Hmm, reading HISTSIZE as a last resort would reduce the inter-version
breakage a little, at least for users who today rely solely on
HISTSIZE (not ~/.gdbinit) to set their history size.  This can be
done, I guess.

>
> Otherwise, the documentation part is OK.
Pedro Alves June 15, 2015, 2:54 p.m. UTC | #4 
On 06/04/2015 04:30 PM, Patrick Palka wrote:

> Hmm, reading HISTSIZE as a last resort would reduce the inter-version
> breakage a little, at least for users who today rely solely on
> HISTSIZE (not ~/.gdbinit) to set their history size.  This can be
> done, I guess.

I'd propose going ahead with the patch that ignores HISTSIZE and
seeing if anyone notices/complains.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 745444b..1be239f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -48,6 +48,10 @@ 
 
 * GDB now supports the vector ABI on S/390 GNU/Linux targets.
 
+* The HISTSIZE environment variable is no longer read when determining
+  the size of GDB's command history.  GDB now instead reads the dedicated
+  GDBHISTSIZE environment variable.
+
 * Guile Scripting
 
   ** Memory ports can now be unbuffered.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e38fd31..c21a312 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22599,12 +22599,12 @@  Stop recording command history in a file.
 
 @cindex history size
 @kindex set history size
-@cindex @env{HISTSIZE}, environment variable
+@cindex @env{GDBHISTSIZE}, environment variable
 @item set history size @var{size}
 @itemx set history size unlimited
 Set the number of commands which @value{GDBN} keeps in its history list.
 This defaults to the value of the environment variable
-@code{HISTSIZE}, or to 256 if this variable is not set.  If @var{size}
+@code{GDBHISTSIZE}, or to 256 if this variable is not set.  If @var{size}
 is @code{unlimited}, the number of commands @value{GDBN} keeps in the
 history list is unlimited.
 @end table
diff --git a/gdb/testsuite/gdb.base/gdbinit-history.exp b/gdb/testsuite/gdb.base/gdbinit-history.exp
index 8adfd68..cbd965a 100644
--- a/gdb/testsuite/gdb.base/gdbinit-history.exp
+++ b/gdb/testsuite/gdb.base/gdbinit-history.exp
@@ -31,10 +31,10 @@  proc test_gdbinit_history_setting { home size } {
 
     set env(HOME) "$srcdir/$subdir/$home"
 
-    # The HISTSIZE environment variable takes precedence over whatever
+    # The GDBHISTSIZE environment variable takes precedence over whatever
     # history size is set in .gdbinit.  Make sure the former is not
     # set.
-    unset -nocomplain env(HISTSIZE)
+    unset -nocomplain env(GDBHISTSIZE)
 
     set saved_internal_gdbflags $INTERNAL_GDBFLAGS
     set INTERNAL_GDBFLAGS [string map {"-nx" ""} $INTERNAL_GDBFLAGS]
diff --git a/gdb/testsuite/gdb.base/readline.exp b/gdb/testsuite/gdb.base/readline.exp
index c444285..f0490a2 100644
--- a/gdb/testsuite/gdb.base/readline.exp
+++ b/gdb/testsuite/gdb.base/readline.exp
@@ -185,11 +185,11 @@  gdb_test_multiple "if 1 > 0\n\033\[A\033\[A\nend" $msg {
 if [info exists env(GDBHISTFILE)] {
     set old_gdbhistfile $env(GDBHISTFILE)
 }
-if [info exists env(HISTSIZE)] {
-    set old_histsize $env(HISTSIZE)
+if [info exists env(GDBHISTSIZE)] {
+    set old_gdbhistsize $env(GDBHISTSIZE)
 }
 set env(GDBHISTFILE) "${srcdir}/${subdir}/gdb_history"
-set env(HISTSIZE) "10"
+set env(GDBHISTSIZE) "10"
 
 gdb_exit
 gdb_start
@@ -207,10 +207,10 @@  if [info exists old_gdbhistfile] {
 } else {
     unset env(GDBHISTFILE)
 }
-if [info exists old_histsize] {
-    set env(HISTSIZE) $old_histsize
+if [info exists old_gdbhistsize] {
+    set env(GDBHISTSIZE) $old_gdbhistsize
 } else {
-    unset env(HISTSIZE)
+    unset env(GDBHISTSIZE)
 }
 set timeout $oldtimeout1
 
diff --git a/gdb/top.c b/gdb/top.c
index 74e1e07..6f0421d 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1680,7 +1680,7 @@  init_history (void)
 {
   char *tmpenv;
 
-  tmpenv = getenv ("HISTSIZE");
+  tmpenv = getenv ("GDBHISTSIZE");
   if (tmpenv)
     {
       int var;
@@ -1856,7 +1856,7 @@  Show the size of the command history,"), _("\
 ie. the number of previous commands to keep a record of.\n\
 If set to \"unlimited\", the number of commands kept in the history\n\
 list is unlimited.  This defaults to the value of the environment\n\
-variable \"HISTSIZE\", or to 256 if this variable is not set."),
+variable \"GDBHISTSIZE\", or to 256 if this variable is not set."),
 			    set_history_size_command,
 			    show_history_size,
 			    &sethistlist, &showhistlist);