* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
@ 2017-01-19 8:18 Thierry Volpiatto
2017-01-19 16:01 ` Eli Zaretskii
0 siblings, 1 reply; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-19 8:18 UTC (permalink / raw)
To: 25482
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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-19 8:18 bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Thierry Volpiatto
@ 2017-01-19 16:01 ` Eli Zaretskii
[not found] ` <87vatac0a2.fsf@gmail.com>
2017-01-20 5:54 ` Thierry Volpiatto
0 siblings, 2 replies; 32+ messages in thread
From: Eli Zaretskii @ 2017-01-19 16:01 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 25482
> From: Thierry Volpiatto <thierry.volpiatto@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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: Fwd: Re: bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
[not found] ` <87vatac0a2.fsf@gmail.com>
@ 2017-01-20 5:42 ` Thierry Volpiatto
2017-01-20 7:56 ` Eli Zaretskii
1 sibling, 0 replies; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-20 5:42 UTC (permalink / raw)
To: 25482
Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:
> Eli Zaretskii <eliz@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-19 16:01 ` Eli Zaretskii
[not found] ` <87vatac0a2.fsf@gmail.com>
@ 2017-01-20 5:54 ` Thierry Volpiatto
1 sibling, 0 replies; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-20 5:54 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
[not found] ` <87vatac0a2.fsf@gmail.com>
2017-01-20 5:42 ` bug#25482: Fwd: " Thierry Volpiatto
@ 2017-01-20 7:56 ` Eli Zaretskii
2017-01-20 11:35 ` Thierry Volpiatto
1 sibling, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2017-01-20 7:56 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 25482
[You've replied only to me.]
> From: Thierry Volpiatto <thierry.volpiatto@gmail.com>
> Date: Thu, 19 Jan 2017 20:33:41 +0100
>
>
> Eli Zaretskii <eliz@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?
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-20 7:56 ` Eli Zaretskii
@ 2017-01-20 11:35 ` Thierry Volpiatto
2017-01-20 13:06 ` Eli Zaretskii
0 siblings, 1 reply; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-20 11:35 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482
Eli Zaretskii <eliz@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-20 11:35 ` Thierry Volpiatto
@ 2017-01-20 13:06 ` Eli Zaretskii
2017-01-20 15:16 ` Thierry Volpiatto
2017-01-21 10:05 ` Thierry Volpiatto
0 siblings, 2 replies; 32+ messages in thread
From: Eli Zaretskii @ 2017-01-20 13:06 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 25482
> From: Thierry Volpiatto <thierry.volpiatto@gmail.com>
> Cc: 25482@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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-20 13:06 ` Eli Zaretskii
@ 2017-01-20 15:16 ` Thierry Volpiatto
2017-01-22 19:06 ` Eli Zaretskii
2017-01-21 10:05 ` Thierry Volpiatto
1 sibling, 1 reply; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-20 15:16 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482
Eli Zaretskii <eliz@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-20 13:06 ` Eli Zaretskii
2017-01-20 15:16 ` Thierry Volpiatto
@ 2017-01-21 10:05 ` Thierry Volpiatto
2017-01-21 10:24 ` Thierry Volpiatto
1 sibling, 1 reply; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-21 10:05 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482
Eli Zaretskii <eliz@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-21 10:05 ` Thierry Volpiatto
@ 2017-01-21 10:24 ` Thierry Volpiatto
0 siblings, 0 replies; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-21 10:24 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482
Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:
> Eli Zaretskii <eliz@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-20 15:16 ` Thierry Volpiatto
@ 2017-01-22 19:06 ` Eli Zaretskii
2017-01-22 20:14 ` Thierry Volpiatto
0 siblings, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2017-01-22 19:06 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 25482
> From: Thierry Volpiatto <thierry.volpiatto@gmail.com>
> Cc: 25482@debbugs.gnu.org
> Date: Fri, 20 Jan 2017 16:16:34 +0100
>
>
> Eli Zaretskii <eliz@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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-22 19:06 ` Eli Zaretskii
@ 2017-01-22 20:14 ` Thierry Volpiatto
2017-01-22 20:32 ` Eli Zaretskii
0 siblings, 1 reply; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-22 20:14 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482
Eli Zaretskii <eliz@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-22 20:14 ` Thierry Volpiatto
@ 2017-01-22 20:32 ` Eli Zaretskii
2017-01-23 8:13 ` Thierry Volpiatto
0 siblings, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2017-01-22 20:32 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 25482
> From: Thierry Volpiatto <thierry.volpiatto@gmail.com>
> Cc: 25482@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?
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-22 20:32 ` Eli Zaretskii
@ 2017-01-23 8:13 ` Thierry Volpiatto
2017-01-28 6:50 ` Thierry Volpiatto
0 siblings, 1 reply; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-23 8:13 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Thierry Volpiatto <thierry.volpiatto@gmail.com>
>> Cc: 25482@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-23 8:13 ` Thierry Volpiatto
@ 2017-01-28 6:50 ` Thierry Volpiatto
2017-01-28 7:31 ` Eli Zaretskii
0 siblings, 1 reply; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-28 6:50 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-28 6:50 ` Thierry Volpiatto
@ 2017-01-28 7:31 ` Eli Zaretskii
2017-01-28 10:43 ` Thierry Volpiatto
0 siblings, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2017-01-28 7:31 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 25482
> From: Thierry Volpiatto <thierry.volpiatto@gmail.com>
> Cc: 25482@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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-28 7:31 ` Eli Zaretskii
@ 2017-01-28 10:43 ` Thierry Volpiatto
2017-01-28 11:01 ` Eli Zaretskii
0 siblings, 1 reply; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-28 10:43 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482
Eli Zaretskii <eliz@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-28 10:43 ` Thierry Volpiatto
@ 2017-01-28 11:01 ` Eli Zaretskii
2017-01-28 11:23 ` Thierry Volpiatto
0 siblings, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2017-01-28 11:01 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 25482
> From: Thierry Volpiatto <thierry.volpiatto@gmail.com>
> Cc: 25482@debbugs.gnu.org
> Date: Sat, 28 Jan 2017 11:43:48 +0100
>
> Eli Zaretskii <eliz@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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-28 11:01 ` Eli Zaretskii
@ 2017-01-28 11:23 ` Thierry Volpiatto
2017-01-28 13:50 ` Eli Zaretskii
0 siblings, 1 reply; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-28 11:23 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482
Eli Zaretskii <eliz@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-28 11:23 ` Thierry Volpiatto
@ 2017-01-28 13:50 ` Eli Zaretskii
2017-01-28 16:46 ` Thierry Volpiatto
0 siblings, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2017-01-28 13:50 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 25482
> From: Thierry Volpiatto <thierry.volpiatto@gmail.com>
> Cc: 25482@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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-28 13:50 ` Eli Zaretskii
@ 2017-01-28 16:46 ` Thierry Volpiatto
[not found] ` <87k29ey93n.fsf@mail.linkov.net>
2017-02-13 0:37 ` Juri Linkov
0 siblings, 2 replies; 32+ messages in thread
From: Thierry Volpiatto @ 2017-01-28 16:46 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Thierry Volpiatto <thierry.volpiatto@gmail.com>
>> Cc: 25482@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
[not found] ` <87k29ey93n.fsf@mail.linkov.net>
@ 2017-01-29 3:42 ` Eli Zaretskii
2017-01-30 0:16 ` Juri Linkov
0 siblings, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2017-01-29 3:42 UTC (permalink / raw)
To: Juri Linkov; +Cc: 25482, thierry.volpiatto
> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>, 25482@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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-29 3:42 ` Eli Zaretskii
@ 2017-01-30 0:16 ` Juri Linkov
0 siblings, 0 replies; 32+ messages in thread
From: Juri Linkov @ 2017-01-30 0:16 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482, thierry.volpiatto
>> >> 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 ^@.
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-01-28 16:46 ` Thierry Volpiatto
[not found] ` <87k29ey93n.fsf@mail.linkov.net>
@ 2017-02-13 0:37 ` Juri Linkov
2017-02-13 2:24 ` Glenn Morris
` (2 more replies)
1 sibling, 3 replies; 32+ messages in thread
From: Juri Linkov @ 2017-02-13 0:37 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 25482-done
>> 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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-02-13 0:37 ` Juri Linkov
@ 2017-02-13 2:24 ` Glenn Morris
2017-02-13 3:04 ` Glenn Morris
2017-02-13 5:51 ` Eli Zaretskii
2017-02-13 5:53 ` Thierry Volpiatto
2 siblings, 1 reply; 32+ messages in thread
From: Glenn Morris @ 2017-02-13 2:24 UTC (permalink / raw)
To: 25482; +Cc: juri, thierry.volpiatto
Hi, this doesn't seem to bootstrap:
http://hydra.nixos.org/build/48432485
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-02-13 2:24 ` Glenn Morris
@ 2017-02-13 3:04 ` Glenn Morris
0 siblings, 0 replies; 32+ messages in thread
From: Glenn Morris @ 2017-02-13 3:04 UTC (permalink / raw)
To: 25482; +Cc: juri, thierry.volpiatto
Never mind, it seems there was a trivial fix (ef6132c55fc).
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-02-13 0:37 ` Juri Linkov
2017-02-13 2:24 ` Glenn Morris
@ 2017-02-13 5:51 ` Eli Zaretskii
2017-02-14 0:04 ` Juri Linkov
2017-02-13 5:53 ` Thierry Volpiatto
2 siblings, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2017-02-13 5:51 UTC (permalink / raw)
To: Juri Linkov; +Cc: 25482, thierry.volpiatto
> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>, 25482-done@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?)
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-02-13 0:37 ` Juri Linkov
2017-02-13 2:24 ` Glenn Morris
2017-02-13 5:51 ` Eli Zaretskii
@ 2017-02-13 5:53 ` Thierry Volpiatto
2017-02-16 20:43 ` Juri Linkov
2 siblings, 1 reply; 32+ messages in thread
From: Thierry Volpiatto @ 2017-02-13 5:53 UTC (permalink / raw)
To: Juri Linkov; +Cc: 25482-done
Juri Linkov <juri@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-02-13 5:51 ` Eli Zaretskii
@ 2017-02-14 0:04 ` Juri Linkov
2017-02-14 0:24 ` Drew Adams
0 siblings, 1 reply; 32+ messages in thread
From: Juri Linkov @ 2017-02-14 0:04 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 25482, thierry.volpiatto
> 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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-02-14 0:04 ` Juri Linkov
@ 2017-02-14 0:24 ` Drew Adams
2017-02-18 10:45 ` Eli Zaretskii
0 siblings, 1 reply; 32+ messages in thread
From: Drew Adams @ 2017-02-14 0:24 UTC (permalink / raw)
To: Juri Linkov, Eli Zaretskii; +Cc: 25482, thierry.volpiatto
> > 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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-02-13 5:53 ` Thierry Volpiatto
@ 2017-02-16 20:43 ` Juri Linkov
0 siblings, 0 replies; 32+ messages in thread
From: Juri Linkov @ 2017-02-16 20:43 UTC (permalink / raw)
To: Thierry Volpiatto; +Cc: 25482
>> 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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil
2017-02-14 0:24 ` Drew Adams
@ 2017-02-18 10:45 ` Eli Zaretskii
0 siblings, 0 replies; 32+ messages in thread
From: Eli Zaretskii @ 2017-02-18 10:45 UTC (permalink / raw)
To: Drew Adams; +Cc: 25482-done, juri, thierry.volpiatto
> Date: Mon, 13 Feb 2017 16:24:41 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 25482@debbugs.gnu.org, thierry.volpiatto@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.
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2017-02-18 10:45 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-19 8:18 bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil Thierry Volpiatto
2017-01-19 16:01 ` Eli Zaretskii
[not found] ` <87vatac0a2.fsf@gmail.com>
2017-01-20 5:42 ` bug#25482: Fwd: " Thierry Volpiatto
2017-01-20 7:56 ` Eli Zaretskii
2017-01-20 11:35 ` Thierry Volpiatto
2017-01-20 13:06 ` Eli Zaretskii
2017-01-20 15:16 ` Thierry Volpiatto
2017-01-22 19:06 ` Eli Zaretskii
2017-01-22 20:14 ` Thierry Volpiatto
2017-01-22 20:32 ` Eli Zaretskii
2017-01-23 8:13 ` Thierry Volpiatto
2017-01-28 6:50 ` Thierry Volpiatto
2017-01-28 7:31 ` Eli Zaretskii
2017-01-28 10:43 ` Thierry Volpiatto
2017-01-28 11:01 ` Eli Zaretskii
2017-01-28 11:23 ` Thierry Volpiatto
2017-01-28 13:50 ` Eli Zaretskii
2017-01-28 16:46 ` Thierry Volpiatto
[not found] ` <87k29ey93n.fsf@mail.linkov.net>
2017-01-29 3:42 ` Eli Zaretskii
2017-01-30 0:16 ` Juri Linkov
2017-02-13 0:37 ` Juri Linkov
2017-02-13 2:24 ` Glenn Morris
2017-02-13 3:04 ` Glenn Morris
2017-02-13 5:51 ` Eli Zaretskii
2017-02-14 0:04 ` Juri Linkov
2017-02-14 0:24 ` Drew Adams
2017-02-18 10:45 ` Eli Zaretskii
2017-02-13 5:53 ` Thierry Volpiatto
2017-02-16 20:43 ` Juri Linkov
2017-01-21 10:05 ` Thierry Volpiatto
2017-01-21 10:24 ` Thierry Volpiatto
2017-01-20 5:54 ` Thierry Volpiatto
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.