From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Juri Linkov Newsgroups: gmane.emacs.devel Subject: Re: should search ring contain duplicates? Date: Wed, 03 May 2006 15:48:48 +0300 Organization: JURTA Message-ID: <87bqufwbls.fsf@jurta.org> References: <200605030727.k437R2Wx009975@amrm2.ics.uci.edu> NNTP-Posting-Host: main.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: sea.gmane.org 1146661536 20512 80.91.229.2 (3 May 2006 13:05:36 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Wed, 3 May 2006 13:05:36 +0000 (UTC) Cc: dann@ics.uci.edu, emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed May 03 15:05:30 2006 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by ciao.gmane.org with esmtp (Exim 4.43) id 1FbH2n-0000XA-3o for ged-emacs-devel@m.gmane.org; Wed, 03 May 2006 15:05:30 +0200 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1FbH2l-0000md-Jy for ged-emacs-devel@m.gmane.org; Wed, 03 May 2006 09:05:27 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1FbH2Y-0000mK-7o for emacs-devel@gnu.org; Wed, 03 May 2006 09:05:14 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1FbH2W-0000m0-EN for emacs-devel@gnu.org; Wed, 03 May 2006 09:05:13 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1FbH2W-0000lu-8F for emacs-devel@gnu.org; Wed, 03 May 2006 09:05:12 -0400 Original-Received: from [217.25.160.1] (helo=relay1.binet.com.ua) by monty-python.gnu.org with esmtp (Exim 4.52) id 1FbH2s-0003op-N0 for emacs-devel@gnu.org; Wed, 03 May 2006 09:05:36 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by relay1.binet.com.ua (Postfix) with ESMTP id 332E777F2E; Wed, 3 May 2006 16:05:09 +0300 (EEST) Original-Received: from mail.binet.com.ua (i42.dialup.binet.com.ua [217.25.161.106]) by relay1.binet.com.ua (Postfix) with ESMTP id E280477F20; Wed, 3 May 2006 16:04:47 +0300 (EEST) Original-To: storm@cua.dk (Kim F. Storm) In-Reply-To: (Kim F. Storm's message of "Wed, 03 May 2006 10:26:02 +0200") User-Agent: Gnus/5.110004 (No Gnus v0.4) Emacs/22.0.50 (gnu/linux) X-Virus-Scanned: by ClamAv at binet.com.ua X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:53844 Archived-At: > 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 non-@code{nil}, 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 non-@code{nil}, then it uses *************** *** 162,170 **** Representations}) from whichever buffer was current before entering the minibuffer. - If @var{keep-all} is non-@code{nil}, even empty and duplicate inputs - are added to the history list. - Use of @var{initial-contents} is mostly deprecated; we recommend using a non-@code{nil} 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/