emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: should search ring contain duplicates?


From: Juri Linkov
Subject: Re: should search ring contain duplicates?
Date: Wed, 03 May 2006 15:48:48 +0300
User-agent: Gnus/5.110004 (No Gnus v0.4) Emacs/22.0.50 (gnu/linux)

> IMO, that would be a very good candidate for a standard macro
> in subr.el.
>
> There are several places which does this (or should do it -- but
> actually doesn't get this right!):
>
>    (history-push ELT HIST &optional LENGTH KEEPDUPS)
>
> Default for LENGTH would be history-length, but it should also look at
> the history-length property of HIST.
>
> If KEEPDUPS is t, ignore value of history-delete-duplicates.
>
> This would make it easy for Lisp code to do the same history
> manipulation as C code.

I think this is a good idea.  This would help to get rid of a new
useless argument `keep-all' added a few months ago to the function
`read-from-minibuffer' only for the sake of query-replace.
Since query-replace is the only place in Emacs that uses this new argument,
it is better to remove it now before the release, and to use a new
macro `history-push' in `query-replace-read-to' to treat the query-replace
history specially.

Below is a tested patch that removes `keep-all' from `read-from-minibuffer',
adds duplicate replacement strings to the query-replace history explicitly,
and fixes more places where the value of `history-delete-duplicates'
is not taken into account yet (namely, `repeat-complex-command',
`call-interactively', and `isearch-update-ring').  This patch doesn't use
a new macro `history-push', but it would be easy to add it later to two
places in `isearch-update-ring' and `query-replace-read-to'.

Index: emacs/etc/NEWS
===================================================================
RCS file: /sources/emacs/emacs/etc/NEWS,v
retrieving revision 1.1336
diff -u -r1.1336 NEWS
*** emacs/etc/NEWS      29 Apr 2006 14:14:53 -0000      1.1336
--- emacs/etc/NEWS      3 May 2006 12:47:00 -0000
***************
*** 4271,4280 ****
  was selected when entering the minibuffer.
  
  +++
- *** `read-from-minibuffer' now accepts an additional argument KEEP-ALL
- saying to put all inputs in the history list, even empty ones.
- 
- +++
  *** The `read-file-name' function now takes an additional argument which
  specifies a predicate which the file name read must satisfy.  The
  new variable `read-file-name-predicate' contains the predicate argument
--- 4276,4281 ----
Index: emacs/lispref/minibuf.texi
===================================================================
RCS file: /sources/emacs/emacs/lispref/minibuf.texi,v
retrieving revision 1.77
diff -u -r1.77 minibuf.texi
*** emacs/lispref/minibuf.texi  19 Feb 2006 23:40:25 -0000      1.77
--- emacs/lispref/minibuf.texi  3 May 2006 12:47:18 -0000
***************
*** 110,116 ****
  reading the arguments for a command, in the @code{interactive}
  specification.  @xref{Defining Commands}.
  
! @defun read-from-minibuffer prompt-string &optional initial-contents keymap 
read hist default inherit-input-method keep-all
  This function is the most general way to get input through the
  minibuffer.  By default, it accepts arbitrary text and returns it as a
  string; however, if @var{read} is address@hidden, then it uses
--- 110,116 ----
  reading the arguments for a command, in the @code{interactive}
  specification.  @xref{Defining Commands}.
  
! @defun read-from-minibuffer prompt-string &optional initial-contents keymap 
read hist default inherit-input-method
  This function is the most general way to get input through the
  minibuffer.  By default, it accepts arbitrary text and returns it as a
  string; however, if @var{read} is address@hidden, then it uses
***************
*** 162,170 ****
  Representations}) from whichever buffer was current before entering the
  minibuffer.
  
- If @var{keep-all} is address@hidden, even empty and duplicate inputs
- are added to the history list.
- 
  Use of @var{initial-contents} is mostly deprecated; we recommend using
  a address@hidden value only in conjunction with specifying a cons cell
  for @var{hist}.  @xref{Initial Input}.
--- 162,167 ----
Index: emacs/src/lisp.h
===================================================================
RCS file: /sources/emacs/emacs/src/lisp.h,v
retrieving revision 1.562
diff -u -r1.562 lisp.h
*** emacs/src/lisp.h    12 Apr 2006 08:06:25 -0000      1.562
--- emacs/src/lisp.h    3 May 2006 12:47:46 -0000
***************
*** 2899,2905 ****
  extern Lisp_Object last_minibuf_string;
  extern void choose_minibuf_frame P_ ((void));
  EXFUN (Fcompleting_read, 8);
! EXFUN (Fread_from_minibuffer, 8);
  EXFUN (Fread_variable, 2);
  EXFUN (Fread_buffer, 3);
  EXFUN (Fread_minibuffer, 2);
--- 2900,2906 ----
  extern Lisp_Object last_minibuf_string;
  extern void choose_minibuf_frame P_ ((void));
  EXFUN (Fcompleting_read, 8);
! EXFUN (Fread_from_minibuffer, 7);
  EXFUN (Fread_variable, 2);
  EXFUN (Fread_buffer, 3);
  EXFUN (Fread_minibuffer, 2);
Index: emacs/src/minibuf.c
===================================================================
RCS file: /sources/emacs/emacs/src/minibuf.c,v
retrieving revision 1.301
diff -u -r1.301 minibuf.c
*** emacs/src/minibuf.c 6 Feb 2006 15:23:21 -0000       1.301
--- emacs/src/minibuf.c 3 May 2006 12:48:12 -0000
***************
*** 219,225 ****
                                     Lisp_Object, Lisp_Object,
                                     int, Lisp_Object,
                                     Lisp_Object, Lisp_Object,
!                                    int, int, int));
  static Lisp_Object read_minibuf_noninteractive P_ ((Lisp_Object, Lisp_Object,
                                                    Lisp_Object, Lisp_Object,
                                                    int, Lisp_Object,
--- 219,225 ----
                                     Lisp_Object, Lisp_Object,
                                     int, Lisp_Object,
                                     Lisp_Object, Lisp_Object,
!                                    int, int));
  static Lisp_Object read_minibuf_noninteractive P_ ((Lisp_Object, Lisp_Object,
                                                    Lisp_Object, Lisp_Object,
                                                    int, Lisp_Object,
***************
*** 440,447 ****
  
  static Lisp_Object
  read_minibuf (map, initial, prompt, backup_n, expflag,
!             histvar, histpos, defalt, allow_props, inherit_input_method,
!             keep_all)
       Lisp_Object map;
       Lisp_Object initial;
       Lisp_Object prompt;
--- 440,446 ----
  
  static Lisp_Object
  read_minibuf (map, initial, prompt, backup_n, expflag,
!             histvar, histpos, defalt, allow_props, inherit_input_method)
       Lisp_Object map;
       Lisp_Object initial;
       Lisp_Object prompt;
***************
*** 452,458 ****
       Lisp_Object defalt;
       int allow_props;
       int inherit_input_method;
-      int keep_all;
  {
    Lisp_Object val;
    int count = SPECPDL_INDEX ();
--- 451,456 ----
***************
*** 747,756 ****
    last_minibuf_string = val;
  
    /* Choose the string to add to the history.  */
!   if (SCHARS (val) != 0 || keep_all)
      histstring = val;
    else if (STRINGP (defalt))
      histstring = defalt;
    else
      histstring = Qnil;
  
--- 745,756 ----
    last_minibuf_string = val;
  
    /* Choose the string to add to the history.  */
!   if (SCHARS (val) != 0)
      histstring = val;
    else if (STRINGP (defalt))
      histstring = defalt;
    else
      histstring = Qnil;
  
***************
*** 774,781 ****
        if (NILP (histval)
          || (CONSP (histval)
              /* Don't duplicate the most recent entry in the history.  */
!             && (keep_all
!                 || NILP (Fequal (histstring, Fcar (histval))))))
        {
          Lisp_Object length;
  
--- 774,781 ----
        if (NILP (histval)
          || (CONSP (histval)
              /* Don't duplicate the most recent entry in the history.  */
!             && (NILP (Fequal (histstring, Fcar (histval))))))
        {
          Lisp_Object length;
  
***************
*** 937,943 ****
  }
  
  
! DEFUN ("read-from-minibuffer", Fread_from_minibuffer, Sread_from_minibuffer, 
1, 8, 0,
         doc: /* Read a string from the minibuffer, prompting with string 
PROMPT.
  The optional second arg INITIAL-CONTENTS is an obsolete alternative to
    DEFAULT-VALUE.  It normally should be nil in new code, except when
--- 937,943 ----
  }
  
  
! DEFUN ("read-from-minibuffer", Fread_from_minibuffer, Sread_from_minibuffer, 
1, 7, 0,
         doc: /* Read a string from the minibuffer, prompting with string 
PROMPT.
  The optional second arg INITIAL-CONTENTS is an obsolete alternative to
    DEFAULT-VALUE.  It normally should be nil in new code, except when
***************
*** 961,968 ****
    the empty string.
  Seventh arg INHERIT-INPUT-METHOD, if non-nil, means the minibuffer inherits
   the current input method and the setting of `enable-multibyte-characters'.
- Eight arg KEEP-ALL, if non-nil, says to put all inputs in the history list,
-  even empty or duplicate inputs.
  If the variable `minibuffer-allow-text-properties' is non-nil,
   then the string which is returned includes whatever text properties
   were present in the minibuffer.  Otherwise the value has no text properties.
--- 961,966 ----
***************
*** 978,986 ****
  one puts point at the beginning of the string.  *Note* that this
  behavior differs from the way such arguments are used in `completing-read'
  and some related functions, which use zero-indexing for POSITION.  */)
!   (prompt, initial_contents, keymap, read, hist, default_value, 
inherit_input_method, keep_all)
       Lisp_Object prompt, initial_contents, keymap, read, hist, default_value;
!      Lisp_Object inherit_input_method, keep_all;
  {
    Lisp_Object histvar, histpos, val;
    struct gcpro gcpro1;
--- 976,984 ----
  one puts point at the beginning of the string.  *Note* that this
  behavior differs from the way such arguments are used in `completing-read'
  and some related functions, which use zero-indexing for POSITION.  */)
!   (prompt, initial_contents, keymap, read, hist, default_value, 
inherit_input_method)
       Lisp_Object prompt, initial_contents, keymap, read, hist, default_value;
!      Lisp_Object inherit_input_method;
  {
    Lisp_Object histvar, histpos, val;
    struct gcpro gcpro1;
***************
*** 1011,1018 ****
                      Qnil, !NILP (read),
                      histvar, histpos, default_value,
                      minibuffer_allow_text_properties,
!                     !NILP (inherit_input_method),
!                     !NILP (keep_all));
    UNGCPRO;
    return val;
  }
--- 1009,1015 ----
                      Qnil, !NILP (read),
                      histvar, histpos, default_value,
                      minibuffer_allow_text_properties,
!                     !NILP (inherit_input_method));
    UNGCPRO;
    return val;
  }
***************
*** 1029,1035 ****
    CHECK_STRING (prompt);
    return read_minibuf (Vminibuffer_local_map, initial_contents,
                       prompt, Qnil, 1, Qminibuffer_history,
!                      make_number (0), Qnil, 0, 0, 0);
  }
  
  DEFUN ("eval-minibuffer", Feval_minibuffer, Seval_minibuffer, 1, 2, 0,
--- 1026,1032 ----
    CHECK_STRING (prompt);
    return read_minibuf (Vminibuffer_local_map, initial_contents,
                       prompt, Qnil, 1, Qminibuffer_history,
!                      make_number (0), Qnil, 0, 0);
  }
  
  DEFUN ("eval-minibuffer", Feval_minibuffer, Seval_minibuffer, 1, 2, 0,
***************
*** 1067,1075 ****
    Lisp_Object val;
    val = Fread_from_minibuffer (prompt, initial_input, Qnil,
                               Qnil, history, default_value,
!                              inherit_input_method, Qnil);
    if (STRINGP (val) && SCHARS (val) == 0 && ! NILP (default_value))
!     val = default_value;
    return val;
  }
  
--- 1064,1072 ----
    Lisp_Object val;
    val = Fread_from_minibuffer (prompt, initial_input, Qnil,
                               Qnil, history, default_value,
!                              inherit_input_method);
    if (STRINGP (val) && SCHARS (val) == 0 && ! NILP (default_value))
!     val = default_value;
    return val;
  }
  
***************
*** 1089,1095 ****
    CHECK_STRING (prompt);
    return read_minibuf (Vminibuffer_local_ns_map, initial, prompt, Qnil,
                       0, Qminibuffer_history, make_number (0), Qnil, 0,
!                      !NILP (inherit_input_method), 0);
  }
  
  DEFUN ("read-command", Fread_command, Sread_command, 1, 2, 0,
--- 1086,1092 ----
    CHECK_STRING (prompt);
    return read_minibuf (Vminibuffer_local_ns_map, initial, prompt, Qnil,
                       0, Qminibuffer_history, make_number (0), Qnil, 0,
!                      !NILP (inherit_input_method));
  }
  
  DEFUN ("read-command", Fread_command, Sread_command, 1, 2, 0,
***************
*** 1778,1787 ****
                         : Vminibuffer_local_must_match_filename_map),
                      init, prompt, make_number (pos), 0,
                      histvar, histpos, def, 0,
!                     !NILP (inherit_input_method), 0);
  
    if (STRINGP (val) && SCHARS (val) == 0 && ! NILP (def))
!     val = def;
  
    RETURN_UNGCPRO (unbind_to (count, val));
  }
--- 1775,1784 ----
                         : Vminibuffer_local_must_match_filename_map),
                      init, prompt, make_number (pos), 0,
                      histvar, histpos, def, 0,
!                     !NILP (inherit_input_method));
  
    if (STRINGP (val) && SCHARS (val) == 0 && ! NILP (def))
!     val = CONSP (def) ? XCAR (def) : def;
  
    RETURN_UNGCPRO (unbind_to (count, val));
  }
Index: emacs/lisp/replace.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/replace.el,v
retrieving revision 1.238
diff -u -r1.238 replace.el
*** emacs/lisp/replace.el       6 Feb 2006 14:33:35 -0000       1.238
--- emacs/lisp/replace.el       3 May 2006 14:45:00 -0000
***************
*** 101,107 ****
            ;; a region in order to specify the minibuffer input.
            ;; That should not clobber the region for the query-replace itself.
            (save-excursion
!             (when (equal lastfrom lastto)
                ;; Typically, this is because the two histlists are shared.
                (setq lastfrom (cadr (symbol-value
                                      query-replace-from-history-variable))))
--- 101,109 ----
            ;; a region in order to specify the minibuffer input.
            ;; That should not clobber the region for the query-replace itself.
            (save-excursion
!             (when (and (equal lastfrom lastto)
!                        (eq query-replace-from-history-variable
!                            query-replace-to-history-variable))
                ;; Typically, this is because the two histlists are shared.
                (setq lastfrom (cadr (symbol-value
                                      query-replace-from-history-variable))))
***************
*** 113,125 ****
                 (format "%s: " prompt))
               nil nil nil
               query-replace-from-history-variable
!              nil t t))))
        (if (and (zerop (length from)) lastto lastfrom)
!         (progn
!           (set query-replace-from-history-variable
!                (cdr (symbol-value query-replace-from-history-variable)))
!           (cons lastfrom
!                 (query-replace-compile-replacement lastto regexp-flag)))
        ;; Warn if user types \n or \t, but don't reject the input.
        (and regexp-flag
             (string-match "\\(\\`\\|[^\\]\\)\\(\\\\\\\\\\)*\\(\\\\[nt]\\)" 
from)
--- 115,124 ----
                 (format "%s: " prompt))
               nil nil nil
               query-replace-from-history-variable
!              nil t))))
        (if (and (zerop (length from)) lastto lastfrom)
!         (cons lastfrom
!               (query-replace-compile-replacement lastto regexp-flag))
        ;; Warn if user types \n or \t, but don't reject the input.
        (and regexp-flag
             (string-match "\\(\\`\\|[^\\]\\)\\(\\\\\\\\\\)*\\(\\\\[nt]\\)" 
from)
***************
*** 175,187 ****
  
  (defun query-replace-read-to (from prompt regexp-flag)
    "Query and return the `to' argument of a query-replace operation."
!   (query-replace-compile-replacement
!    (save-excursion
!      (read-from-minibuffer
!       (format "%s %s with: " prompt (query-replace-descr from))
!       nil nil nil
!       query-replace-to-history-variable from t t))
!    regexp-flag))
  
  (defun query-replace-read-args (prompt regexp-flag &optional noerror)
    (unless noerror
--- 174,187 ----
  
  (defun query-replace-read-to (from prompt regexp-flag)
    "Query and return the `to' argument of a query-replace operation."
!   (let* ((to-history (symbol-value query-replace-to-history-variable))
!        (to (save-excursion
!              (read-from-minibuffer
!               (format "%s %s with: " prompt (query-replace-descr from))
!               nil nil nil
!               query-replace-to-history-variable from t))))
!     (set query-replace-to-history-variable (cons to to-history))
!     (query-replace-compile-replacement to regexp-flag)))
  
  (defun query-replace-read-args (prompt regexp-flag &optional noerror)
    (unless noerror
Index: emacs/lisp/simple.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/simple.el,v
retrieving revision 1.797
diff -u -r1.797 simple.el
*** emacs/lisp/simple.el        9 Apr 2006 23:03:48 -0000       1.797
--- emacs/lisp/simple.el        3 May 2006 12:45:28 -0000
***************
*** 1135,1141 ****
                    ;; evaluable expressions there.
                    (if (stringp (car command-history))
                        (setq command-history (cdr command-history))))))
! 
          ;; If command to be redone does not match front of history,
          ;; add it to the history.
          (or (equal newcmd (car command-history))
--- 1135,1142 ----
                    ;; evaluable expressions there.
                    (if (stringp (car command-history))
                        (setq command-history (cdr command-history))))))
!         (if history-delete-duplicates
!             (setq command-history (delete newcmd command-history)))
          ;; If command to be redone does not match front of history,
          ;; add it to the history.
          (or (equal newcmd (car command-history))
Index: emacs/src/callint.c
===================================================================
RCS file: /sources/emacs/emacs/src/callint.c,v
retrieving revision 1.141
diff -u -r1.141 callint.c
*** emacs/src/callint.c 6 Feb 2006 15:23:20 -0000       1.141
--- emacs/src/callint.c 3 May 2006 12:45:18 -0000
***************
*** 40,45 ****
--- 40,46 ----
  Lisp_Object Qcall_interactively;
  Lisp_Object Vcommand_history;
  
+ extern int history_delete_duplicates;
  extern Lisp_Object Vhistory_length;
  extern Lisp_Object Vthis_original_command, real_this_command;
  
***************
*** 386,398 ****
        if (i != num_input_events || !NILP (record_flag))
        {
          /* We should record this command on the command history.  */
          Lisp_Object values;
          /* Make a copy of the list of values, for the command history,
             and turn them into things we can eval.  */
          values = quotify_args (Fcopy_sequence (specs));
          fix_command (input, values);
!         Vcommand_history
!           = Fcons (Fcons (function, values), Vcommand_history);
  
          /* Don't keep command history around forever.  */
          if (INTEGERP (Vhistory_length) && XINT (Vhistory_length) > 0)
--- 386,402 ----
        if (i != num_input_events || !NILP (record_flag))
        {
          /* We should record this command on the command history.  */
+         Lisp_Object histelem;
          Lisp_Object values;
          /* Make a copy of the list of values, for the command history,
             and turn them into things we can eval.  */
          values = quotify_args (Fcopy_sequence (specs));
          fix_command (input, values);
! 
!         histelem = Fcons (function, values);
!         if (history_delete_duplicates)
!           Vcommand_history = Fdelete (histelem, Vcommand_history);
!         Vcommand_history = Fcons (histelem, Vcommand_history);
  
          /* Don't keep command history around forever.  */
          if (INTEGERP (Vhistory_length) && XINT (Vhistory_length) > 0)
***************
*** 730,736 ****
  
                tem = Fread_from_minibuffer (build_string (callint_message),
                                             Qnil, Qnil, Qnil, Qnil, Qnil,
!                                            Qnil, Qnil);
                if (! STRINGP (tem) || SCHARS (tem) == 0)
                  args[i] = Qnil;
                else
--- 737,743 ----
  
                tem = Fread_from_minibuffer (build_string (callint_message),
                                             Qnil, Qnil, Qnil, Qnil, Qnil,
!                                            Qnil);
                if (! STRINGP (tem) || SCHARS (tem) == 0)
                  args[i] = Qnil;
                else
***************
*** 842,847 ****
--- 849,855 ----
  
    if (arg_from_tty || !NILP (record_flag))
      {
+       Lisp_Object histelem;
        visargs[0] = function;
        for (i = 1; i < count + 1; i++)
        {
***************
*** 850,857 ****
          else
            visargs[i] = quotify_arg (args[i]);
        }
!       Vcommand_history = Fcons (Flist (count + 1, visargs),
!                               Vcommand_history);
        /* Don't keep command history around forever.  */
        if (INTEGERP (Vhistory_length) && XINT (Vhistory_length) > 0)
        {
--- 858,868 ----
          else
            visargs[i] = quotify_arg (args[i]);
        }
! 
!       histelem = Flist (count + 1, visargs);
!       if (history_delete_duplicates)
!       Vcommand_history = Fdelete (histelem, Vcommand_history);
!       Vcommand_history = Fcons (histelem, Vcommand_history);
        /* Don't keep command history around forever.  */
        if (INTEGERP (Vhistory_length) && XINT (Vhistory_length) > 0)
        {
Index: emacs/lisp/isearch.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/isearch.el,v
retrieving revision 1.285
diff -u -r1.285 isearch.el
*** emacs/lisp/isearch.el       18 Mar 2006 15:11:48 -0000      1.285
--- emacs/lisp/isearch.el       3 May 2006 12:44:36 -0000
***************
*** 835,840 ****
--- 842,849 ----
        (if (or (null regexp-search-ring)
              (not (string= string (car regexp-search-ring))))
          (progn
+           (if history-delete-duplicates
+               (setq regexp-search-ring (delete string regexp-search-ring)))
            (push string regexp-search-ring)
            (if (> (length regexp-search-ring) regexp-search-ring-max)
                (setcdr (nthcdr (1- search-ring-max) regexp-search-ring)
***************
*** 842,847 ****
--- 851,858 ----
      (if (or (null search-ring)
            (not (string= string (car search-ring))))
        (progn
+         (if history-delete-duplicates
+             (setq search-ring (delete string search-ring)))
          (push string search-ring)
          (if (> (length search-ring) search-ring-max)
              (setcdr (nthcdr (1- search-ring-max) search-ring) nil))))))

-- 
Juri Linkov
http://www.jurta.org/emacs/





reply via email to

[Prev in Thread] Current Thread [Next in Thread]