GNU bug report logs - #25482
26.0.50; Allow setting `query-replace-from-to-separator` to nil

Previous Next

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


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#25482; Package emacs. (Thu, 19 Jan 2017 08:41:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Thierry Volpiatto <thierry.volpiatto <at> gmail.com>:
New bug report received and forwarded. Copy sent to 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




Information forwarded to 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.




Information forwarded to 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




Information forwarded to 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




Information forwarded to 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?




Information forwarded to 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




Information forwarded to 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.




Information forwarded to 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




Information forwarded to 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




Information forwarded to 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




Information forwarded to 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.




Information forwarded to 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




Information forwarded to 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?




Information forwarded to 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




Information forwarded to 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




Information forwarded to 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.




Information forwarded to 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




Information forwarded to 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.




Information forwarded to 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




Information forwarded to 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.




Information forwarded to 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




Information forwarded to 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.




Information forwarded to 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.




Information forwarded to 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 ^@.




Reply sent to Juri Linkov <juri <at> linkov.net>:
You have taken responsibility. (Mon, 13 Feb 2017 00:41:02 GMT) Full text and rfc822 format available.

Notification sent to Thierry Volpiatto <thierry.volpiatto <at> gmail.com>:
bug acknowledged by developer. (Mon, 13 Feb 2017 00:41:02 GMT) Full text and rfc822 format available.

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.




Information forwarded to 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




Information forwarded to 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).




Information forwarded to 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?)




Information forwarded to 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




Information forwarded to 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.




Information forwarded to 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.




Information forwarded to 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




Information forwarded to 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.




bug archived. Request was from 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.

This bug report was last modified 8 years and 174 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.