Package: emacs;
Reported by: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Date: Thu, 19 Jan 2017 08:41:01 UTC
Severity: minor
Found in version 26.0.50
Done: Juri Linkov <juri <at> linkov.net>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25482 in the body.
You can then email your comments to 25482 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Thu, 19 Jan 2017 08:41:01 GMT) Full text and rfc822 format available.Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
:bug-gnu-emacs <at> gnu.org
.
(Thu, 19 Jan 2017 08:41:01 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: bug-gnu-emacs <at> gnu.org Subject: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Thu, 19 Jan 2017 09:18:07 +0100
Actually it is not possible to customize easily `query-replace-from-to-separator` and it is impossible to set it to nil. If I use `setq`, `custom-reevaluate-setting` reevaluate the original sexp from `query-replace-from-to-separator` and if I use customize I can add a string or a sexp, nothing else. But the code in `query-replace-read-from` is checking if `query-replace-from-to-separator` is non-nil, which is impossible actually. Thus it adds difficulties to load such sexp when dumping emacs. I would expect setting `query-replace-from-to-separator` to nil to retrieve same behavior as emacs-24.5 and not have unicode strings in my minibuffer-history. This patch allow this, please review it. Thanks. diff --git a/lisp/replace.el b/lisp/replace.el index ff917344453..dfc991b1691 100644 --- a/lisp/replace.el +++ b/lisp/replace.el @@ -79,15 +79,15 @@ That becomes the \"string to replace\".") to the minibuffer that reads the string to replace, or invoke replacements from Isearch by using a key sequence like `C-s C-s M-%'." "24.3") -(defcustom query-replace-from-to-separator - (propertize (if (char-displayable-p ?→) " → " " -> ") - 'face 'minibuffer-prompt) - "String that separates FROM and TO in the history of replacement pairs." - ;; Avoids error when attempt to autoload char-displayable-p fails - ;; while preparing to dump, also stops customize-rogue listing this. - :initialize 'custom-initialize-delay +(defcustom query-replace-from-to-separator " → " + "String that separates FROM and TO in the history of replacement pairs. +When nil the default separator \" -> \" will be used as a plain string +and the pair will not be added to `query-replace-history' +\(Same behavior as in emacs 24.5)." :group 'matching - :type '(choice string (sexp :tag "Display specification")) + :type '(choice + (const :tag "Disabled" nil) + string) :version "25.1") (defcustom query-replace-from-history-variable 'query-replace-history @@ -165,9 +165,13 @@ The return value can also be a pair (FROM . TO) indicating that the user wants to replace FROM with TO." (if query-replace-interactive (car (if regexp-flag regexp-search-ring search-ring)) - ;; Reevaluating will check char-displayable-p that is - ;; unavailable while preparing to dump. - (custom-reevaluate-setting 'query-replace-from-to-separator) + (when (stringp query-replace-from-to-separator) + (setq query-replace-from-to-separator + (propertize (if (char-displayable-p + (string-to-char query-replace-from-to-separator)) + query-replace-from-to-separator + " -> ") + 'face 'minibuffer-prompt))) (let* ((history-add-new-input nil) (separator (when query-replace-from-to-separator @@ -185,9 +189,13 @@ wants to replace FROM with TO." (symbol-value query-replace-from-history-variable))) (minibuffer-allow-text-properties t) ; separator uses text-properties (prompt - (if (and query-replace-defaults separator) - (format "%s (default %s): " prompt (car minibuffer-history)) - (format "%s: " prompt))) + (cond ((and query-replace-defaults separator) + (format "%s (default %s): " prompt (car minibuffer-history))) + (query-replace-defaults + (format "%s (default %s -> %s): " prompt + (query-replace-descr (caar query-replace-defaults)) + (query-replace-descr (cdar query-replace-defaults)))) + (t (format "%s: " prompt)))) (from ;; The save-excursion here is in case the user marks and copies ;; a region in order to specify the minibuffer input. In GNU Emacs 26.0.50.2 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8) of 2017-01-06 built on dell-14z Repository revision: 1d714e41ea73af89b56fb4bf19f8f0c3f443c268 Windowing system distributor 'The X.Org Foundation', version 11.0.11701000 System Description: Linux Mint 17.3 Rosa Recent messages: Mark set Auto-saving...done Auto-saving...done Mark set Sending... Entering debugger... Back to top level previous-line: Beginning of buffer Mark set Message modified; kill anyway? (y or n) y Configured using: 'configure CFLAGS=-O3' Configured features: XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS NOTIFY LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 Important settings: value of $LC_MONETARY: fr_FR.UTF-8 value of $LC_NUMERIC: fr_FR.UTF-8 value of $LC_TIME: fr_FR.UTF-8 value of $LANG: fr_FR.UTF-8 locale-coding-system: utf-8-unix Major mode: Emacs-Lisp Minor modes in effect: global-disable-mouse-mode: t global-git-gutter-mode: t git-gutter-mode: t eldoc-in-minibuffer-mode: t global-undo-tree-mode: t undo-tree-mode: t diff-auto-refine-mode: t magit-auto-revert-mode: t auto-revert-mode: t global-git-commit-mode: t psession-mode: t dired-async-mode: t display-time-mode: t winner-mode: t auto-image-file-mode: t savehist-mode: t show-paren-mode: t helm-descbinds-mode: t helm-top-poll-mode: t helm-push-mark-mode: t helm-mode: t shell-dirtrack-mode: t helm-adaptive-mode: t helm-popup-tip-mode: t async-bytecomp-package-mode: t minibuffer-depth-indicate-mode: t override-global-mode: t tooltip-mode: t global-eldoc-mode: t eldoc-mode: t mouse-wheel-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t column-number-mode: t line-number-mode: t transient-mark-mode: t Load-path shadows: /home/thierry/.emacs.d/elpa/org-20161224/ob-keys hides /usr/local/share/emacs/26.0.50/lisp/org/ob-keys /home/thierry/.emacs.d/elpa/org-20161224/ob-ref hides /usr/local/share/emacs/26.0.50/lisp/org/ob-ref /home/thierry/.emacs.d/elpa/org-20161224/ox-org hides /usr/local/share/emacs/26.0.50/lisp/org/ox-org /home/thierry/.emacs.d/elpa/org-20161224/ob-sass hides /usr/local/share/emacs/26.0.50/lisp/org/ob-sass /home/thierry/.emacs.d/elpa/org-20161224/org-bbdb hides /usr/local/share/emacs/26.0.50/lisp/org/org-bbdb /home/thierry/.emacs.d/elpa/org-20161224/ox-latex hides /usr/local/share/emacs/26.0.50/lisp/org/ox-latex /home/thierry/.emacs.d/elpa/org-20161224/ox-beamer hides /usr/local/share/emacs/26.0.50/lisp/org/ox-beamer /home/thierry/.emacs.d/elpa/org-20161224/org-crypt hides /usr/local/share/emacs/26.0.50/lisp/org/org-crypt /home/thierry/.emacs.d/elpa/org-20161224/ob-maxima hides /usr/local/share/emacs/26.0.50/lisp/org/ob-maxima /home/thierry/.emacs.d/elpa/org-20161224/ob-R hides /usr/local/share/emacs/26.0.50/lisp/org/ob-R /home/thierry/.emacs.d/elpa/org-20161224/ob-eval hides /usr/local/share/emacs/26.0.50/lisp/org/ob-eval /home/thierry/.emacs.d/elpa/org-20161224/org-datetree hides /usr/local/share/emacs/26.0.50/lisp/org/org-datetree /home/thierry/.emacs.d/elpa/org-20161224/org-element hides /usr/local/share/emacs/26.0.50/lisp/org/org-element /home/thierry/.emacs.d/elpa/org-20161224/ob-core hides /usr/local/share/emacs/26.0.50/lisp/org/ob-core /home/thierry/.emacs.d/elpa/org-20161224/ox-md hides /usr/local/share/emacs/26.0.50/lisp/org/ox-md /home/thierry/.emacs.d/elpa/org-20161224/org-indent hides /usr/local/share/emacs/26.0.50/lisp/org/org-indent /home/thierry/.emacs.d/elpa/org-20161224/ox hides /usr/local/share/emacs/26.0.50/lisp/org/ox /home/thierry/.emacs.d/elpa/org-20161224/ob-fortran hides /usr/local/share/emacs/26.0.50/lisp/org/ob-fortran /home/thierry/.emacs.d/elpa/org-20161224/ob-matlab hides /usr/local/share/emacs/26.0.50/lisp/org/ob-matlab /home/thierry/.emacs.d/elpa/org-20161224/org-macro hides /usr/local/share/emacs/26.0.50/lisp/org/org-macro /home/thierry/.emacs.d/elpa/org-20161224/ox-texinfo hides /usr/local/share/emacs/26.0.50/lisp/org/ox-texinfo /home/thierry/.emacs.d/elpa/org-20161224/ob-sqlite hides /usr/local/share/emacs/26.0.50/lisp/org/ob-sqlite /home/thierry/.emacs.d/elpa/org-20161224/org-faces hides /usr/local/share/emacs/26.0.50/lisp/org/org-faces /home/thierry/.emacs.d/elpa/org-20161224/org-pcomplete hides /usr/local/share/emacs/26.0.50/lisp/org/org-pcomplete /home/thierry/.emacs.d/elpa/org-20161224/org-mouse hides /usr/local/share/emacs/26.0.50/lisp/org/org-mouse /home/thierry/.emacs.d/elpa/org-20161224/ob-emacs-lisp hides /usr/local/share/emacs/26.0.50/lisp/org/ob-emacs-lisp /home/thierry/.emacs.d/elpa/org-20161224/org-archive hides /usr/local/share/emacs/26.0.50/lisp/org/org-archive /home/thierry/.emacs.d/elpa/org-20161224/org-capture hides /usr/local/share/emacs/26.0.50/lisp/org/org-capture /home/thierry/.emacs.d/elpa/org-20161224/ob-awk hides /usr/local/share/emacs/26.0.50/lisp/org/ob-awk /home/thierry/.emacs.d/elpa/org-20161224/ob-octave hides /usr/local/share/emacs/26.0.50/lisp/org/ob-octave /home/thierry/.emacs.d/elpa/org-20161224/org-timer hides /usr/local/share/emacs/26.0.50/lisp/org/org-timer /home/thierry/.emacs.d/elpa/org-20161224/ob-sql hides /usr/local/share/emacs/26.0.50/lisp/org/ob-sql /home/thierry/.emacs.d/elpa/org-20161224/ob-latex hides /usr/local/share/emacs/26.0.50/lisp/org/ob-latex /home/thierry/.emacs.d/elpa/org-20161224/org-macs hides /usr/local/share/emacs/26.0.50/lisp/org/org-macs /home/thierry/.emacs.d/elpa/org-20161224/org-rmail hides /usr/local/share/emacs/26.0.50/lisp/org/org-rmail /home/thierry/.emacs.d/elpa/org-20161224/org-w3m hides /usr/local/share/emacs/26.0.50/lisp/org/org-w3m /home/thierry/.emacs.d/elpa/org-20161224/ob-io hides /usr/local/share/emacs/26.0.50/lisp/org/ob-io /home/thierry/.emacs.d/elpa/org-20161224/ob hides /usr/local/share/emacs/26.0.50/lisp/org/ob /home/thierry/.emacs.d/elpa/org-20161224/ob-perl hides /usr/local/share/emacs/26.0.50/lisp/org/ob-perl /home/thierry/.emacs.d/elpa/org-20161224/ob-mscgen hides /usr/local/share/emacs/26.0.50/lisp/org/ob-mscgen /home/thierry/.emacs.d/elpa/org-20161224/ob-lilypond hides /usr/local/share/emacs/26.0.50/lisp/org/ob-lilypond /home/thierry/.emacs.d/elpa/org-20161224/org-footnote hides /usr/local/share/emacs/26.0.50/lisp/org/org-footnote /home/thierry/.emacs.d/elpa/org-20161224/ob-java hides /usr/local/share/emacs/26.0.50/lisp/org/ob-java /home/thierry/.emacs.d/elpa/org-20161224/ox-html hides /usr/local/share/emacs/26.0.50/lisp/org/ox-html /home/thierry/.emacs.d/elpa/org-20161224/ob-haskell hides /usr/local/share/emacs/26.0.50/lisp/org/ob-haskell /home/thierry/.emacs.d/elpa/org-20161224/org-docview hides /usr/local/share/emacs/26.0.50/lisp/org/org-docview /home/thierry/.emacs.d/elpa/org-20161224/ob-comint hides /usr/local/share/emacs/26.0.50/lisp/org/ob-comint /home/thierry/.emacs.d/elpa/org-20161224/ob-css hides /usr/local/share/emacs/26.0.50/lisp/org/ob-css /home/thierry/.emacs.d/elpa/org-20161224/ob-ditaa hides /usr/local/share/emacs/26.0.50/lisp/org/ob-ditaa /home/thierry/.emacs.d/elpa/org-20161224/ob-scala hides /usr/local/share/emacs/26.0.50/lisp/org/ob-scala /home/thierry/.emacs.d/elpa/org-20161224/org hides /usr/local/share/emacs/26.0.50/lisp/org/org /home/thierry/.emacs.d/elpa/org-20161224/org-mobile hides /usr/local/share/emacs/26.0.50/lisp/org/org-mobile /home/thierry/.emacs.d/elpa/org-20161224/ob-lisp hides /usr/local/share/emacs/26.0.50/lisp/org/ob-lisp /home/thierry/.emacs.d/elpa/org-20161224/ob-gnuplot hides /usr/local/share/emacs/26.0.50/lisp/org/ob-gnuplot /home/thierry/.emacs.d/elpa/org-20161224/org-src hides /usr/local/share/emacs/26.0.50/lisp/org/org-src /home/thierry/.emacs.d/elpa/org-20161224/ox-ascii hides /usr/local/share/emacs/26.0.50/lisp/org/ox-ascii /home/thierry/.emacs.d/elpa/org-20161224/ob-calc hides /usr/local/share/emacs/26.0.50/lisp/org/ob-calc /home/thierry/.emacs.d/elpa/org-20161224/org-irc hides /usr/local/share/emacs/26.0.50/lisp/org/org-irc /home/thierry/.emacs.d/elpa/org-20161224/org-loaddefs hides /usr/local/share/emacs/26.0.50/lisp/org/org-loaddefs /home/thierry/.emacs.d/elpa/org-20161224/org-install hides /usr/local/share/emacs/26.0.50/lisp/org/org-install /home/thierry/.emacs.d/elpa/org-20161224/org-info hides /usr/local/share/emacs/26.0.50/lisp/org/org-info /home/thierry/.emacs.d/elpa/org-20161224/ob-plantuml hides /usr/local/share/emacs/26.0.50/lisp/org/ob-plantuml /home/thierry/.emacs.d/elpa/org-20161224/org-feed hides /usr/local/share/emacs/26.0.50/lisp/org/org-feed /home/thierry/.emacs.d/elpa/org-20161224/org-version hides /usr/local/share/emacs/26.0.50/lisp/org/org-version /home/thierry/.emacs.d/elpa/org-20161224/ob-makefile hides /usr/local/share/emacs/26.0.50/lisp/org/ob-makefile /home/thierry/.emacs.d/elpa/org-20161224/org-entities hides /usr/local/share/emacs/26.0.50/lisp/org/org-entities /home/thierry/.emacs.d/elpa/org-20161224/ob-python hides /usr/local/share/emacs/26.0.50/lisp/org/ob-python /home/thierry/.emacs.d/elpa/org-20161224/ob-ledger hides /usr/local/share/emacs/26.0.50/lisp/org/ob-ledger /home/thierry/.emacs.d/elpa/org-20161224/ox-man hides /usr/local/share/emacs/26.0.50/lisp/org/ox-man /home/thierry/.emacs.d/elpa/org-20161224/ob-shen hides /usr/local/share/emacs/26.0.50/lisp/org/ob-shen /home/thierry/.emacs.d/elpa/org-20161224/org-inlinetask hides /usr/local/share/emacs/26.0.50/lisp/org/org-inlinetask /home/thierry/.emacs.d/elpa/org-20161224/org-list hides /usr/local/share/emacs/26.0.50/lisp/org/org-list /home/thierry/.emacs.d/elpa/org-20161224/ox-publish hides /usr/local/share/emacs/26.0.50/lisp/org/ox-publish /home/thierry/.emacs.d/elpa/org-20161224/org-gnus hides /usr/local/share/emacs/26.0.50/lisp/org/org-gnus /home/thierry/.emacs.d/elpa/org-20161224/org-agenda hides /usr/local/share/emacs/26.0.50/lisp/org/org-agenda /home/thierry/.emacs.d/elpa/org-20161224/org-id hides /usr/local/share/emacs/26.0.50/lisp/org/org-id /home/thierry/.emacs.d/elpa/org-20161224/org-plot hides /usr/local/share/emacs/26.0.50/lisp/org/org-plot /home/thierry/.emacs.d/elpa/org-20161224/ob-C hides /usr/local/share/emacs/26.0.50/lisp/org/ob-C /home/thierry/.emacs.d/elpa/org-20161224/org-clock hides /usr/local/share/emacs/26.0.50/lisp/org/org-clock /home/thierry/.emacs.d/elpa/org-20161224/org-attach hides /usr/local/share/emacs/26.0.50/lisp/org/org-attach /home/thierry/.emacs.d/elpa/org-20161224/ob-ruby hides /usr/local/share/emacs/26.0.50/lisp/org/ob-ruby /home/thierry/.emacs.d/elpa/org-20161224/org-habit hides /usr/local/share/emacs/26.0.50/lisp/org/org-habit /home/thierry/.emacs.d/elpa/org-20161224/org-eshell hides /usr/local/share/emacs/26.0.50/lisp/org/org-eshell /home/thierry/.emacs.d/elpa/org-20161224/ob-ocaml hides /usr/local/share/emacs/26.0.50/lisp/org/ob-ocaml /home/thierry/.emacs.d/elpa/org-20161224/ox-odt hides /usr/local/share/emacs/26.0.50/lisp/org/ox-odt /home/thierry/.emacs.d/elpa/org-20161224/ob-exp hides /usr/local/share/emacs/26.0.50/lisp/org/ob-exp /home/thierry/.emacs.d/elpa/org-20161224/ob-dot hides /usr/local/share/emacs/26.0.50/lisp/org/ob-dot /home/thierry/.emacs.d/elpa/org-20161224/ob-scheme hides /usr/local/share/emacs/26.0.50/lisp/org/ob-scheme /home/thierry/.emacs.d/elpa/org-20161224/ob-org hides /usr/local/share/emacs/26.0.50/lisp/org/ob-org /home/thierry/.emacs.d/elpa/org-20161224/org-bibtex hides /usr/local/share/emacs/26.0.50/lisp/org/org-bibtex /home/thierry/.emacs.d/elpa/org-20161224/org-compat hides /usr/local/share/emacs/26.0.50/lisp/org/org-compat /home/thierry/.emacs.d/elpa/org-20161224/ox-icalendar hides /usr/local/share/emacs/26.0.50/lisp/org/ox-icalendar /home/thierry/.emacs.d/elpa/org-20161224/org-colview hides /usr/local/share/emacs/26.0.50/lisp/org/org-colview /home/thierry/.emacs.d/elpa/org-20161224/ob-picolisp hides /usr/local/share/emacs/26.0.50/lisp/org/ob-picolisp /home/thierry/.emacs.d/elpa/org-20161224/org-mhe hides /usr/local/share/emacs/26.0.50/lisp/org/org-mhe /home/thierry/.emacs.d/elpa/org-20161224/org-table hides /usr/local/share/emacs/26.0.50/lisp/org/org-table /home/thierry/.emacs.d/elpa/org-20161224/ob-clojure hides /usr/local/share/emacs/26.0.50/lisp/org/ob-clojure /home/thierry/.emacs.d/elpa/org-20161224/ob-tangle hides /usr/local/share/emacs/26.0.50/lisp/org/ob-tangle /home/thierry/.emacs.d/elpa/org-20161224/ob-table hides /usr/local/share/emacs/26.0.50/lisp/org/ob-table /home/thierry/.emacs.d/elpa/org-20161224/ob-asymptote hides /usr/local/share/emacs/26.0.50/lisp/org/ob-asymptote /home/thierry/.emacs.d/elpa/org-20161224/org-ctags hides /usr/local/share/emacs/26.0.50/lisp/org/org-ctags /home/thierry/.emacs.d/elpa/org-20161224/ob-screen hides /usr/local/share/emacs/26.0.50/lisp/org/ob-screen /home/thierry/.emacs.d/elpa/org-20161224/org-protocol hides /usr/local/share/emacs/26.0.50/lisp/org/org-protocol /home/thierry/.emacs.d/elpa/org-20161224/ob-js hides /usr/local/share/emacs/26.0.50/lisp/org/ob-js /home/thierry/.emacs.d/elpa/org-20161224/ob-lob hides /usr/local/share/emacs/26.0.50/lisp/org/ob-lob Features: (mail-extr sort shadow epa-mail face-remap emacsbug whitespace helm-ls-git vc vc-dispatcher helm-misc helm-dabbrev eieio-opt help-fns radix-tree tabify helm-command helm-imenu js sgml-mode dom imenu slime-xref-browser tree-widget slime-banner slime-tramp slime-asdf slime-fancy slime-trace-dialog slime-fontifying-fu slime-package-fu slime-references slime-compiler-notes-tree slime-scratch slime-presentations bridge slime-macrostep slime-mdot-fu slime-enclosing-context slime-fuzzy slime-fancy-trace slime-fancy-inspector slime-c-p-c slime-editing-commands slime-autodoc slime-repl slime-parse slime etags xref project arc-mode archive-mode hyperspec make-mode cc-awk macrostep-c cmacexp macrostep cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs conf-mode vc-git naquadah-theme view solar cal-dst holidays hol-loaddefs em-unix em-term term disp-table ehelp em-script em-prompt em-ls em-hist em-pred em-glob em-dirs em-cmpl em-basic em-banner em-alias tv-utils gh gh-users gh-issues gh-pulls gh-repos gh-comments gh-gist gh-oauth gh-api logito gh-cache pcache eieio-base gh-auth gh-url url-http tls gnutls url-auth url-gw nsm disable-mouse powerline powerline-separators color powerline-themes windmove benchmark-init toc-org ert ewoc debug elp cl-indent esh-var esh-io esh-cmd esh-opt esh-ext esh-proc esh-arg esh-groups eshell esh-module esh-mode esh-util markdown-mode addressbook-bookmark mu4e-config org-mu4e helm-mu mu4e-contrib mu4e desktop frameset mu4e-speedbar speedbar sb-image ezimage dframe mu4e-main mu4e-context mu4e-view mu4e-headers mu4e-compose mu4e-draft mu4e-actions ido rfc2368 smtpmail sendmail mu4e-mark mu4e-message html2text mu4e-proc mu4e-utils mu4e-lists mu4e-vars hl-line cl mu4e-meta config-w3m w3m-search w3m doc-view jka-compr timezone w3m-hist w3m-fb bookmark-w3m w3m-ems w3m-ccl ccl w3m-favicon w3m-image w3m-proc w3m-util git-gutter cus-edit wid-edit appt diary-lib diary-loaddefs ange-ftp xdvi-search eldoc-eval undo-tree diff magit-obsolete magit-blame magit-stash magit-bisect magit-remote magit-commit magit-sequence magit-notes magit-worktree magit-branch magit-files magit-refs magit-status magit magit-repos magit-apply magit-wip magit-log magit-diff smerge-mode diff-mode magit-core magit-autorevert autorevert filenotify magit-process magit-margin magit-mode magit-git crm magit-section magit-popup git-commit magit-utils log-edit message subr-x puny rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils mailheader pcvs-util add-log with-editor tramp-sh server pcomplete-extension pcmpl-unix pcmpl-gnu psession iterator iedit iedit-lib dired-extension org-config-thierry ob-sh org-crypt org-element avl-tree org-location-google-maps org-agenda google-maps google-maps-static google-maps-geocode google-maps-base org org-macro org-footnote org-pcomplete org-list org-faces org-entities noutline outline org-version ob-emacs-lisp ob ob-tangle org-src ob-ref ob-lob ob-table ob-keys ob-exp ob-comint ob-core ob-eval org-compat org-macs org-loaddefs find-func cal-menu calendar cal-loaddefs dired-async net-utils time winner w3m-wget wget wget-sysdep cmake-mode autotest-mode autoconf-mode sh-script smie executable ps-print ps-print-loaddefs ps-def lpr rst image-file savehist paren woman man ediff-merg ediff-wind ediff-diff ediff-mult ediff-help ediff-init ediff-util ediff init-helm-thierry helm-descbinds helm-sys popup helm-ring helm-elisp helm-eval edebug helm-mode helm-files image-dired image-mode tramp tramp-compat tramp-loaddefs trampver shell pcomplete parse-time format-spec dired-x dired-aux ffap thingatpt helm-buffers helm-elscreen helm-tags helm-bookmark helm-adaptive helm-info bookmark pp helm-locate helm-grep wgrep-helm wgrep grep helm-regexp helm-external helm-net browse-url xml url url-proxy url-privacy url-expand url-methods url-history url-cookie url-domsuf url-util mailcap helm-utils compile comint ansi-color ring helm-help helm-types helm helm-source helm-multi-match helm-lib dired dired-loaddefs helm-extensions-autoloads helm-config helm-autoloads helm-easymenu async-bytecomp advice async mb-depth edmacro kmacro use-package diminish bind-key easy-mmode finder-inf tex-site gh-common gh-profile rx s ucs-normalize marshal eieio-compat ht json map dash slime-autoloads info package url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs password-cache url-vars seq byte-opt gv bytecomp byte-compile cl-extra help-mode easymenu cconv time-date avoid cus-start cus-load cl-loaddefs pcase cl-lib mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock syntax facemenu font-core term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese composite charscript case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer cl-preloaded 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 dbusbind inotify dynamic-setting system-font-setting font-render-setting move-toolbar gtk x-toolkit x multi-tty make-network-process emacs) Memory information: ((conses 16 878437 367693) (symbols 48 66030 13) (miscs 40 9143 3337) (strings 32 204861 111153) (string-bytes 1 7231121) (vectors 16 104738) (vector-slots 8 1950618 149106) (floats 8 3475 3306) (intervals 56 12606 6385) (buffers 976 203) (heap 1024 108498 9771)) -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Thu, 19 Jan 2017 16:02:02 GMT) Full text and rfc822 format available.Message #8 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Thu, 19 Jan 2017 18:01:12 +0200
> From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> > Date: Thu, 19 Jan 2017 09:18:07 +0100 > > > Actually it is not possible to customize easily > `query-replace-from-to-separator` and it is impossible to set it to nil. > > If I use `setq`, `custom-reevaluate-setting` reevaluate the original > sexp from `query-replace-from-to-separator` and if I use customize I can > add a string or a sexp, nothing else. > But the code in `query-replace-read-from` is checking if > `query-replace-from-to-separator` is non-nil, which is impossible > actually. > Thus it adds difficulties to load such sexp when dumping emacs. > > I would expect setting `query-replace-from-to-separator` to nil to > retrieve same behavior as emacs-24.5 and not have unicode strings in my > minibuffer-history. Sorry, I don't think I follow: you should be able to set this variable to " -> ", a string, to get the old behavior and avoid non-ASCII strings in your minibuffer-history. So why did you need the value of nil? > + (when (stringp query-replace-from-to-separator) > + (setq query-replace-from-to-separator > + (propertize (if (char-displayable-p > + (string-to-char query-replace-from-to-separator)) This doesn't look right: string-to-char will return the blank character, which is the first character of " → ", and I don't think you meant that. In any case, this looks more complex than it has to be, because you for some reason want to have the nil value. Please elaborate on the need for that. Thanks.
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Fri, 20 Jan 2017 05:43:02 GMT) Full text and rfc822 format available.Message #11 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: 25482 <at> debbugs.gnu.org Subject: Fwd: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Fri, 20 Jan 2017 06:42:10 +0100
Thierry Volpiatto <thierry.volpiatto <at> gmail.com> writes: > Eli Zaretskii <eliz <at> gnu.org> writes: > >> Sorry, I don't think I follow: you should be able to set this variable >> to " -> ", a string, to get the old behavior and avoid non-ASCII >> strings in your minibuffer-history. > > No, you don't have the same behavior, you endup with a non ascii string > which goes to minibuffer-history, so the behavior is not as emacs-24.5 > if you set this variable to " -> ". > >> So why did you need the value of nil? > > Because when it is nil the string is not added to history, just like in > 24.5. > >>> + (when (stringp query-replace-from-to-separator) >>> + (setq query-replace-from-to-separator >>> + (propertize (if (char-displayable-p >>> + (string-to-char query-replace-from-to-separator)) >> >> This doesn't look right: string-to-char will return the blank >> character, which is the first character of " → ", and I don't think >> you meant that. > > Yes, you are right, thanks, however see how the actual code is wrong, > what if someone set the separator to another unicode string. > >> In any case, this looks more complex than it has to be, because you >> for some reason want to have the nil value. Please elaborate on the >> need for that. > > To retrieve the same behavior as in emacs-24.5, see above. -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Fri, 20 Jan 2017 05:55:01 GMT) Full text and rfc822 format available.Message #14 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Fri, 20 Jan 2017 06:54:02 +0100
Here the patch updated: 1 file changed, 25 insertions(+), 16 deletions(-) lisp/replace.el | 41 +++++++++++++++++++++++++---------------- modified lisp/replace.el @@ -79,15 +79,15 @@ That becomes the \"string to replace\".") to the minibuffer that reads the string to replace, or invoke replacements from Isearch by using a key sequence like `C-s C-s M-%'." "24.3") -(defcustom query-replace-from-to-separator - (propertize (if (char-displayable-p ?→) " → " " -> ") - 'face 'minibuffer-prompt) - "String that separates FROM and TO in the history of replacement pairs." - ;; Avoids error when attempt to autoload char-displayable-p fails - ;; while preparing to dump, also stops customize-rogue listing this. - :initialize 'custom-initialize-delay +(defcustom query-replace-from-to-separator " → " + "String that separates FROM and TO in the history of replacement pairs. +When nil the default separator \" -> \" will be used as a plain string +and the pair will not be added to `query-replace-history' +\(Same behavior as in emacs 24.5)." :group 'matching - :type '(choice string (sexp :tag "Display specification")) + :type '(choice + (const :tag "Disabled" nil) + string) :version "25.1") (defcustom query-replace-from-history-variable 'query-replace-history @@ -165,9 +165,15 @@ The return value can also be a pair (FROM . TO) indicating that the user wants to replace FROM with TO." (if query-replace-interactive (car (if regexp-flag regexp-search-ring search-ring)) - ;; Reevaluating will check char-displayable-p that is - ;; unavailable while preparing to dump. - (custom-reevaluate-setting 'query-replace-from-to-separator) + (when (stringp query-replace-from-to-separator) + (setq query-replace-from-to-separator + (propertize (if (char-displayable-p + (string-to-char + (replace-regexp-in-string + " " "" query-replace-from-to-separator))) + query-replace-from-to-separator + " -> ") + 'face 'minibuffer-prompt))) (let* ((history-add-new-input nil) (separator (when query-replace-from-to-separator @@ -185,9 +191,13 @@ wants to replace FROM with TO." (symbol-value query-replace-from-history-variable))) (minibuffer-allow-text-properties t) ; separator uses text-properties (prompt - (if (and query-replace-defaults separator) - (format "%s (default %s): " prompt (car minibuffer-history)) - (format "%s: " prompt))) + (cond ((and query-replace-defaults separator) + (format "%s (default %s): " prompt (car minibuffer-history))) + (query-replace-defaults + (format "%s (default %s -> %s): " prompt + (query-replace-descr (caar query-replace-defaults)) + (query-replace-descr (cdar query-replace-defaults)))) + (t (format "%s: " prompt)))) (from ;; The save-excursion here is in case the user marks and copies ;; a region in order to specify the minibuffer input. @@ -200,8 +210,7 @@ wants to replace FROM with TO." (if regexp-flag (read-regexp prompt nil 'minibuffer-history) (read-from-minibuffer - prompt nil nil nil nil - (car (if regexp-flag regexp-search-ring search-ring)) t))))) + prompt nil nil nil nil (car search-ring) t))))) (to)) (if (and (zerop (length from)) query-replace-defaults) (cons (caar query-replace-defaults) -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Fri, 20 Jan 2017 07:57:01 GMT) Full text and rfc822 format available.Message #17 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Fri, 20 Jan 2017 09:56:29 +0200
[You've replied only to me.] > From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> > Date: Thu, 19 Jan 2017 20:33:41 +0100 > > > Eli Zaretskii <eliz <at> gnu.org> writes: > > > Sorry, I don't think I follow: you should be able to set this variable > > to " -> ", a string, to get the old behavior and avoid non-ASCII > > strings in your minibuffer-history. > > No, you don't have the same behavior, you endup with a non ascii string > which goes to minibuffer-history, so the behavior is not as emacs-24.5 > if you set this variable to " -> ". So you are saying that even setting the string to " -> " gets " → " into the history? If so, that's the bug that we should solve, I think. > > So why did you need the value of nil? > > Because when it is nil the string is not added to history, just like in > 24.5. Is it a problem to have the ASCII " -> " in the history? Assuming we solve this part of the problem, would you still need to disable adding the string to the history?
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Fri, 20 Jan 2017 11:36:02 GMT) Full text and rfc822 format available.Message #20 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Fri, 20 Jan 2017 12:35:29 +0100
Eli Zaretskii <eliz <at> gnu.org> writes: > [You've replied only to me.] Yes sorry, I am so used to github that I sometimes forget to reply to all. > So you are saying that even setting the string to " -> " gets " → " > into the history? No even if you set the string to " -> " the string is not a plain string but "\0" with 'display property " -> ". > If so, that's the bug that we should solve, I think. The patch is fixing this. > Is it a problem to have the ASCII " -> " in the history? It is not ASCII but ^@. > Assuming we solve this part of the problem, would you still need to > disable adding the string to the history? No that would be ok as long as we use a plain string for " -> ". But note that the fix I provided make the defcustom simpler and easier to maintain both from user and developer side, thus it allow to use both custom and setq which is not the case with actual code. So to resume, With actual code: 1) Using (setq query-replace-from-to-separator " -> ") is not working. 2) Using (customize-set-variable query-replace-from-to-separator " -> ") is working but " -> " will not be a plain ascii string and will be added to history. 3) Setting query-replace-from-to-separator to nil is not possible, apart perhaps using a sexp that evaluate to nil, which is not obvious and in which case do not add a prompt with a default value like in 24.5 (regression). With the patch: 1) and 2) are working and preserve the actual behavior, which is adding "from<separator>to" to history. 3) Is working and provide the exact behavior as 24.5, that is use a plain ascii string " -> " as separator and do not add "from<separator>to" to history. The default setting is the same as the actual code, no change. Unrelated, I also fixed this block of code: (if regexp-flag (read-regexp prompt nil 'minibuffer-history) (read-from-minibuffer prompt nil nil nil nil (car (if regexp-flag regexp-search-ring search-ring)) t)) -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Fri, 20 Jan 2017 13:08:02 GMT) Full text and rfc822 format available.Message #23 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Fri, 20 Jan 2017 15:06:59 +0200
> From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> > Cc: 25482 <at> debbugs.gnu.org > Date: Fri, 20 Jan 2017 12:35:29 +0100 > > With actual code: > > 1) Using (setq query-replace-from-to-separator " -> ") > is not working. > > 2) Using (customize-set-variable query-replace-from-to-separator " -> ") > is working but " -> " will not be a plain ascii string and will be added > to history. > > 3) Setting query-replace-from-to-separator to nil is not possible, apart perhaps > using a sexp that evaluate to nil, which is not obvious and in which > case do not add a prompt with a default value like in 24.5 (regression). > > With the patch: > > 1) and 2) are working and preserve the actual behavior, which is adding > "from<separator>to" to history. > > 3) Is working and provide the exact behavior as 24.5, that is use a > plain ascii string " -> " as separator and do not add > "from<separator>to" to history. > > The default setting is the same as the actual code, no change. Yes, I understand. I'm asking whether the (fixed) behavior, whereby using setq to change the value to " -> " would add the ASCII string " -> " to the minibuffer history, would be acceptable. If not, please tell why. Thanks.
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Fri, 20 Jan 2017 15:17:02 GMT) Full text and rfc822 format available.Message #26 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Fri, 20 Jan 2017 16:16:34 +0100
Eli Zaretskii <eliz <at> gnu.org> writes: > I'm asking whether the (fixed) behavior, whereby using setq to change > the value to " -> " would add the ASCII string " -> " to the > minibuffer history, would be acceptable. If not, please tell why. Would be good, but we will have to modify also `query-replace--split-string` just for this purpose as it assume the separator is one char long and have a text property 'separator. I wanted to keep things as simple as possible in this patch. -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sat, 21 Jan 2017 10:07:01 GMT) Full text and rfc822 format available.Message #29 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sat, 21 Jan 2017 11:05:59 +0100
Eli Zaretskii <eliz <at> gnu.org> writes: > I'm asking whether the (fixed) behavior, whereby using setq to change > the value to " -> " would add the ASCII string " -> " to the > minibuffer history, would be acceptable. If not, please tell why. Here the patch updated adding the plain string to history when it is an ascii string. 1 file changed, 92 insertions(+), 74 deletions(-) lisp/replace.el | 166 +++++++++++++++++++++++++++++++------------------------- modified lisp/replace.el @@ -79,15 +79,17 @@ That becomes the \"string to replace\".") to the minibuffer that reads the string to replace, or invoke replacements from Isearch by using a key sequence like `C-s C-s M-%'." "24.3") -(defcustom query-replace-from-to-separator - (propertize (if (char-displayable-p ?→) " → " " -> ") - 'face 'minibuffer-prompt) - "String that separates FROM and TO in the history of replacement pairs." - ;; Avoids error when attempt to autoload char-displayable-p fails - ;; while preparing to dump, also stops customize-rogue listing this. - :initialize 'custom-initialize-delay +(defcustom query-replace-from-to-separator " → " + "String that separates FROM and TO in the history of replacement pairs. +When non-nil, the pair (FROM TO) will be added to `query-replace-history' as +a string \"FROM<separator>TO\". +When nil the default separator \" -> \" will be used as a plain string +and the pair will not be added to `query-replace-history' +\(Same behavior as in emacs 24.5)." :group 'matching - :type '(choice string (sexp :tag "Display specification")) + :type '(choice + (const :tag "Disabled" nil) + string) :version "25.1") (defcustom query-replace-from-history-variable 'query-replace-history @@ -150,14 +152,20 @@ See `replace-regexp' and `query-replace-regexp-eval'.") (mapconcat 'isearch-text-char-description string "")) (defun query-replace--split-string (string) - "Split string STRING at a character with property `separator'" + "Split string STRING at `query-replace-from-to-separator'. +Split at a character with property `separator' or at place matching regexp +`query-replace-from-to-separator'." (let* ((length (length string)) (split-pos (text-property-any 0 length 'separator t string))) - (if (not split-pos) - (substring-no-properties string) - (cl-assert (not (text-property-any (1+ split-pos) length 'separator t string))) - (cons (substring-no-properties string 0 split-pos) - (substring-no-properties string (1+ split-pos) length))))) + (cond (split-pos + (cl-assert (not (text-property-any + (1+ split-pos) length 'separator t string))) + (cons (substring-no-properties string 0 split-pos) + (substring-no-properties string (1+ split-pos) length))) + ((string-match query-replace-from-to-separator string) + (cons (substring-no-properties string 0 (match-beginning 0)) + (substring-no-properties string (match-end 0) length))) + (t (substring-no-properties string))))) (defun query-replace-read-from (prompt regexp-flag) "Query and return the `from' argument of a query-replace operation. @@ -165,66 +173,76 @@ The return value can also be a pair (FROM . TO) indicating that the user wants to replace FROM with TO." (if query-replace-interactive (car (if regexp-flag regexp-search-ring search-ring)) - ;; Reevaluating will check char-displayable-p that is - ;; unavailable while preparing to dump. - (custom-reevaluate-setting 'query-replace-from-to-separator) - (let* ((history-add-new-input nil) - (separator - (when query-replace-from-to-separator - (propertize "\0" - 'display query-replace-from-to-separator - 'separator t))) - (minibuffer-history - (append - (when separator - (mapcar (lambda (from-to) - (concat (query-replace-descr (car from-to)) - separator - (query-replace-descr (cdr from-to)))) - query-replace-defaults)) - (symbol-value query-replace-from-history-variable))) - (minibuffer-allow-text-properties t) ; separator uses text-properties - (prompt - (if (and query-replace-defaults separator) - (format "%s (default %s): " prompt (car minibuffer-history)) - (format "%s: " prompt))) - (from - ;; The save-excursion here is in case the user marks and copies - ;; a region in order to specify the minibuffer input. - ;; That should not clobber the region for the query-replace itself. - (save-excursion - (minibuffer-with-setup-hook - (lambda () - (setq-local text-property-default-nonsticky - (cons '(separator . t) text-property-default-nonsticky))) - (if regexp-flag - (read-regexp prompt nil 'minibuffer-history) - (read-from-minibuffer - prompt nil nil nil nil - (car (if regexp-flag regexp-search-ring search-ring)) t))))) - (to)) - (if (and (zerop (length from)) query-replace-defaults) - (cons (caar query-replace-defaults) - (query-replace-compile-replacement - (cdar query-replace-defaults) regexp-flag)) - (setq from (query-replace--split-string from)) - (when (consp from) (setq to (cdr from) from (car from))) - (add-to-history query-replace-from-history-variable from nil t) - ;; Warn if user types \n or \t, but don't reject the input. - (and regexp-flag - (string-match "\\(\\`\\|[^\\]\\)\\(\\\\\\\\\\)*\\(\\\\[nt]\\)" from) - (let ((match (match-string 3 from))) - (cond - ((string= match "\\n") - (message "Note: `\\n' here doesn't match a newline; to do that, type C-q C-j instead")) - ((string= match "\\t") - (message "Note: `\\t' here doesn't match a tab; to do that, just type TAB"))) - (sit-for 2))) - (if (not to) - from - (add-to-history query-replace-to-history-variable to nil t) - (add-to-history 'query-replace-defaults (cons from to) nil t) - (cons from (query-replace-compile-replacement to regexp-flag))))))) + (let ((sep-char (replace-regexp-in-string + " " "" query-replace-from-to-separator))) + (when (stringp query-replace-from-to-separator) + (setq query-replace-from-to-separator + (propertize (if (char-displayable-p (string-to-char sep-char)) + query-replace-from-to-separator + " -> ") + 'face 'minibuffer-prompt))) + (let* ((history-add-new-input nil) + (separator + (if (and query-replace-from-to-separator + (> (string-bytes sep-char) (length sep-char))) + (propertize "\0" + 'display query-replace-from-to-separator + 'separator t) + (propertize query-replace-from-to-separator 'separator t))) + (minibuffer-history + (append + (when separator + (mapcar (lambda (from-to) + (concat (query-replace-descr (car from-to)) + separator + (query-replace-descr (cdr from-to)))) + query-replace-defaults)) + (symbol-value query-replace-from-history-variable))) + (minibuffer-allow-text-properties t) ; separator uses text-properties + (prompt + (cond ((and query-replace-defaults separator) + (format "%s (default %s): " prompt (car minibuffer-history))) + (query-replace-defaults + (format "%s (default %s -> %s): " prompt + (query-replace-descr (caar query-replace-defaults)) + (query-replace-descr (cdar query-replace-defaults)))) + (t (format "%s: " prompt)))) + (from + ;; The save-excursion here is in case the user marks and copies + ;; a region in order to specify the minibuffer input. + ;; That should not clobber the region for the query-replace itself. + (save-excursion + (minibuffer-with-setup-hook + (lambda () + (setq-local text-property-default-nonsticky + (cons '(separator . t) text-property-default-nonsticky))) + (if regexp-flag + (read-regexp prompt nil 'minibuffer-history) + (read-from-minibuffer + prompt nil nil nil nil (car search-ring) t))))) + (to)) + (if (and (zerop (length from)) query-replace-defaults) + (cons (caar query-replace-defaults) + (query-replace-compile-replacement + (cdar query-replace-defaults) regexp-flag)) + (setq from (query-replace--split-string from)) + (when (consp from) (setq to (cdr from) from (car from))) + (add-to-history query-replace-from-history-variable from nil t) + ;; Warn if user types \n or \t, but don't reject the input. + (and regexp-flag + (string-match "\\(\\`\\|[^\\]\\)\\(\\\\\\\\\\)*\\(\\\\[nt]\\)" from) + (let ((match (match-string 3 from))) + (cond + ((string= match "\\n") + (message "Note: `\\n' here doesn't match a newline; to do that, type C-q C-j instead")) + ((string= match "\\t") + (message "Note: `\\t' here doesn't match a tab; to do that, just type TAB"))) + (sit-for 2))) + (if (not to) + from + (add-to-history query-replace-to-history-variable to nil t) + (add-to-history 'query-replace-defaults (cons from to) nil t) + (cons from (query-replace-compile-replacement to regexp-flag)))))))) (defun query-replace-compile-replacement (to regexp-flag) "Maybe convert a regexp replacement TO to Lisp. -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sat, 21 Jan 2017 10:25:01 GMT) Full text and rfc822 format available.Message #32 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sat, 21 Jan 2017 11:24:19 +0100
Thierry Volpiatto <thierry.volpiatto <at> gmail.com> writes: > Eli Zaretskii <eliz <at> gnu.org> writes: > >> I'm asking whether the (fixed) behavior, whereby using setq to change >> the value to " -> " would add the ASCII string " -> " to the >> minibuffer history, would be acceptable. If not, please tell why. > > Here the patch updated adding the plain string to history when it is an > ascii string. Which fail now when query-replace-from-to-separator is nil but I fixed it here (not resending new patch unless you ask for it). -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sun, 22 Jan 2017 19:07:02 GMT) Full text and rfc822 format available.Message #35 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sun, 22 Jan 2017 21:06:15 +0200
> From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> > Cc: 25482 <at> debbugs.gnu.org > Date: Fri, 20 Jan 2017 16:16:34 +0100 > > > Eli Zaretskii <eliz <at> gnu.org> writes: > > > I'm asking whether the (fixed) behavior, whereby using setq to change > > the value to " -> " would add the ASCII string " -> " to the > > minibuffer history, would be acceptable. If not, please tell why. > > Would be good, but we will have to modify also > `query-replace--split-string` just for this purpose as it assume the > separator is one char long and have a text property 'separator. I'm afraid we are mis-communicating. The current code uses a null character as the separator, giving it a display property and a separator property. The display property uses a non-ASCII character (→). What I wanted to suggest is to keep the same code, and just allow users to specify the ASCII string " -> " for the display property. That should be much simpler than your proposed patch, and I don't really see how it would add non-ASCII characters to your minibuffer history. It also eliminates the need for making changes in query-replace--split-string. What am I missing? Thanks.
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sun, 22 Jan 2017 20:15:02 GMT) Full text and rfc822 format available.Message #38 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sun, 22 Jan 2017 21:14:03 +0100
Eli Zaretskii <eliz <at> gnu.org> writes: > I'm afraid we are mis-communicating. Yes. > The current code uses a null character as the separator, giving it a > display property and a separator property. The display property uses > a non-ASCII character (→). What I wanted to suggest is to keep the > same code, and just allow users to specify the ASCII string " -> " for > the display property. No change is needed for this, it's what the current code does. > That should be much simpler than your proposed patch, What would be much more simple would be to not use at all properties, the actual implementation is too complex for something quite simple. I will not have much time to continue on this, I will look into this maybe later, if you think this change is not important, just close the bug for now (if you need the last patch I wrote which works fine, let me know). Thanks. -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sun, 22 Jan 2017 20:33:02 GMT) Full text and rfc822 format available.Message #41 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sun, 22 Jan 2017 22:32:32 +0200
> From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> > Cc: 25482 <at> debbugs.gnu.org > Date: Sun, 22 Jan 2017 21:14:03 +0100 > > What would be much more simple would be to not use at all properties, > the actual implementation is too complex for something quite simple. But why is it a problem in the first place? In any case, some change is still needed, I believe, because you said customization of this is not easy. Can you show a simple recipe that demonstrates the problems with customizing this option?
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Mon, 23 Jan 2017 08:14:01 GMT) Full text and rfc822 format available.Message #44 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Mon, 23 Jan 2017 09:13:31 +0100
Eli Zaretskii <eliz <at> gnu.org> writes: >> From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> >> Cc: 25482 <at> debbugs.gnu.org >> Date: Sun, 22 Jan 2017 21:14:03 +0100 >> >> What would be much more simple would be to not use at all properties, >> the actual implementation is too complex for something quite simple. > > But why is it a problem in the first place? The problem is that you endup in the history with a crap string, see https://github.com/emacs-helm/helm/issues/1667. > In any case, some change is still needed, I believe, because you said > customization of this is not easy. Can you show a simple recipe that > demonstrates the problems with customizing this option? I think just looking at the defcustom using a sexp which need delay in the defcustom to ensure char-displayable-p is ready to use and the need to reevaluate this defcustom at each call of query-replace-read-from explain all. This patch don't change the actual behavior, I have removed the fallback to emacs-24.5 behavior, so now setting query-replace-from-to-separator to nil just fallback to " -> ". With this patch the whole string is added to history. Also usage of setq is possible, with actual implementation it is not possible. Finally the implementation is much simpler not relaying on props. I will not have the time for further comments or modifications of patch, I leave it here for memory, if you want to merge it let me know. Thanks. 1 file changed, 28 insertions(+), 35 deletions(-) lisp/replace.el | 63 +++++++++++++++++++++++++-------------------------------- modified lisp/replace.el @@ -79,15 +79,12 @@ That becomes the \"string to replace\".") to the minibuffer that reads the string to replace, or invoke replacements from Isearch by using a key sequence like `C-s C-s M-%'." "24.3") -(defcustom query-replace-from-to-separator - (propertize (if (char-displayable-p ?→) " → " " -> ") - 'face 'minibuffer-prompt) - "String that separates FROM and TO in the history of replacement pairs." - ;; Avoids error when attempt to autoload char-displayable-p fails - ;; while preparing to dump, also stops customize-rogue listing this. - :initialize 'custom-initialize-delay +(defcustom query-replace-from-to-separator " → " + "String that separates FROM and TO in the history of replacement pairs. +When nil or the string provided not displayable the default separator \" -> \" +will be used instead." :group 'matching - :type '(choice string (sexp :tag "Display specification")) + :type '(choice string) :version "25.1") (defcustom query-replace-from-history-variable 'query-replace-history @@ -150,14 +147,12 @@ See `replace-regexp' and `query-replace-regexp-eval'.") (mapconcat 'isearch-text-char-description string "")) (defun query-replace--split-string (string) - "Split string STRING at a character with property `separator'" - (let* ((length (length string)) - (split-pos (text-property-any 0 length 'separator t string))) - (if (not split-pos) - (substring-no-properties string) - (cl-assert (not (text-property-any (1+ split-pos) length 'separator t string))) - (cons (substring-no-properties string 0 split-pos) - (substring-no-properties string (1+ split-pos) length))))) + "Split string STRING at `query-replace-from-to-separator'." + (let ((separator (or query-replace-from-to-separator " -> "))) + (cond ((string-match separator string) + (cons (substring-no-properties string 0 (match-beginning 0)) + (substring-no-properties string (match-end 0)))) + (t (substring-no-properties string))))) (defun query-replace-read-from (prompt regexp-flag) "Query and return the `from' argument of a query-replace operation. @@ -165,43 +160,41 @@ The return value can also be a pair (FROM . TO) indicating that the user wants to replace FROM with TO." (if query-replace-interactive (car (if regexp-flag regexp-search-ring search-ring)) - ;; Reevaluating will check char-displayable-p that is - ;; unavailable while preparing to dump. - (custom-reevaluate-setting 'query-replace-from-to-separator) (let* ((history-add-new-input nil) - (separator - (when query-replace-from-to-separator - (propertize "\0" - 'display query-replace-from-to-separator - 'separator t))) + (sep-char (and (stringp query-replace-from-to-separator) + (replace-regexp-in-string + " " "" query-replace-from-to-separator))) + (separator (propertize + (if (and sep-char + (char-displayable-p (string-to-char sep-char))) + query-replace-from-to-separator + " -> ") + 'face 'minibuffer-prompt)) (minibuffer-history (append - (when separator (mapcar (lambda (from-to) (concat (query-replace-descr (car from-to)) separator (query-replace-descr (cdr from-to)))) - query-replace-defaults)) + query-replace-defaults) (symbol-value query-replace-from-history-variable))) - (minibuffer-allow-text-properties t) ; separator uses text-properties (prompt - (if (and query-replace-defaults separator) - (format "%s (default %s): " prompt (car minibuffer-history)) + (if query-replace-defaults + (format "%s (default %s %s %s): " + prompt + (query-replace-descr (caar query-replace-defaults)) + separator + (query-replace-descr (cdar query-replace-defaults))) (format "%s: " prompt))) (from ;; The save-excursion here is in case the user marks and copies ;; a region in order to specify the minibuffer input. ;; That should not clobber the region for the query-replace itself. (save-excursion - (minibuffer-with-setup-hook - (lambda () - (setq-local text-property-default-nonsticky - (cons '(separator . t) text-property-default-nonsticky))) (if regexp-flag (read-regexp prompt nil 'minibuffer-history) (read-from-minibuffer - prompt nil nil nil nil - (car (if regexp-flag regexp-search-ring search-ring)) t))))) + prompt nil nil nil nil (car search-ring) t)))) (to)) (if (and (zerop (length from)) query-replace-defaults) (cons (caar query-replace-defaults) -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sat, 28 Jan 2017 06:51:02 GMT) Full text and rfc822 format available.Message #47 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sat, 28 Jan 2017 07:50:19 +0100
Just to not leave this unfinished, here the last patch I did which simplify code. - defcustom is simple (string or nil). - No need to reevaluate defcustom at every time. - setq is usable. - splitting is now simple with all the text property dance removed. - plain string is added to history instead of a crap string. 1 file changed, 29 insertions(+), 37 deletions(-) lisp/replace.el | 66 +++++++++++++++++++++++++-------------------------------- modified lisp/replace.el @@ -79,15 +79,12 @@ That becomes the \"string to replace\".") to the minibuffer that reads the string to replace, or invoke replacements from Isearch by using a key sequence like `C-s C-s M-%'." "24.3") -(defcustom query-replace-from-to-separator - (propertize (if (char-displayable-p ?→) " → " " -> ") - 'face 'minibuffer-prompt) - "String that separates FROM and TO in the history of replacement pairs." - ;; Avoids error when attempt to autoload char-displayable-p fails - ;; while preparing to dump, also stops customize-rogue listing this. - :initialize 'custom-initialize-delay +(defcustom query-replace-from-to-separator " → " + "String that separates FROM and TO in the history of replacement pairs. +When nil or the string provided not displayable the default separator \" -> \" +will be used instead." :group 'matching - :type '(choice string (sexp :tag "Display specification")) + :type '(choice string) :version "25.1") (defcustom query-replace-from-history-variable 'query-replace-history @@ -149,15 +146,12 @@ See `replace-regexp' and `query-replace-regexp-eval'.") (defun query-replace-descr (string) (mapconcat 'isearch-text-char-description string "")) -(defun query-replace--split-string (string) - "Split string STRING at a character with property `separator'" - (let* ((length (length string)) - (split-pos (text-property-any 0 length 'separator t string))) - (if (not split-pos) - (substring-no-properties string) - (cl-assert (not (text-property-any (1+ split-pos) length 'separator t string))) - (cons (substring-no-properties string 0 split-pos) - (substring-no-properties string (1+ split-pos) length))))) +(defun query-replace--split-string (string separator) + "Split string STRING at `query-replace-from-to-separator'." + (cond ((string-match separator string) + (cons (substring-no-properties string 0 (match-beginning 0)) + (substring-no-properties string (match-end 0)))) + (t (substring-no-properties string)))) (defun query-replace-read-from (prompt regexp-flag) "Query and return the `from' argument of a query-replace operation. @@ -165,49 +159,47 @@ The return value can also be a pair (FROM . TO) indicating that the user wants to replace FROM with TO." (if query-replace-interactive (car (if regexp-flag regexp-search-ring search-ring)) - ;; Reevaluating will check char-displayable-p that is - ;; unavailable while preparing to dump. - (custom-reevaluate-setting 'query-replace-from-to-separator) (let* ((history-add-new-input nil) - (separator - (when query-replace-from-to-separator - (propertize "\0" - 'display query-replace-from-to-separator - 'separator t))) + (sep-char (and (stringp query-replace-from-to-separator) + (replace-regexp-in-string + " " "" query-replace-from-to-separator))) + (separator (propertize + (if (and sep-char + (char-displayable-p (string-to-char sep-char))) + query-replace-from-to-separator + " -> ") + 'face 'minibuffer-prompt)) (minibuffer-history (append - (when separator (mapcar (lambda (from-to) (concat (query-replace-descr (car from-to)) separator (query-replace-descr (cdr from-to)))) - query-replace-defaults)) + query-replace-defaults) (symbol-value query-replace-from-history-variable))) - (minibuffer-allow-text-properties t) ; separator uses text-properties (prompt - (if (and query-replace-defaults separator) - (format "%s (default %s): " prompt (car minibuffer-history)) + (if query-replace-defaults + (format "%s (default %s %s %s): " + prompt + (query-replace-descr (caar query-replace-defaults)) + separator + (query-replace-descr (cdar query-replace-defaults))) (format "%s: " prompt))) (from ;; The save-excursion here is in case the user marks and copies ;; a region in order to specify the minibuffer input. ;; That should not clobber the region for the query-replace itself. (save-excursion - (minibuffer-with-setup-hook - (lambda () - (setq-local text-property-default-nonsticky - (cons '(separator . t) text-property-default-nonsticky))) (if regexp-flag (read-regexp prompt nil 'minibuffer-history) (read-from-minibuffer - prompt nil nil nil nil - (car (if regexp-flag regexp-search-ring search-ring)) t))))) + prompt nil nil nil nil (car search-ring) t)))) (to)) (if (and (zerop (length from)) query-replace-defaults) (cons (caar query-replace-defaults) (query-replace-compile-replacement (cdar query-replace-defaults) regexp-flag)) - (setq from (query-replace--split-string from)) + (setq from (query-replace--split-string from separator)) (when (consp from) (setq to (cdr from) from (car from))) (add-to-history query-replace-from-history-variable from nil t) ;; Warn if user types \n or \t, but don't reject the input. -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sat, 28 Jan 2017 07:32:02 GMT) Full text and rfc822 format available.Message #50 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sat, 28 Jan 2017 09:31:38 +0200
> From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> > Cc: 25482 <at> debbugs.gnu.org > Date: Sat, 28 Jan 2017 07:50:19 +0100 > > Just to not leave this unfinished, here the last patch I did which > simplify code. > > - defcustom is simple (string or nil). > - No need to reevaluate defcustom at every time. > - setq is usable. > - splitting is now simple with all the text property dance removed. > - plain string is added to history instead of a crap string. Thanks, but I don't understand why you want to get rid of the display property. (I also object to calling the result of that "crap".) I'd rather we left the display property and its supporting code intact, and only moved the char-displayable-p call to the place where the string is used instead of having it in the defcustom, thus allowing customizations with setq. That should solve the real problems with the current code, leaving the style-related issues alone. I realize that you don't like that style, but I think we should respect style preferences of others and not change their code just because we would have written it differently.
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sat, 28 Jan 2017 10:44:01 GMT) Full text and rfc822 format available.Message #53 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sat, 28 Jan 2017 11:43:48 +0100
Eli Zaretskii <eliz <at> gnu.org> writes: > Thanks, but I don't understand why you want to get rid of the display > property. This complicate the code and force external apps to handle this, see https://github.com/emacs-helm/helm/commit/2b8576c0705145b5d133e41478976eef5b3d7390 > (I also object to calling the result of that "crap".) Sorry, but it is: Selecting foo → bar in a list results in foo^@bar being entered into the minibuffer, not with the query-replace interface but when handling the history list from somewhere else. > I'd rather we left the display property and its supporting code > intact, and only moved the char-displayable-p call to the place where > the string is used instead of having it in the defcustom, thus > allowing customizations with setq. That should solve the real > problems with the current code, leaving the style-related issues > alone. It doesn't see above. > I realize that you don't like that style, but I think we should > respect style preferences of others and not change their code just > because we would have written it differently. The patch change nothing and allow same customization as before. I wrote this patch because I think it makes the code better, allow easier customization and fix issue described above. I understand if you want to keep the code as it is to be sure nothing wrong is introduced in a code that is already "working". Thanks. -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sat, 28 Jan 2017 11:02:01 GMT) Full text and rfc822 format available.Message #56 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sat, 28 Jan 2017 13:01:36 +0200
> From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> > Cc: 25482 <at> debbugs.gnu.org > Date: Sat, 28 Jan 2017 11:43:48 +0100 > > Eli Zaretskii <eliz <at> gnu.org> writes: > > > Thanks, but I don't understand why you want to get rid of the display > > property. > > This complicate the code and force external apps to handle this, see > https://github.com/emacs-helm/helm/commit/2b8576c0705145b5d133e41478976eef5b3d7390 Maybe I'm missing something, but the above is no different from the need to adapt to any new feature in core Emacs. > Selecting foo → bar in a list results in foo^@bar being entered into the > minibuffer, not with the query-replace interface but when handling the > history list from somewhere else. Yes, the external packages need to adapt to this, if they want to use the internal features. I don't think this can be avoided in general.
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sat, 28 Jan 2017 11:25:02 GMT) Full text and rfc822 format available.Message #59 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sat, 28 Jan 2017 12:23:54 +0100
Eli Zaretskii <eliz <at> gnu.org> writes: > Yes, the external packages need to adapt to this, if they want to use > the internal features. I don't think this can be avoided in general. What can be avoided is writing complex code when simple code can be used for the same result. -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sat, 28 Jan 2017 13:51:01 GMT) Full text and rfc822 format available.Message #62 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sat, 28 Jan 2017 15:50:28 +0200
> From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> > Cc: 25482 <at> debbugs.gnu.org > Date: Sat, 28 Jan 2017 12:23:54 +0100 > > > Yes, the external packages need to adapt to this, if they want to use > > the internal features. I don't think this can be avoided in general. > > What can be avoided is writing complex code when simple code can be used > for the same result. Maybe so, but that ship has sailed more than 2 years ago, and was preceded by a long discussion. I didn't read all of it now, but AFAIU, the reason for using a character with a display property was to make the separator not editable when the user edits the history. So that design decision and the resulting code we have now had some justification, and any replacement will have to support the same features, which I think a simple string will not do.
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sat, 28 Jan 2017 16:47:01 GMT) Full text and rfc822 format available.Message #65 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sat, 28 Jan 2017 17:46:29 +0100
Eli Zaretskii <eliz <at> gnu.org> writes: >> From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> >> Cc: 25482 <at> debbugs.gnu.org >> Date: Sat, 28 Jan 2017 12:23:54 +0100 >> >> > Yes, the external packages need to adapt to this, if they want to use >> > the internal features. I don't think this can be avoided in general. >> >> What can be avoided is writing complex code when simple code can be used >> for the same result. > > Maybe so, but that ship has sailed more than 2 years ago, and was > preceded by a long discussion. I didn't read all of it now, but > AFAIU, the reason for using a character with a display property was to > make the separator not editable when the user edits the history. Ok, I understand, don't think many people are editing history file and people doing so, I guess known what they are doing. > So that design decision and the resulting code we have now had some > justification, and any replacement will have to support the same > features, which I think a simple string will not do. Ok, thanks for explanation, perhaps you can close this bug. -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sun, 29 Jan 2017 01:04:01 GMT) Full text and rfc822 format available.Message #68 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Juri Linkov <juri <at> linkov.net> To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> Cc: 25482 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sun, 29 Jan 2017 02:59:24 +0200
>> So that design decision and the resulting code we have now had some >> justification, and any replacement will have to support the same >> features, which I think a simple string will not do. > > Ok, thanks for explanation, perhaps you can close this bug. Please don't close this bug. I think your idea for customization was good. We just need to find the right implementation that will keep the current default intact, at the same time allowing you to customize the string or set it to nil.
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sun, 29 Jan 2017 03:43:01 GMT) Full text and rfc822 format available.Message #71 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Juri Linkov <juri <at> linkov.net> Cc: 25482 <at> debbugs.gnu.org, thierry.volpiatto <at> gmail.com Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sun, 29 Jan 2017 05:42:35 +0200
> From: Juri Linkov <juri <at> linkov.net> > Cc: Eli Zaretskii <eliz <at> gnu.org>, 25482 <at> debbugs.gnu.org > Date: Sun, 29 Jan 2017 02:59:24 +0200 > > >> So that design decision and the resulting code we have now had some > >> justification, and any replacement will have to support the same > >> features, which I think a simple string will not do. > > > > Ok, thanks for explanation, perhaps you can close this bug. > > Please don't close this bug. I think your idea for customization was good. > We just need to find the right implementation that will keep the current > default intact, at the same time allowing you to customize the string > or set it to nil. I intended to move the char-displayable-p call from the defcustom, and remove the re-evaluate call. But if you want to handle this part, it would be even better. It's just that you've been very silent on this issue until now. Thanks.
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Mon, 30 Jan 2017 00:35:02 GMT) Full text and rfc822 format available.Message #74 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Juri Linkov <juri <at> linkov.net> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org, thierry.volpiatto <at> gmail.com Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Mon, 30 Jan 2017 02:16:10 +0200
>> >> So that design decision and the resulting code we have now had some >> >> justification, and any replacement will have to support the same >> >> features, which I think a simple string will not do. >> > >> > Ok, thanks for explanation, perhaps you can close this bug. >> >> Please don't close this bug. I think your idea for customization was good. >> We just need to find the right implementation that will keep the current >> default intact, at the same time allowing you to customize the string >> or set it to nil. > > I intended to move the char-displayable-p call from the defcustom, and > remove the re-evaluate call. But if you want to handle this part, it > would be even better. It's just that you've been very silent on this > issue until now. I've been silent because I agreed to your replies to Thierry :) At one point in the beginning of this thread I believe Thierry sent a patch closest to what would be good to have (before “code simplification” where development went astray). We could return to that patch and test if it does these things right: instead of re-evaluating checks char-displayable-p, but when query-replace-from-to-separator is nil falls back to pre-24.5 that adds strings to the history without ^@.
Juri Linkov <juri <at> linkov.net>
:Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
:Message #79 received at 25482-done <at> debbugs.gnu.org (full text, mbox):
From: Juri Linkov <juri <at> linkov.net> To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> Cc: 25482-done <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Mon, 13 Feb 2017 02:37:56 +0200
>> So that design decision and the resulting code we have now had some >> justification, and any replacement will have to support the same >> features, which I think a simple string will not do. > > Ok, thanks for explanation, perhaps you can close this bug. Thanks, Thierry, I installed your second patch (from 20 Jan) with some modifications. As for your idea of using plain ASCII separators without text properties, the problem is that it will not work if the user wants to replace the same string as is used for the separator, e.g. to replace → with -> in the current buffer. If you have more ideas how to make the separator unambiguous without using text properties, then please reopen this bug.
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Mon, 13 Feb 2017 02:25:02 GMT) Full text and rfc822 format available.Message #82 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Glenn Morris <rgm <at> gnu.org> To: 25482 <at> debbugs.gnu.org Cc: thierry.volpiatto <at> gmail.com, juri <at> linkov.net Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sun, 12 Feb 2017 21:24:14 -0500
Hi, this doesn't seem to bootstrap: http://hydra.nixos.org/build/48432485
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Mon, 13 Feb 2017 03:05:02 GMT) Full text and rfc822 format available.Message #85 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Glenn Morris <rgm <at> gnu.org> To: 25482 <at> debbugs.gnu.org Cc: thierry.volpiatto <at> gmail.com, juri <at> linkov.net Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sun, 12 Feb 2017 22:04:17 -0500
Never mind, it seems there was a trivial fix (ef6132c55fc).
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Mon, 13 Feb 2017 05:52:02 GMT) Full text and rfc822 format available.Message #88 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Juri Linkov <juri <at> linkov.net> Cc: 25482 <at> debbugs.gnu.org, thierry.volpiatto <at> gmail.com Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Mon, 13 Feb 2017 07:51:49 +0200
> From: Juri Linkov <juri <at> linkov.net> > Cc: Eli Zaretskii <eliz <at> gnu.org>, 25482-done <at> debbugs.gnu.org > Date: Mon, 13 Feb 2017 02:37:56 +0200 > > >> So that design decision and the resulting code we have now had some > >> justification, and any replacement will have to support the same > >> features, which I think a simple string will not do. > > > > Ok, thanks for explanation, perhaps you can close this bug. > > Thanks, Thierry, I installed your second patch (from 20 Jan) > with some modifications. Thanks, but please also update NEWS and the Emacs manual with the information about the new nil value of the separator. (Maybe it's a good idea to describe in the ELisp manual the structure of query-replace-history elements?)
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Mon, 13 Feb 2017 05:54:02 GMT) Full text and rfc822 format available.Message #91 received at 25482-done <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> To: Juri Linkov <juri <at> linkov.net> Cc: 25482-done <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Mon, 13 Feb 2017 06:53:36 +0100
Juri Linkov <juri <at> linkov.net> writes: > Thanks, Thierry, I installed your second patch (from 20 Jan) > with some modifications. Thanks. > As for your idea of using plain ASCII separators without text properties, > the problem is that it will not work if the user wants to replace > the same string as is used for the separator, e.g. to replace > → with -> in the current buffer. So yes, this is a good reason. -- Thierry
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Tue, 14 Feb 2017 00:10:01 GMT) Full text and rfc822 format available.Message #94 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Juri Linkov <juri <at> linkov.net> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org, thierry.volpiatto <at> gmail.com Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Tue, 14 Feb 2017 02:04:45 +0200
> Thanks, but please also update NEWS and the Emacs manual with the > information about the new nil value of the separator. I updated NEWS, but not the Emacs manual because the new nil value is useful only for some old users (who want its old behavior), not for new users. > (Maybe it's a good idea to describe in the ELisp manual the structure > of query-replace-history elements?) Currently the structure of query-replace-history is very simple, but in bug#25591 we are going to find a solution to store more meta-information in the replacement history that might require documenting it in the ELisp manual.
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Tue, 14 Feb 2017 00:25:01 GMT) Full text and rfc822 format available.Message #97 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Drew Adams <drew.adams <at> oracle.com> To: Juri Linkov <juri <at> linkov.net>, Eli Zaretskii <eliz <at> gnu.org> Cc: 25482 <at> debbugs.gnu.org, thierry.volpiatto <at> gmail.com Subject: RE: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Mon, 13 Feb 2017 16:24:41 -0800 (PST)
> > Thanks, but please also update NEWS and the Emacs manual with the > > information about the new nil value of the separator. > > I updated NEWS, but not the Emacs manual because the new nil value > is useful only for some old users (who want its old behavior), > not for new users. Why would you suppose that? The behavior, which includes nil as a possible value, should be described. Let users decide whether this or that value is better for them. Please document the option correctly.
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Thu, 16 Feb 2017 21:06:01 GMT) Full text and rfc822 format available.Message #100 received at 25482 <at> debbugs.gnu.org (full text, mbox):
From: Juri Linkov <juri <at> linkov.net> To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com> Cc: 25482 <at> debbugs.gnu.org Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Thu, 16 Feb 2017 22:43:08 +0200
>> As for your idea of using plain ASCII separators without text properties, >> the problem is that it will not work if the user wants to replace >> the same string as is used for the separator, e.g. to replace >> → with -> in the current buffer. > > So yes, this is a good reason. Wait, maybe you are right in the idea to use the separator string as is, instead of the null character. We still need to use text properties to unambiguously mark separator boundaries, so we can't rely on regexp matching to find separator boundaries. But at least “ → ” is not more frequent string to replace than the null character. Putting special text properties on the separator itself instead of the null character might be more beneficial for such situations where the separator is exposed without text properties, e.g. when copying the contents of the minibuffer. However, one drawback is that searching in the replacement minibuffer history for the same string as the separator (e.g. “→”) will find matches in the separator, not only in replacement strings. This patch demonstrates what we could do. It still uses the ‘display’ property because I see no other simple way to make the separator string intangible. diff --git a/lisp/replace.el b/lisp/replace.el index b96c883..6600ff6 100644 --- a/lisp/replace.el +++ b/lisp/replace.el @@ -149,14 +149,17 @@ query-replace-descr (mapconcat 'isearch-text-char-description string "")) (defun query-replace--split-string (string) - "Split string STRING at a character with property `separator'" + "Split string STRING at a substring with property `separator'" (let* ((length (length string)) (split-pos (text-property-any 0 length 'separator t string))) (if (not split-pos) (substring-no-properties string) - (cl-assert (not (text-property-any (1+ split-pos) length 'separator t string))) (cons (substring-no-properties string 0 split-pos) - (substring-no-properties string (1+ split-pos) length))))) + (substring-no-properties + string (or (text-property-not-all + (1+ split-pos) length 'separator t string) + length) + length))))) (defun query-replace-read-from (prompt regexp-flag) "Query and return the `from' argument of a query-replace operation. @@ -165,17 +168,19 @@ query-replace-read-from (if query-replace-interactive (car (if regexp-flag regexp-search-ring search-ring)) (let* ((history-add-new-input nil) + (separator-string + (when query-replace-from-to-separator + ;; Check if the first non-whitespace char is displayable + (if (char-displayable-p + (string-to-char (replace-regexp-in-string + " " "" query-replace-from-to-separator))) + query-replace-from-to-separator + " -> "))) (separator - (when query-replace-from-to-separator - (propertize "\0" - 'display - (propertize - (if (char-displayable-p - (string-to-char (replace-regexp-in-string - " " "" query-replace-from-to-separator))) - query-replace-from-to-separator - " -> ") - 'face 'minibuffer-prompt) + (when separator-string + (propertize separator-string + 'display separator-string + 'face 'minibuffer-prompt 'separator t))) (minibuffer-history (append @@ -203,7 +210,8 @@ query-replace-read-from (minibuffer-with-setup-hook (lambda () (setq-local text-property-default-nonsticky - (cons '(separator . t) text-property-default-nonsticky))) + (append '((separator . t) (face . t)) + text-property-default-nonsticky))) (if regexp-flag (read-regexp prompt nil 'minibuffer-history) (read-from-minibuffer
bug-gnu-emacs <at> gnu.org
:bug#25482
; Package emacs
.
(Sat, 18 Feb 2017 10:45:02 GMT) Full text and rfc822 format available.Message #103 received at 25482-done <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Drew Adams <drew.adams <at> oracle.com> Cc: 25482-done <at> debbugs.gnu.org, thierry.volpiatto <at> gmail.com, juri <at> linkov.net Subject: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Date: Sat, 18 Feb 2017 12:45:13 +0200
> Date: Mon, 13 Feb 2017 16:24:41 -0800 (PST) > From: Drew Adams <drew.adams <at> oracle.com> > Cc: 25482 <at> debbugs.gnu.org, thierry.volpiatto <at> gmail.com > > > > Thanks, but please also update NEWS and the Emacs manual with the > > > information about the new nil value of the separator. > > > > I updated NEWS, but not the Emacs manual because the new nil value > > is useful only for some old users (who want its old behavior), > > not for new users. > > Why would you suppose that? The behavior, which includes nil as > a possible value, should be described. Let users decide whether > this or that value is better for them. Please document the option > correctly. Done.
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org
.
(Sat, 18 Mar 2017 11:24:05 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.