bug#17430: 24.4.50; minibuffer history forgets the first entry

bug#17430: 24.4.50; minibuffer history forgets the first entry

[Jarek Czekalski]

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


Steps:
1. (read-from-minibuffer "pro: " nil nil nil 'hist)
2. type sth
3. check hist: M-S-: hist

Expected: sth, Actual: nil

The history value is not properly initialized. Seems like r87775
introduced a regression, because histval is not set to nil in minibuf.c.

During some more tests I noticed another faulty behaviour. With empty 
history
(unbound) I get an error message "variable void", when trying to get this
history (arrow up in minibuffer). Let's move the initialization of the 
history
a bit earlier, to prevent this error and receive expected message "Beginning
of history". Previously this initialization was at "add new entry" stage, so
history browsing was failing.

Finally the patch contains a fix for the regression and additionally a 
further
improvement.

Stefan, could you review the patch? If I am to commit this, to which
branch? emacs-24?




In GNU Emacs 24.4.50.1 (i686-pc-mingw32)
  of 2014-05-07 on BONSOFTW7
Repository revision: 117068 
eggert@cs.ucla.edu-20140506211337-crc6ozt8y6967wq3
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
  `configure --prefix=d:/program_files/emacs-master'

Configured features:
PNG NOTIFY ACL GNUTLS LIBXML2 ZLIB

Important settings:
   value of $LANG: pl
   locale-coding-system: cp1250

Major mode: Text

Minor modes in effect:
   diff-auto-refine-mode: t
   global-hl-line-mode: t
   recentf-mode: t
   tooltip-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   size-indication-mode: t
   column-number-mode: t
   line-number-mode: t
   transient-mark-mode: t

Recent input:
<help-echo> C-x C-f j : / l a n g / g n u / e m <tab>
/ h i <tab> 1 <tab> <return> <next> <prior> C-x C-f
<up> <up> <up> <up> <down> <return> <C-end> <up> C-SPC
<up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up>
<up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up>
<up> <up> <down> M-w M-x r e p o r t <return>

Recent messages:
[yas] Loading compiled snippets from 
c:/Users/Jarek/AppData/Roaming/.emacs.d/snippets/sql-mode
[yas] Loading compiled snippets from 
c:/Users/Jarek/AppData/Roaming/.emacs.d/snippets/text-mode
Loading 
c:/Users/Jarek/AppData/Roaming/.emacs.d/snippets/text-mode/.yas-compiled-snippets.el 
(source)...done
[yas] Loading snippet files from 
c:/Users/Jarek/AppData/Roaming/.emacs.d/snippets/text-mode
[yas] Loaded ~/.emacs.d/snippets
[yas] Reloaded everything....
Loading hl-line...done
Loading whitespace...done
Loading grep...done
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message dired format-spec
rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils diff-mode grep compile comint ansi-color
ring whitespace hl-line cus-start cus-load xml-parse yasnippet derived
easy-mmode edmacro kmacro help-mode folding-isearch folding cl-macs
advice help-fns cl gv bookmark pp recentf tree-widget wid-edit
cl-loaddefs cl-lib easymenu server time-date tooltip electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel dos-w32 ls-lisp
w32-common-fns disp-table w32-win w32-vars tool-bar dnd fontset image
regexp-opt fringe tabulated-list newcomment lisp-mode prog-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core frame cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew
greek romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process w32notify w32
multi-tty emacs)

Memory information:
((conses 8 150932 5821)
  (symbols 24 22859 0)
  (miscs 20 102 150)
  (strings 16 24546 3867)
  (string-bytes 1 712166)
  (vectors 8 13203)
  (vector-slots 4 417745 4112)
  (floats 8 134 272)
  (intervals 28 363 32)
  (buffers 508 15))

[-- Attachment #2: history_1_01.patch --]
[-- Type: text/plain, Size: 1823 bytes --]

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2014-05-06 21:13:37 +0000
+++ src/ChangeLog	2014-05-07 10:11:31 +0000
@@ -1,3 +1,9 @@
+2014-05-07  Jarek Czekalski  <jarekczek@poczta.onet.pl>
+
+	Fix initialization of minibuffer history variable (Bug#xxxxx).
+	* minibuf.c (read_minibuf): Initialize histval to Qnil if unbound.
+	Move the initialization up to prevent any "value void" message.
+
 2014-05-06  Paul Eggert  <eggert@cs.ucla.edu>
 
 	* image.c: Do not use libpng if HAVE_NS, as NS does its own thing.

=== modified file 'src/minibuf.c'
--- src/minibuf.c	2014-03-21 17:04:50 +0000
+++ src/minibuf.c	2014-05-07 10:09:50 +0000
@@ -384,6 +384,7 @@
   EMACS_INT pos = 0;
   /* String to add to the history.  */
   Lisp_Object histstring;
+  Lisp_Object histval;
 
   Lisp_Object empty_minibuf;
   Lisp_Object dummy, frame;
@@ -535,6 +536,13 @@
   if (!NILP (Vminibuffer_completing_file_name))
     Vminibuffer_completing_file_name = Qlambda;
 
+  /* If variable is unbound, make it nil.  */
+  histval = find_symbol_value (Vminibuffer_history_variable);
+  if (EQ (histval, Qunbound)) {
+    Fset (Vminibuffer_history_variable, Qnil);
+    histval = Qnil;
+  }
+
   if (inherit_input_method)
     {
       /* `current-input-method' is buffer local.  So, remember it in
@@ -703,13 +711,6 @@
     {
       /* If the caller wanted to save the value read on a history list,
 	 then do so if the value is not already the front of the list.  */
-      Lisp_Object histval;
-
-      /* If variable is unbound, make it nil.  */
-
-      histval = find_symbol_value (Vminibuffer_history_variable);
-      if (EQ (histval, Qunbound))
-	Fset (Vminibuffer_history_variable, Qnil);
 
       /* The value of the history variable must be a cons or nil.  Other
 	 values are unacceptable.  We silently ignore these values.  */

bug#17430: 24.4.50; minibuffer history forgets the first entry

[Stefan Monnier]

> The history value is not properly initialized. Seems like r87775
> introduced a regression, because histval is not set to nil in minibuf.c.

Indeed, thanks.

> Stefan, could you review the patch? If I am to commit this, to which
> branch? emacs-24?

Looks good.  Please install on emacs-24.
But please also just initialize the variable directly in its
declaration, as in:

  Lisp_Object histval = find_symbol_value (Vminibuffer_history_variable);


-- Stefan

bug#17430: 24.4.50; minibuffer history forgets the first entry

[Jarek Czekalski]

Applied as r117077 to emacs-24 branch [1]. Will be included in version 24.4.

http://bzr.savannah.gnu.org/lh/emacs/emacs-24/revision/117077