* bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' @ 2015-05-28 21:12 Drew Adams 2015-06-01 20:53 ` Juri Linkov 2015-06-02 13:08 ` Kaushal 0 siblings, 2 replies; 12+ messages in thread From: Drew Adams @ 2015-05-28 21:12 UTC (permalink / raw) To: 20687 You can bind any key you like in `query-replace-map'. But `perform-replace', in effect, hard-codes the keys that it recognizes. This is not necessary. You shbould be able to bind a new key in that map to some command, and have `perform-replace' invoke that command. All that's required for this is to add another `cond' clause, just before the final `t' (otherwise) clause, to do this: (cond ... (def (call-interactively def)) ; User-defined key - invoke it. (t ;; Note: we do not need to treat `exit-prefix' ;; specially here, since we reread ;; any unrecognized character. ...)) It seems silly for the code to be written like it is. We already look up the key you press in the q-r keymap. If we find a DEF that is not one of those predefined by Emacs then we ignore it? That makes no sense (to me). In GNU Emacs 25.0.50.1 (i686-pc-mingw32) of 2014-10-20 on LEG570 Bzr revision: 118168 rgm@gnu.org-20141020195941-icp42t8ttcnud09g Windowing system distributor `Microsoft Corp.', version 6.1.7601 Configured using: `configure --enable-checking=yes,glyphs CPPFLAGS=-DGLYPH_DEBUG=1' ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' 2015-05-28 21:12 bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' Drew Adams @ 2015-06-01 20:53 ` Juri Linkov 2015-06-01 21:11 ` Drew Adams 2015-06-02 13:08 ` Kaushal 1 sibling, 1 reply; 12+ messages in thread From: Juri Linkov @ 2015-06-01 20:53 UTC (permalink / raw) To: Drew Adams; +Cc: 20687 > You can bind any key you like in `query-replace-map'. But > `perform-replace', in effect, hard-codes the keys that it recognizes. > > This is not necessary. You shbould be able to bind a new key in that > map to some command, and have `perform-replace' invoke that command. > > All that's required for this is to add another `cond' clause, just > before the final `t' (otherwise) clause, to do this: > > (cond > ... > (def (call-interactively def)) ; User-defined key - invoke it. > (t > ;; Note: we do not need to treat `exit-prefix' > ;; specially here, since we reread > ;; any unrecognized character. > ...)) > > It seems silly for the code to be written like it is. We already look > up the key you press in the q-r keymap. If we find a DEF that is not > one of those predefined by Emacs then we ignore it? That makes no sense > (to me). Could you please send an example of your custom keybindings in `query-replace-map' that currently don't work. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' 2015-06-01 20:53 ` Juri Linkov @ 2015-06-01 21:11 ` Drew Adams 2015-06-02 22:01 ` Juri Linkov 0 siblings, 1 reply; 12+ messages in thread From: Drew Adams @ 2015-06-01 21:11 UTC (permalink / raw) To: Juri Linkov; +Cc: 20687 > Could you please send an example of your custom keybindings in > `query-replace-map' that currently don't work. I don't have any custom keybindings in `query-replace-map' that don't work (in fact, I don't have any custom bindings in that map at all). This bug report came from this emacs.StackExchange answer - see the discussion in the comments. http://emacs.stackexchange.com/a/12781/105. The aim here was to add `C' to `query-replace-map', to have it toggle `case-fold-search'. But it doesn't matter what key a user might want to bind to what command during q-r. The point is that a user can do that (that's what keymaps and key bindings are for), but currently `perform-replace' refuses to recognize such a key and its command. There is no good reason for this, AFAICT. It should be OK for a user to do this. Of course, that doesn't update the doc string to reflect the new key and its action, but that's all. At the user level, this should be something that users can do easily, without needing to perform surgery. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' 2015-06-01 21:11 ` Drew Adams @ 2015-06-02 22:01 ` Juri Linkov 2015-06-02 22:12 ` Drew Adams 2020-09-17 18:11 ` Lars Ingebrigtsen 0 siblings, 2 replies; 12+ messages in thread From: Juri Linkov @ 2015-06-02 22:01 UTC (permalink / raw) To: Drew Adams; +Cc: 20687 > The point is that a user can do that (that's what keymaps and > key bindings are for), but currently `perform-replace' refuses to > recognize such a key and its command. I see there are already commands (as opposed to special internal values such as `act' and `skip') in query-replace-map: (define-key map "\C-v" 'scroll-up) (define-key map "\M-v" 'scroll-down) (define-key map [next] 'scroll-up) (define-key map [prior] 'scroll-down) (define-key map [?\C-\M-v] 'scroll-other-window) (define-key map [M-next] 'scroll-other-window) (define-key map [?\C-\M-\S-v] 'scroll-other-window-down) (define-key map [M-prior] 'scroll-other-window-down) These bindings look like real commands intended to be called interactively, so after enabling this feature in query-replace they will start doing their job which is good. The only suggestion I have is to check whether the binding is a command before trying to call it, i.e.: diff --git a/lisp/replace.el b/lisp/replace.el index 1bf1343..503531a 100644 --- a/lisp/replace.el +++ b/lisp/replace.el @@ -2404,6 +2404,8 @@ (defun perform-replace (from-string replacements (replace-dehighlight) (save-excursion (recursive-edit)) (setq replaced t)) + ((commandp def t) + (call-interactively def)) ;; Note: we do not need to treat `exit-prefix' ;; specially here, since we reread ;; any unrecognized character. ^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' 2015-06-02 22:01 ` Juri Linkov @ 2015-06-02 22:12 ` Drew Adams 2020-09-17 18:11 ` Lars Ingebrigtsen 1 sibling, 0 replies; 12+ messages in thread From: Drew Adams @ 2015-06-02 22:12 UTC (permalink / raw) To: Juri Linkov; +Cc: 20687 > These bindings look like real commands intended to be called > interactively, so after enabling this feature in query-replace > they will start doing their job which is good. > > The only suggestion I have is to check whether the binding > is a command before trying to call it, i.e.: Good idea. Thx. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' 2015-06-02 22:01 ` Juri Linkov 2015-06-02 22:12 ` Drew Adams @ 2020-09-17 18:11 ` Lars Ingebrigtsen 1 sibling, 0 replies; 12+ messages in thread From: Lars Ingebrigtsen @ 2020-09-17 18:11 UTC (permalink / raw) To: Juri Linkov; +Cc: 20687, Kaushal Juri Linkov <juri@linkov.net> writes: > I see there are already commands (as opposed to special internal > values such as `act' and `skip') in query-replace-map: > > (define-key map "\C-v" 'scroll-up) > (define-key map "\M-v" 'scroll-down) > (define-key map [next] 'scroll-up) > (define-key map [prior] 'scroll-down) > (define-key map [?\C-\M-v] 'scroll-other-window) > (define-key map [M-next] 'scroll-other-window) > (define-key map [?\C-\M-\S-v] 'scroll-other-window-down) > (define-key map [M-prior] 'scroll-other-window-down) > > These bindings look like real commands intended to be called > interactively, so after enabling this feature in query-replace > they will start doing their job which is good. [...] > + ((commandp def t) > + (call-interactively def)) I'm not sure I quite understand Kaushal's proposed patch here, and how it relates to the problem Drew describes, but Juri's patch here seems like the right thing, at least? As far as I can tell, it was never applied. So I've applied it to Emacs 28, and I'm closing this bug report. If there are other related things to be done in this area, it might be better to open a separate bug report and restate what the requested feature is. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' 2015-05-28 21:12 bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' Drew Adams 2015-06-01 20:53 ` Juri Linkov @ 2015-06-02 13:08 ` Kaushal 2015-06-02 22:02 ` Juri Linkov 1 sibling, 1 reply; 12+ messages in thread From: Kaushal @ 2015-06-02 13:08 UTC (permalink / raw) To: 20687; +Cc: juri [-- Attachment #1: Type: text/plain, Size: 2111 bytes --] Hi guys, I tried the fix as Drew suggested and it works great. Patch: --- replace.el 2015-06-02 09:04:57.944380000 -0400 +++ replace-editted.el 2015-06-02 09:08:22.038682000 -0400 @@ -2099,12 +2099,11 @@ ;; Data for the next match. If a cons, it has the same format as ;; (match-data); otherwise it is t if a match is possible at point. (match-again t) - (message (if query-flag (apply 'propertize (substitute-command-keys - "Query replacing %s with %s: (\\<query-replace-map>\\[help] for help) ") + "%sQuery replacing %s with %s: (\\<query-replace-map>\\[help] for help) ") minibuffer-prompt-properties)))) ;; If region is active, in Transient Mark mode, operate on region. @@ -2275,6 +2274,8 @@ nocasify literal)) next-replacement))) (message message + ;; Show whether `case-fold-search' is `t' or `nil' + (if case-fold-search "[case] " "[CaSe] ") (query-replace-descr from-string) (query-replace-descr replacement-presentation))) (setq key (read-event)) @@ -2404,6 +2405,7 @@ (replace-dehighlight) (save-excursion (recursive-edit)) (setq replaced t)) + (def (call-interactively def)) ; User-defined key - invoke it. ;; Note: we do not need to treat `exit-prefix' ;; specially here, since we reread ;; any unrecognized character. Here is then how I add a new binding to the query-replace-map: ;; http://emacs.stackexchange.com/a/12781/115 (defun drew/toggle-case () "Toggle the value of `case-fold-search' between `nil' and non-nil." (interactive) ;; `case-fold-search' automatically becomes buffer-local when set (setq case-fold-search (not case-fold-search))) (define-key query-replace-map (kbd "C") #'drew/toggle-case) ( https://github.com/kaushalmodi/.emacs.d/blob/e690fddc7176368b3d25b0d34ec02510ee92503a/setup-files/setup-search.el#L22-L28 ) -- Kaushal Modi [-- Attachment #2: Type: text/html, Size: 6911 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' 2015-06-02 13:08 ` Kaushal @ 2015-06-02 22:02 ` Juri Linkov 2015-06-02 22:50 ` Drew Adams 0 siblings, 1 reply; 12+ messages in thread From: Juri Linkov @ 2015-06-02 22:02 UTC (permalink / raw) To: Kaushal; +Cc: 20687 > I tried the fix as Drew suggested and it works great. > > Patch: > > --- replace.el 2015-06-02 09:04:57.944380000 -0400 > +++ replace-editted.el 2015-06-02 09:08:22.038682000 -0400 > @@ -2099,12 +2099,11 @@ > ;; Data for the next match. If a cons, it has the same format as > ;; (match-data); otherwise it is t if a match is possible at > point. > (match-again t) > - > (message > (if query-flag > (apply 'propertize > (substitute-command-keys > - "Query replacing %s with %s: (\\<query-replace-map>\\[help] for help) ") > + "%sQuery replacing %s with %s: (\\<query-replace-map>\\[help] for help) ") > minibuffer-prompt-properties)))) > > ;; If region is active, in Transient Mark mode, operate on region. > @@ -2275,6 +2274,8 @@ > nocasify literal)) > next-replacement))) > (message message > + ;; Show whether `case-fold-search' is `t' or `nil' > + (if case-fold-search "[case] " "[CaSe] ") > (query-replace-descr from-string) > (query-replace-descr Maybe we should use the same message about case-folding like in isearch? ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' 2015-06-02 22:02 ` Juri Linkov @ 2015-06-02 22:50 ` Drew Adams 2015-06-03 3:35 ` Kaushal 0 siblings, 1 reply; 12+ messages in thread From: Drew Adams @ 2015-06-02 22:50 UTC (permalink / raw) To: Juri Linkov, Kaushal; +Cc: 20687 > > + ;; Show whether `case-fold-search' is `t' or `nil' > > + (if case-fold-search "[case] " "[CaSe] ") > > Maybe we should use the same message about case-folding like in > isearch? The msg should somehow indicate that what is involved here is (only) case-sensitivity wrt FROM (i.e., wrt search, not replacement). Not sure what the best way to do that would be. IOW, there is more than one use of case sensitivity here, unlike the case for search. There is what `case-fold-search' controls (the search), and there is what `case-replace' controls (the replacement). And then there is what happens for the replacement according to the case of FROM. --- BTW, we might consider binding a key to toggle case sensitivity for search as part of this bug fix (i.e., not just fixing `perform-replace' so it respects keys that user might bind). In that case, maybe the same key we use in Isearch (`M-c') would be a good choice. --- BTW2, I think that Emacs manual node `Replacement and Case' is confusing. The first three paragraphs (2/3 of the node), for instance: If the first argument of a replace command is all lower case, the command ignores case while searching for occurrences to replace--provided `case-fold-search' is non-`nil'. If `case-fold-search' is set to `nil', case is always significant in all searches. An upper-case letter anywhere in the incremental search string makes the search case-sensitive. Thus, searching for `Foo' does not find `foo' or `FOO'. This applies to regular expression search as well as to string search. The effect ceases if you delete the upper-case letter from the search string. If you set the variable `case-fold-search' to `nil', then all letters must match exactly, including case. This is a per-buffer variable; altering the variable normally affects only the current buffer, unless you change its default value. *Note Locals::. This variable applies to nonincremental searches also, including those performed by the replace commands (*note Replace::) and the minibuffer history matching commands (*note Minibuffer History::). These paragraphs really say only that the search part of replace commands acts normally: `case-fold-search' governs. They should be removed or changed to say just that. Leaving them as they are just confuses readers, IMO. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' 2015-06-02 22:50 ` Drew Adams @ 2015-06-03 3:35 ` Kaushal 2015-06-03 4:39 ` Drew Adams 0 siblings, 1 reply; 12+ messages in thread From: Kaushal @ 2015-06-03 3:35 UTC (permalink / raw) To: Drew Adams, Juri Linkov; +Cc: 20687 [-- Attachment #1: Type: text/plain, Size: 10524 bytes --] In that case, the patch becomes much more complicated. We have to modify the query-replace-map in replace.el itself as we cannot access the internal variables required to printed this message in isearch-style with (sit-for 1): (message query-replace-in-progress-message (query-replace-descr from-string) (query-replace-descr replacement-presentation) query-message-momentary) Here: from-string, replacement-presentation are internal variables and cannot be used in a function defined outside that (cond ..) form. So the earlier approach to define a function externally to toggle the case and to bind that to query-replace-map from outside does not apply (if we want to flash the case fold toggle info momentarily as done in isearch). Even the replacement-presentation was in a (let .. ) form not accessible to the (cond ..) form and so I moved it to an outer (let ..) form as seen in the below patch. I tested this out and the M-c and M-r bindings work great. It now also gives clear info on what the user should expect after that binding is used. Please give it a try. I have still kept this line (def (call-interactively def)) ; User-defined key, invoke it. as it could be useful to bind any other function from outside that does not need internal variables. --- replace.el 2015-06-02 23:21:42.631715000 -0400 +++ replace-editted.el 2015-06-02 23:32:47.754001000 -0400 @@ -1834,6 +1834,8 @@ (define-key map [M-next] 'scroll-other-window) (define-key map [?\C-\M-\S-v] 'scroll-other-window-down) (define-key map [M-prior] 'scroll-other-window-down) + (define-key map "\M-c" 'toggle-query-case) + (define-key map "\M-r" 'toggle-replace-preserve-case) ;; Binding ESC would prohibit the M-v binding. Instead, callers ;; should check for ESC specially. ;; (define-key map "\e" 'exit-prefix) @@ -2100,12 +2102,14 @@ ;; (match-data); otherwise it is t if a match is possible at point. (match-again t) - (message + (query-replace-in-progress-message (if query-flag (apply 'propertize (substitute-command-keys - "Query replacing %s with %s: (\\<query-replace-map>\\[help] for help) ") - minibuffer-prompt-properties)))) + (concat "Query replacing %s with %s: " + "(\\<query-replace-map>\\[help] for help) %s ")) + minibuffer-prompt-properties))) + (query-message-momentary "")) ;; If region is active, in Transient Mark mode, operate on region. (if backward @@ -2251,7 +2255,7 @@ noedit real-match-data backward) replace-count (1+ replace-count))) (undo-boundary) - (let (done replaced key def) + (let (done replaced key def replacement-presentation) ;; Loop reading commands until one of them sets done, ;; which means it has finished handling this ;; occurrence. Any command that sets `done' should @@ -2266,17 +2270,18 @@ regexp-flag delimited-flag case-fold-search backward) ;; Bind message-log-max so we don't fill up the message log ;; with a bunch of identical messages. - (let ((message-log-max nil) - (replacement-presentation - (if query-replace-show-replacement - (save-match-data - (set-match-data real-match-data) - (match-substitute-replacement next-replacement - nocasify literal)) - next-replacement))) - (message message + (let ((message-log-max nil)) + (setq replacement-presentation + (if query-replace-show-replacement + (save-match-data + (set-match-data real-match-data) + (match-substitute-replacement next-replacement + nocasify literal)) + next-replacement)) + (message query-replace-in-progress-message (query-replace-descr from-string) - (query-replace-descr replacement-presentation))) + (query-replace-descr replacement-presentation) + query-message-momentary)) (setq key (read-event)) ;; Necessary in case something happens during read-event ;; that clobbers the match data. @@ -2404,6 +2409,51 @@ (replace-dehighlight) (save-excursion (recursive-edit)) (setq replaced t)) + + ((eq def 'toggle-query-case) + (setq case-fold-search (not case-fold-search)) + (let ((message-log-max nil) + (query-message-momentary + (concat "[" + (if case-fold-search + "case insensitive search" + "Case Sensitive Search") + "]"))) + (message query-replace-in-progress-message + (query-replace-descr from-string) + (query-replace-descr replacement-presentation) + query-message-momentary) + (sit-for 1))) + + ((eq def 'toggle-replace-preserve-case) + (let ((message-log-max nil) + (nocasify-value-reason "") + query-message-momentary) + (setq nocasify (not nocasify)) + (cond + ((null case-fold-search) + (setq nocasify nil) + (setq nocasify-value-reason ", as case-fold-search is nil")) + ((null (isearch-no-upper-case-p from-string regexp-flag)) + (setq nocasify nil) + (setq nocasify-value-reason ", as FROM-STRING has an upper case char.")) + ((null (isearch-no-upper-case-p next-replacement regexp-flag)) + (setq nocasify t) + (setq nocasify-value-reason ", as REPLACEMENT has an upper case char."))) + (setq query-message-momentary + (concat "[Replaced text case will " + (if nocasify "NOT " "") + "be preserved" + nocasify-value-reason + "]")) + (message query-replace-in-progress-message + (query-replace-descr from-string) + (query-replace-descr replacement-presentation) + query-message-momentary) + (sit-for 1.5))) + + (def (call-interactively def)) ; User-defined key, invoke it. + ;; Note: we do not need to treat `exit-prefix' ;; specially here, since we reread ;; any unrecognized character. On Tue, Jun 2, 2015 at 6:51 PM Drew Adams <drew.adams@oracle.com> wrote: > > > + ;; Show whether `case-fold-search' is `t' or `nil' > > > + (if case-fold-search "[case] " "[CaSe] ") > > > > Maybe we should use the same message about case-folding like in > > isearch? > > The msg should somehow indicate that what is involved here is (only) > case-sensitivity wrt FROM (i.e., wrt search, not replacement). Not > sure what the best way to do that would be. > > IOW, there is more than one use of case sensitivity here, unlike > the case for search. There is what `case-fold-search' controls (the > search), and there is what `case-replace' controls (the replacement). > And then there is what happens for the replacement according to the > case of FROM. > > --- > > BTW, we might consider binding a key to toggle case sensitivity for > search as part of this bug fix (i.e., not just fixing `perform-replace' > so it respects keys that user might bind). In that case, maybe the > same key we use in Isearch (`M-c') would be a good choice. > > --- > > BTW2, I think that Emacs manual node `Replacement and Case' is confusing. > The first three paragraphs (2/3 of the node), for instance: > > If the first argument of a replace command is all lower case, the > command ignores case while searching for occurrences to > replace--provided `case-fold-search' is non-`nil'. If > `case-fold-search' is set to `nil', case is always significant in all > searches. > > An upper-case letter anywhere in the incremental search string makes > the search case-sensitive. Thus, searching for `Foo' does not find > `foo' or `FOO'. This applies to regular expression search as well as > to string search. The effect ceases if you delete the upper-case > letter from the search string. > > If you set the variable `case-fold-search' to `nil', then all > letters must match exactly, including case. This is a per-buffer > variable; altering the variable normally affects only the current > buffer, unless you change its default value. *Note Locals::. This > variable applies to nonincremental searches also, including those > performed by the replace commands (*note Replace::) and the minibuffer > history matching commands (*note Minibuffer History::). > > These paragraphs really say only that the search part of replace commands > acts normally: `case-fold-search' governs. They should be removed or > changed to say just that. Leaving them as they are just confuses readers, > IMO. > [-- Attachment #2: Type: text/html, Size: 13185 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' 2015-06-03 3:35 ` Kaushal @ 2015-06-03 4:39 ` Drew Adams 2015-06-03 5:10 ` Kaushal 0 siblings, 1 reply; 12+ messages in thread From: Drew Adams @ 2015-06-03 4:39 UTC (permalink / raw) To: Kaushal, Juri Linkov; +Cc: 20687 > I tested this out and the M-c and M-r bindings work great. It now > also gives clear info on what the user should expect after that > binding is used. Please give it a try. I have still kept this line > > (def (call-interactively def)) ; User-defined key, invoke it. > > as it could be useful to bind any other function from outside > that does not need internal variables. 1. I'm OK with whatever you guys come up with. Thanks for working on this. 2. I tried it only a little. When I tried `M-r': * If the replacement string had uppercase chars then I always got the same message, which was very long - too long to read in the short time it was displayed. Could we shorten that message, please? And could we maybe have it logged to *Messages*, so that if someone doesn't have time to read it s?he can look it up? * If the replacement string had no uppercase chars then I always got the same message (about case-fold-search being nil). What is `M-r' really supposed to do? I don't see how it is a toggle, if repeating it always gives the same message, given the same replacement string. Can you describe what the toggling or cycling among states is supposed to do/mean? 3. Wrt this: I have still kept this line (def (call-interactively def)) ; User-defined key, invoke it. as it could be useful to bind any other function from outside that does not need internal variables. I think Juri is right, that it should be the following, because `lookup-key' can return a number if the key is too long: ((commandp def t) ; User-defined key, invoke it. (call-interactively def)) 4. If one of you could replace the paragraphs of the doc that I mentioned by just a statement that search is controlled by `case-fold-search', that would be good. You could then add that you can toggle this using `M-c' etc. IOW, (1) those paragraphs are useless, and (2) now we have something more to say about case sensitivity. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' 2015-06-03 4:39 ` Drew Adams @ 2015-06-03 5:10 ` Kaushal 0 siblings, 0 replies; 12+ messages in thread From: Kaushal @ 2015-06-03 5:10 UTC (permalink / raw) To: Drew Adams, Juri Linkov; +Cc: 20687 [-- Attachment #1: Type: text/plain, Size: 5105 bytes --] > If the replacement string had uppercase chars then I always got the same message, which was very long - too long to read in the short time it was displayed. Could we shorten that message, please? Yes, I am looking for more ideas to get a better, shorter message. > And could we maybe have it logged to *Messages*, so that if someone doesn't have time to read it s?he can look it up? Only for the messages where toggling is not possible, the message can be logged to *Messages*. Sounds good? > If the replacement string had no uppercase chars then I always got the same message (about case-fold-search being nil). The toggling is not unconditional. Toggling case-replace/nocasify is very picky! So I had to put that (cond ..) statement there to handle the picky scenarios where toggling cannot happen even if the user wanted to. For the above case, nocasify will stay t regardless of the value of case-replace IF the user has set case-fold-search to nil. So the user will first need to do M-c (toggle case-fold-search to t) and then do M-r. That too will not work IF the user has used upper case letter in the search/regexp string or the replacement string. This is the ideal case for M-r to always toggle nocasify 1. case-fold-search is t 2. all lower case in search/regexp string 3. all lower case in replacement string > What is `M-r' really supposed to do? I don't see how it is a toggle, if repeating it always gives the same message, given the same replacement string. Can you describe what the toggling or cycling among states is supposed to do/mean? As described above, we cannot unconditionally toggle nocasify.. it depends on a bunch of conditions to be right. > I think Juri is right, that it should be the following, because `lookup-key' can return a number if the key is too long: ((commandp def t) ; User-defined key, invoke it. (call-interactively def)) I agree. Will make the change. > If one of you could replace the paragraphs of the doc that I mentioned by just a statement that search is controlled by `case-fold-search', that would be good. You could then add that you can toggle this using `M-c' etc. IOW, (1) those paragraphs are useless, and (2) now we have something more to say about case sensitivity. Case fold toggling is also a bit picky but the results are obvious, and M-c can force toggle case-fold-search. But default, search-upper-case is t. So if the user has a string with an upper case in the search field of query-replace, case-fold-search will be set to nil automatically (even if it is `t` by default). Then M-r will not work in the beginning. User can, though, use M-c to toggle case-fold-search first and then M-r if they wish. I found the current documentation useful while working on this patch and testing it out. But I will give it a one more read to try to improve it. On Wed, Jun 3, 2015 at 12:39 AM Drew Adams <drew.adams@oracle.com> wrote: > > I tested this out and the M-c and M-r bindings work great. It now > > also gives clear info on what the user should expect after that > > binding is used. Please give it a try. I have still kept this line > > > > (def (call-interactively def)) ; User-defined key, invoke it. > > > > as it could be useful to bind any other function from outside > > that does not need internal variables. > > 1. I'm OK with whatever you guys come up with. Thanks for working > on this. > > 2. I tried it only a little. When I tried `M-r': > > * If the replacement string had uppercase chars then I always > got the same message, which was very long - too long to read > in the short time it was displayed. Could we shorten that > message, please? And could we maybe have it logged to > *Messages*, so that if someone doesn't have time to read it > s?he can look it up? > > * If the replacement string had no uppercase chars then I always > got the same message (about case-fold-search being nil). > > What is `M-r' really supposed to do? I don't see how it is a > toggle, if repeating it always gives the same message, given > the same replacement string. Can you describe what the toggling > or cycling among states is supposed to do/mean? > > 3. Wrt this: > > I have still kept this line > (def (call-interactively def)) ; User-defined key, invoke it. > as it could be useful to bind any other function from outside > that does not need internal variables. > > I think Juri is right, that it should be the following, because > `lookup-key' can return a number if the key is too long: > > ((commandp def t) ; User-defined key, invoke it. > (call-interactively def)) > > 4. If one of you could replace the paragraphs of the doc that I > mentioned by just a statement that search is controlled by > `case-fold-search', that would be good. You could then add > that you can toggle this using `M-c' etc. IOW, (1) those > paragraphs are useless, and (2) now we have something more > to say about case sensitivity. > [-- Attachment #2: Type: text/html, Size: 6533 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-09-17 18:11 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-05-28 21:12 bug#20687: 25.0.50; `perform-replace' should invoke a key that you have bound in `query-replace-map' Drew Adams 2015-06-01 20:53 ` Juri Linkov 2015-06-01 21:11 ` Drew Adams 2015-06-02 22:01 ` Juri Linkov 2015-06-02 22:12 ` Drew Adams 2020-09-17 18:11 ` Lars Ingebrigtsen 2015-06-02 13:08 ` Kaushal 2015-06-02 22:02 ` Juri Linkov 2015-06-02 22:50 ` Drew Adams 2015-06-03 3:35 ` Kaushal 2015-06-03 4:39 ` Drew Adams 2015-06-03 5:10 ` Kaushal
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).