unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#39512: 28.0.50; Add command isearch-yank-region
@ 2020-02-08 18:04 Tino Calancha
  2020-02-08 23:47 ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-02-09  0:31 ` Juri Linkov
  0 siblings, 2 replies; 21+ messages in thread
From: Tino Calancha @ 2020-02-08 18:04 UTC (permalink / raw)
  To: 39512; +Cc: spacibba, npostavs, juri, contovob

Severity: wishlist
Tags: patch
X-Debbugs-Cc: spacibba@aol.com,juri@linkov.net,drew.adams@oracle.com,npostavs@gmail.com,contovob@tcd.ie,eliz@gnu.org

I wish to have this command; it naturally completes other
isearch-yank-... cases.

This topic has been already discussed in the following links:

https://lists.gnu.org/archive/html/emacs-devel/2019-04/msg01125.html
https://lists.gnu.org/archive/html/emacs-devel/2019-05/msg00003.html

Note that in those threads there were plenty of discussions; here I'd
like to focus just in this proposed command.  If needed, I'd recommend
to open further bugs to discuss about other commands.

I let this command to start the interactive search if we are
not already there; from the above links I realized that
such a functionality was also missing.  This is consistent with
`isearch-yank-kill' (I think we should mention that in its docstring).


--8<-----------------------------cut here---------------start------------->8---
commit 1ea1939929fbf22c6d635b075cfcb2b77a4b8243
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Sat Feb 8 18:39:44 2020 +0100

    Add command isearch-yank-region
    
    During an incremental search, this command appends the region
    to the search string.  Otherwise, start an incremental search
    using the region as the search string.
    
    * lisp/isearch.el (isearch-yank-region): New command; bound to 'M-.'
    in isearch-mode-map.
    
    * doc/emacs/search.texi (Isearch Yank): Document it.
    * etc/NEWS: Announce this change.

diff --git a/doc/emacs/search.texi b/doc/emacs/search.texi
index 16916617a2..b443300b6d 100644
--- a/doc/emacs/search.texi
+++ b/doc/emacs/search.texi
@@ -285,6 +285,11 @@ Isearch Yank
 a prefix numeric argument of @var{n}, the command appends everything
 from point to the @var{n}th occurrence of the specified character.
 
+@kindex M-. @r{(Incremental search)}
+@findex isearch-yank-region
+  Likewise, @kbd{M-.} (@code{isearch-yank-region}) appends to
+the search string the selected region.
+
 @kindex C-y @r{(Incremental search)}
 @kindex M-y @r{(Incremental search)}
 @kindex mouse-2 @r{in the minibuffer (Incremental search)}
diff --git a/etc/NEWS b/etc/NEWS
index 55c1a47fbf..cf63176124 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -91,6 +91,13 @@ shows equivalent key bindings for all commands that have them.
 \f
 * Changes in Specialized Modes and Packages in Emacs 28.1
 
+** Search and Replace
+
++++
+*** New isearch bindings.
+'M-.' invokes new fnction 'isearch-yank-region', which yanks the selected
+region into the search string.
+
 ** Help
 
 +++
diff --git a/lisp/isearch.el b/lisp/isearch.el
index ddf9190dc6..266c311c3d 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -514,6 +514,9 @@ isearch-menu-bar-yank-map
     (define-key map [isearch-yank-kill]
       '(menu-item "Current kill" isearch-yank-kill
                   :help "Append current kill to search string"))
+    (define-key map [isearch-yank-region]
+      '(menu-item "Active region" isearch-yank-region
+                  :help "Append active region to search string"))
     (define-key map [isearch-yank-until-char]
       '(menu-item "Until char..." isearch-yank-until-char
                   :help "Yank from point to specified character into search string"))
@@ -708,6 +711,7 @@ isearch-mode-map
     (define-key map "\M-\C-d" 'isearch-del-char)
     (define-key map "\M-\C-y" 'isearch-yank-char)
     (define-key map    "\C-y" 'isearch-yank-kill)
+    (define-key map    "\M-." 'isearch-yank-region)
     (define-key map "\M-\C-z" 'isearch-yank-until-char)
     (define-key map "\M-s\C-e" 'isearch-yank-line)
 
@@ -1007,6 +1011,7 @@ isearch-forward
 Type \\[isearch-yank-line] to yank rest of line onto end of search string\
  and search for it.
 Type \\[isearch-yank-kill] to yank the last string of killed text.
+Type \\[isearch-yank-region] to yank the active region.
 Type \\[isearch-yank-pop] to replace string just yanked into search prompt
  with string killed before it.
 Type \\[isearch-quote-char] to quote control character to search for it.
@@ -2473,6 +2478,17 @@ isearch-yank-kill
   (unless isearch-mode (isearch-mode t))
   (isearch-yank-string (current-kill 0)))
 
+(defun isearch-yank-region ()
+  "Pull region into search string.
+If called out of an incremental search, then start an incremental
+search with the region as the search string."
+  (interactive)
+  (cond ((use-region-p)
+         (unless isearch-mode (isearch-mode t))
+         (isearch-yank-string (funcall region-extract-function))
+         (deactivate-mark))
+        (t (user-error "No selected region"))))
+
 (defun isearch-yank-pop ()
   "Replace just-yanked search string with previously killed string."
   (interactive)

--8<-----------------------------cut here---------------end--------------->8---


In GNU Emacs 28.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0)
 of 2020-02-08 built on calancha-pc.dy.bbexcite.jp
Repository revision: fe903c5ab7354b97f80ecf1b01ca3ff1027be446
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux 10 (buster)






^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-02-08 18:04 bug#39512: 28.0.50; Add command isearch-yank-region Tino Calancha
@ 2020-02-08 23:47 ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-02-09  0:31 ` Juri Linkov
  1 sibling, 0 replies; 21+ messages in thread
From: Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-02-08 23:47 UTC (permalink / raw)
  To: Tino Calancha; +Cc: npostavs, juri, contovob, eliz, 39512, drew.adams

Hi Tino:

It is good to see that someone else is interested on this. I just forgot
it after not having a good agreement about the bindings to use.

The patch seems good for me, I only want to comment that there was a
race condition when the selected region is not contiguous (ex:
rectangular selection). Did you tried that case?

If possible, please consider adding also some function like the one I
proposed in one of the mails (isearch-forward-region) to start the
search directly with the selected region text.

A good improvement to my function would be to use thing-at-point if not
region is active (or modify isearch-forward-symbol-at-point) to do so.

Best, Ergus

On Sat, Feb 08, 2020 at 07:04:57PM +0100, Tino Calancha wrote:
>Severity: wishlist
>Tags: patch
>X-Debbugs-Cc: spacibba@aol.com,juri@linkov.net,drew.adams@oracle.com,npostavs@gmail.com,contovob@tcd.ie,eliz@gnu.org
>
>I wish to have this command; it naturally completes other
>isearch-yank-... cases.
>
>This topic has been already discussed in the following links:
>
>https://lists.gnu.org/archive/html/emacs-devel/2019-04/msg01125.html
>https://lists.gnu.org/archive/html/emacs-devel/2019-05/msg00003.html
>
>Note that in those threads there were plenty of discussions; here I'd
>like to focus just in this proposed command.  If needed, I'd recommend
>to open further bugs to discuss about other commands.
>
>I let this command to start the interactive search if we are
>not already there; from the above links I realized that
>such a functionality was also missing.  This is consistent with
>`isearch-yank-kill' (I think we should mention that in its docstring).
>
>
>--8<-----------------------------cut here---------------start------------->8---
>commit 1ea1939929fbf22c6d635b075cfcb2b77a4b8243
>Author: Tino Calancha <tino.calancha@gmail.com>
>Date:   Sat Feb 8 18:39:44 2020 +0100
>
>    Add command isearch-yank-region
>
>    During an incremental search, this command appends the region
>    to the search string.  Otherwise, start an incremental search
>    using the region as the search string.
>
>    * lisp/isearch.el (isearch-yank-region): New command; bound to 'M-.'
>    in isearch-mode-map.
>
>    * doc/emacs/search.texi (Isearch Yank): Document it.
>    * etc/NEWS: Announce this change.
>
>diff --git a/doc/emacs/search.texi b/doc/emacs/search.texi
>index 16916617a2..b443300b6d 100644
>--- a/doc/emacs/search.texi
>+++ b/doc/emacs/search.texi
>@@ -285,6 +285,11 @@ Isearch Yank
> a prefix numeric argument of @var{n}, the command appends everything
> from point to the @var{n}th occurrence of the specified character.
>
>+@kindex M-. @r{(Incremental search)}
>+@findex isearch-yank-region
>+  Likewise, @kbd{M-.} (@code{isearch-yank-region}) appends to
>+the search string the selected region.
>+
> @kindex C-y @r{(Incremental search)}
> @kindex M-y @r{(Incremental search)}
> @kindex mouse-2 @r{in the minibuffer (Incremental search)}
>diff --git a/etc/NEWS b/etc/NEWS
>index 55c1a47fbf..cf63176124 100644
>--- a/etc/NEWS
>+++ b/etc/NEWS
>@@ -91,6 +91,13 @@ shows equivalent key bindings for all commands that have them.
> \f
> * Changes in Specialized Modes and Packages in Emacs 28.1
>
>+** Search and Replace
>+
>++++
>+*** New isearch bindings.
>+'M-.' invokes new fnction 'isearch-yank-region', which yanks the selected
>+region into the search string.
>+
> ** Help
>
> +++
>diff --git a/lisp/isearch.el b/lisp/isearch.el
>index ddf9190dc6..266c311c3d 100644
>--- a/lisp/isearch.el
>+++ b/lisp/isearch.el
>@@ -514,6 +514,9 @@ isearch-menu-bar-yank-map
>     (define-key map [isearch-yank-kill]
>       '(menu-item "Current kill" isearch-yank-kill
>                   :help "Append current kill to search string"))
>+    (define-key map [isearch-yank-region]
>+      '(menu-item "Active region" isearch-yank-region
>+                  :help "Append active region to search string"))
>     (define-key map [isearch-yank-until-char]
>       '(menu-item "Until char..." isearch-yank-until-char
>                   :help "Yank from point to specified character into search string"))
>@@ -708,6 +711,7 @@ isearch-mode-map
>     (define-key map "\M-\C-d" 'isearch-del-char)
>     (define-key map "\M-\C-y" 'isearch-yank-char)
>     (define-key map    "\C-y" 'isearch-yank-kill)
>+    (define-key map    "\M-." 'isearch-yank-region)
>     (define-key map "\M-\C-z" 'isearch-yank-until-char)
>     (define-key map "\M-s\C-e" 'isearch-yank-line)
>
>@@ -1007,6 +1011,7 @@ isearch-forward
> Type \\[isearch-yank-line] to yank rest of line onto end of search string\
>  and search for it.
> Type \\[isearch-yank-kill] to yank the last string of killed text.
>+Type \\[isearch-yank-region] to yank the active region.
> Type \\[isearch-yank-pop] to replace string just yanked into search prompt
>  with string killed before it.
> Type \\[isearch-quote-char] to quote control character to search for it.
>@@ -2473,6 +2478,17 @@ isearch-yank-kill
>   (unless isearch-mode (isearch-mode t))
>   (isearch-yank-string (current-kill 0)))
>
>+(defun isearch-yank-region ()
>+  "Pull region into search string.
>+If called out of an incremental search, then start an incremental
>+search with the region as the search string."
>+  (interactive)
>+  (cond ((use-region-p)
>+         (unless isearch-mode (isearch-mode t))
>+         (isearch-yank-string (funcall region-extract-function))
>+         (deactivate-mark))
>+        (t (user-error "No selected region"))))
>+
> (defun isearch-yank-pop ()
>   "Replace just-yanked search string with previously killed string."
>   (interactive)
>
>--8<-----------------------------cut here---------------end--------------->8---
>
>
>In GNU Emacs 28.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0)
> of 2020-02-08 built on calancha-pc.dy.bbexcite.jp
>Repository revision: fe903c5ab7354b97f80ecf1b01ca3ff1027be446
>Repository branch: master
>Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
>System Description: Debian GNU/Linux 10 (buster)
>
>
>





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-02-08 18:04 bug#39512: 28.0.50; Add command isearch-yank-region Tino Calancha
  2020-02-08 23:47 ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-02-09  0:31 ` Juri Linkov
  2020-02-09 11:21   ` Tino Calancha
                     ` (2 more replies)
  1 sibling, 3 replies; 21+ messages in thread
From: Juri Linkov @ 2020-02-09  0:31 UTC (permalink / raw)
  To: Tino Calancha; +Cc: spacibba, npostavs, 39512, contovob

> I wish to have this command; it naturally completes other
> isearch-yank-... cases.
>
> This topic has been already discussed in the following links:
>
> https://lists.gnu.org/archive/html/emacs-devel/2019-04/msg01125.html
> https://lists.gnu.org/archive/html/emacs-devel/2019-05/msg00003.html
>
> Note that in those threads there were plenty of discussions; here I'd
> like to focus just in this proposed command.  If needed, I'd recommend
> to open further bugs to discuss about other commands.

Thanks for creating a new feature request that unlike these discussions
on emacs-devel won't fall into oblivion.

> I let this command to start the interactive search if we are
> not already there; from the above links I realized that
> such a functionality was also missing.  This is consistent with
> `isearch-yank-kill' (I think we should mention that in its docstring).

Please add this useful feature of `isearch-yank-kill' to the documentation.
Maybe it should be bound to a special key on the global `M-s' prefix map.
The most natural key would be `M-s C-y'.

>     Add command isearch-yank-region
>
>     During an incremental search, this command appends the region
>     to the search string.  Otherwise, start an incremental search
>     using the region as the search string.

What use cases do you think it could be used for?

I don't see any useful case for appending the region to the search string.

I see only 2 useful cases that don't append the region to the search string:

1. Before starting isearch, the user selects the region,
   then types a special command bound to a key on the global `M-s' prefix
   that yanks the region to the initially empty search string
   (i.e. it doesn't append, it replaces the empty search string)

I used such command to do this:

(defun isearch-forward-region ()
  "Do incremental search forward for text from the active region.
Like ordinary incremental search except that text from the region
is added to the search string initially if the region is active."
  (interactive)
  (isearch-forward nil 1)
  (cond
   ((use-region-p)
    (when (< (mark) (point))
      (exchange-point-and-mark))
    (isearch-yank-string
     (buffer-substring-no-properties (region-beginning) (region-end)))
    (deactivate-mark))
   (t
    (setq isearch-error "No active region")
    (isearch-push-state)
    (isearch-update))))

(define-key search-map "\M-." 'isearch-forward-region)

2. The second useful case is to activate the region, start isearch,
   use isearch to find the string at the region end, thus moving the region end
   to a new position, replace (don't append) the search string with region text -
   this is what isearch-yank-region could do.
   IOW, sync the region with the search string.

> +(defun isearch-yank-region ()
> +  "Pull region into search string.
> +If called out of an incremental search, then start an incremental
> +search with the region as the search string."
> +  (interactive)
> +  (cond ((use-region-p)
> +         (unless isearch-mode (isearch-mode t))
> +         (isearch-yank-string (funcall region-extract-function))

Here (funcall region-extract-function) signals the error

  (wrong-number-of-arguments (1 . 1) 0)

Have you tested your patch?





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-02-09  0:31 ` Juri Linkov
@ 2020-02-09 11:21   ` Tino Calancha
  2020-02-09 12:38   ` Tino Calancha
  2020-08-09 11:28   ` Lars Ingebrigtsen
  2 siblings, 0 replies; 21+ messages in thread
From: Tino Calancha @ 2020-02-09 11:21 UTC (permalink / raw)
  To: Juri Linkov; +Cc: spacibba, npostavs, 39512, contovob

Juri Linkov <juri@linkov.net> writes:

>>     During an incremental search, this command appends the region
>>     to the search string.  Otherwise, start an incremental search
>>     using the region as the search string.
>
> What use cases do you think it could be used for?
> I don't see any useful case for appending the region to the search string.
Me either.  I believe I have copied such an append wording from
elsewhere.  What I have in mind is just to set the search string = to
the region.


> I see only 2 useful cases that don't append the region to the search string:
>
> 1. Before starting isearch, the user selects the region,
>    then types a special command bound to a key on the global `M-s' prefix
>    that yanks the region to the initially empty search string
>    (i.e. it doesn't append, it replaces the empty search string)

> 2. The second useful case is to activate the region, start isearch,
>    use isearch to find the string at the region end, thus moving the region end
>    to a new position, replace (don't append) the search string with region text -
>    this is what isearch-yank-region could do.
>    IOW, sync the region with the search string.

I was motivated with a scenario similar to 1.
1. I select a region that I want to search for
2. C-s ; I start an interactive search
3. `some keybinding here'; that sets the search string = to the region.

I think on 3. as an alternative of `isearch-yank-until-char' (and
family).  Sure, having the command to start the search directly with a
M-s prefix is desirable too.
My mind finds easy to remember that I always can do the patter:
C-s
;; Now some keybinding that sets the search string with `isearch-yank- (whatever)

>> +(defun isearch-yank-region ()
>> +  "Pull region into search string.
>> +If called out of an incremental search, then start an incremental
>> +search with the region as the search string."
>> +  (interactive)
>> +  (cond ((use-region-p)
>> +         (unless isearch-mode (isearch-mode t))
>> +         (isearch-yank-string (funcall region-extract-function))
>
> Here (funcall region-extract-function) signals the error
>
>   (wrong-number-of-arguments (1 . 1) 0)
>
> Have you tested your patch?

Opps, I tested without emacs -Q. sorry for that!  I have some advice
in that function that lets me call it with no arguments.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-02-09  0:31 ` Juri Linkov
  2020-02-09 11:21   ` Tino Calancha
@ 2020-02-09 12:38   ` Tino Calancha
  2020-02-10  0:45     ` Juri Linkov
  2020-08-09 11:28   ` Lars Ingebrigtsen
  2 siblings, 1 reply; 21+ messages in thread
From: Tino Calancha @ 2020-02-09 12:38 UTC (permalink / raw)
  To: Juri Linkov; +Cc: spacibba, npostavs, 39512, contovob

Juri Linkov <juri@linkov.net> writes:


>> such a functionality was also missing.  This is consistent with
>> `isearch-yank-kill' (I think we should mention that in its docstring).
>
> Please add this useful feature of `isearch-yank-kill' to the documentation.
> Maybe it should be bound to a special key on the global `M-s' prefix map.
> The most natural key would be `M-s C-y'.
Done.  Added global binding `M-s C-y' for `isearch-yank-kill'.
Updated its docstring and the NEWS.


> (defun isearch-forward-region ()
>   "Do incremental search forward for text from the active region.
> Like ordinary incremental search except that text from the region
> is added to the search string initially if the region is active."
>   (interactive)
>   (isearch-forward nil 1)
>   (cond
>    ((use-region-p)
>     (when (< (mark) (point))
>       (exchange-point-and-mark))
>     (isearch-yank-string
>      (buffer-substring-no-properties (region-beginning) (region-end)))
>     (deactivate-mark))
>    (t
>     (setq isearch-error "No active region")
>     (isearch-push-state)
>     (isearch-update))))
I got inspired by your function; I took you default case in the
`cond' to not exit the interactive search, as I was doing.

I have added global keybinding `M-s M-.' for my `isearch-yank-region'.
This naturally open the following question.

`isearch-yank-kill' and `isearch-yank-region' are good names for the
use case of calling them once we are in an interactive search.

The names are not any good for the use case of calling them directly
(from a global keybinding).  We might want:

1. aliases `isearch-forward-kill', `isearch-forward-region'

2. restrict the use of them for just inside the interactive search, and
   define the new commands (`isearch-forward-kill',`isearch-forward-region')
   as those calling `isearch-mode' at the beginning.

[might case a regression of Bug#21419.]

--8<-----------------------------cut here---------------start------------->8---
commit 9cfe28b2b9661ca5cb11b0a99649faad5d6cf708
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Sun Feb 9 13:19:51 2020 +0100

    Add command isearch-yank-region
    
    Right after start an interactive search, set the search string
    equal to the active region; this is analogous to other
    'isearch-yank-...' commands.
    
    Calling the command out of an interactive search, then it starts
    an interactive search with the region as the search string.  This
    is consistent with `isearch-yank-kill'.
    
    * lisp/isearch.el (isearch-yank-region): New command; bound to 'M-.'
    in isearch-mode-map.  Bind it globally to 'M-s M-.'.
    (isearch-yank-kill): Update dosctring.  Bind this command globally
    to 'M-s C-y'.
    
    * doc/emacs/search.texi (Isearch Yank): Document these changes.
    * etc/NEWS: Announce these changes.

diff --git a/doc/emacs/search.texi b/doc/emacs/search.texi
index 16916617a2..07a40c18cf 100644
--- a/doc/emacs/search.texi
+++ b/doc/emacs/search.texi
@@ -250,6 +250,16 @@ Isearch Yank
 search string.  The commands described in this subsection let you do
 that conveniently.
 
+@kindex M-. @r{(Incremental search)}
+@kindex M-s M-.
+@findex isearch-yank-region
+  @kbd{M-.} (@code{isearch-yank-region}) sets the search string equal
+to the active region.  This is useful when you have selected a string
+that you now want to search for.  Then you can start the interactive
+search and use @kbd{M-.}.  Alternatively, you can use @kbd{M-s M-.}
+to start directly an interactive search with the region as the
+search string.
+
 @kindex C-w @r{(Incremental search)}
 @findex isearch-yank-word-or-char
   @kbd{C-w} (@code{isearch-yank-word-or-char}) appends the next
@@ -287,6 +297,7 @@ Isearch Yank
 
 @kindex C-y @r{(Incremental search)}
 @kindex M-y @r{(Incremental search)}
+@kindex M-s C-y
 @kindex mouse-2 @r{in the minibuffer (Incremental search)}
 @findex isearch-yank-kill
 @findex isearch-yank-pop
diff --git a/etc/NEWS b/etc/NEWS
index 55c1a47fbf..ecb8048ade 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -91,6 +91,14 @@ shows equivalent key bindings for all commands that have them.
 \f
 * Changes in Specialized Modes and Packages in Emacs 28.1
 
+** Search and Replace
+
++++
+*** New isearch bindings.
+'M-.' invokes new command 'isearch-yank-region', which yanks the selected
+region into the search string.  It's globally bound to 'M-s M-.'.
+'isearch-yank-kill' now is globally bound to 'M-s C-y'.
+
 ** Help
 
 +++
diff --git a/lisp/isearch.el b/lisp/isearch.el
index ddf9190dc6..c5ae96033d 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -514,6 +514,9 @@ isearch-menu-bar-yank-map
     (define-key map [isearch-yank-kill]
       '(menu-item "Current kill" isearch-yank-kill
                   :help "Append current kill to search string"))
+    (define-key map [isearch-yank-region]
+      '(menu-item "Active region" isearch-yank-region
+                  :help "Append active region to search string"))
     (define-key map [isearch-yank-until-char]
       '(menu-item "Until char..." isearch-yank-until-char
                   :help "Yank from point to specified character into search string"))
@@ -708,6 +711,7 @@ isearch-mode-map
     (define-key map "\M-\C-d" 'isearch-del-char)
     (define-key map "\M-\C-y" 'isearch-yank-char)
     (define-key map    "\C-y" 'isearch-yank-kill)
+    (define-key map    "\M-." 'isearch-yank-region)
     (define-key map "\M-\C-z" 'isearch-yank-until-char)
     (define-key map "\M-s\C-e" 'isearch-yank-line)
 
@@ -973,6 +977,8 @@ isearch--saved-overriding-local-map
 (defvar-local isearch-mode nil) ;; Name of the minor mode, if non-nil.
 
 (define-key global-map "\C-s" 'isearch-forward)
+(define-key global-map "\M-s\M-." 'isearch-yank-region)
+(define-key global-map "\M-s\C-y" 'isearch-yank-kill)
 (define-key esc-map "\C-s" 'isearch-forward-regexp)
 (define-key global-map "\C-r" 'isearch-backward)
 (define-key esc-map "\C-r" 'isearch-backward-regexp)
@@ -1007,6 +1013,7 @@ isearch-forward
 Type \\[isearch-yank-line] to yank rest of line onto end of search string\
  and search for it.
 Type \\[isearch-yank-kill] to yank the last string of killed text.
+Type \\[isearch-yank-region] to yank the active region.
 Type \\[isearch-yank-pop] to replace string just yanked into search prompt
  with string killed before it.
 Type \\[isearch-quote-char] to quote control character to search for it.
@@ -2468,11 +2475,28 @@ isearch-yank-string
    string (mapconcat 'isearch-text-char-description string "")))
 
 (defun isearch-yank-kill ()
-  "Pull string from kill ring into search string."
+  "Pull string from kill ring into search string.
+If called out of an incremental search, then start an incremental
+search with the last string of killed text as the search string."
   (interactive)
   (unless isearch-mode (isearch-mode t))
   (isearch-yank-string (current-kill 0)))
 
+(defun isearch-yank-region ()
+  "Pull region into search string.
+If called out of an incremental search, then start an incremental
+search with the region as the search string."
+  (interactive)
+  (unless isearch-mode (isearch-mode t))
+  (cond ((use-region-p)
+         (isearch-yank-string (funcall region-extract-function nil))
+         (deactivate-mark))
+        (t
+         (setq isearch-error "No active region")
+         (isearch-push-state)
+         (isearch-update))))
+
+
 (defun isearch-yank-pop ()
   "Replace just-yanked search string with previously killed string."
   (interactive)

--8<-----------------------------cut here---------------end--------------->8---






^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-02-09 12:38   ` Tino Calancha
@ 2020-02-10  0:45     ` Juri Linkov
  2020-02-12 22:10       ` Juri Linkov
  0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2020-02-10  0:45 UTC (permalink / raw)
  To: Tino Calancha; +Cc: spacibba, npostavs, 39512, contovob

> I have added global keybinding `M-s M-.' for my `isearch-yank-region'.

Thanks.

> `isearch-yank-kill' and `isearch-yank-region' are good names for the
> use case of calling them once we are in an interactive search.
>
> The names are not any good for the use case of calling them directly
> (from a global keybinding).  We might want:
>
> 1. aliases `isearch-forward-kill', `isearch-forward-region'

Aliases might add more confusion and make impression that
these are separate commands when displayed in a list of isearch commands,
e.g. in completion list, in documentation.

> 2. restrict the use of them for just inside the interactive search, and
>    define the new commands (`isearch-forward-kill',`isearch-forward-region')
>    as those calling `isearch-mode' at the beginning.

Restricting would be worse.

So maybe `isearch-yank-kill' and `isearch-yank-region' still are
not too bad names to use as entry points to enable isearch mode.

> +    (define-key map    "\M-." 'isearch-yank-region)

Some doubts are about `M-.' - what if some users might want to use `M-.'
to exit isearch and run its global binding `xref-find-definitions'
on the found symbol?  Maybe better to bind `isearch-yank-region'
to `M-s M-.' in isearch-mode too?

> I have some advice in that function that lets me call it with no arguments.

Maybe a new function could be added as a wrapper around
`(funcall region-extract-function)' and with its argument optional.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-02-10  0:45     ` Juri Linkov
@ 2020-02-12 22:10       ` Juri Linkov
  0 siblings, 0 replies; 21+ messages in thread
From: Juri Linkov @ 2020-02-12 22:10 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 39512

>> I have some advice in that function that lets me call it with no arguments.
>
> Maybe a new function could be added as a wrapper around
> `(funcall region-extract-function)' and with its argument optional.

What do you think about a new function like we added `region-bounds' in
https://lists.gnu.org/archive/html/emacs-devel/2017-01/msg00402.html

(defun region (&optional arg)
  "Return the region content as a string.
For non-contiguous regions return a list of strings."
  (funcall region-extract-function arg))





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-02-09  0:31 ` Juri Linkov
  2020-02-09 11:21   ` Tino Calancha
  2020-02-09 12:38   ` Tino Calancha
@ 2020-08-09 11:28   ` Lars Ingebrigtsen
  2020-08-09 23:23     ` Juri Linkov
  2 siblings, 1 reply; 21+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-09 11:28 UTC (permalink / raw)
  To: Juri Linkov; +Cc: spacibba, Tino Calancha, npostavs, 39512, contovob

Juri Linkov <juri@linkov.net> writes:

>> https://lists.gnu.org/archive/html/emacs-devel/2019-04/msg01125.html
>> https://lists.gnu.org/archive/html/emacs-devel/2019-05/msg00003.html
>>
>> Note that in those threads there were plenty of discussions; here I'd
>> like to focus just in this proposed command.  If needed, I'd recommend
>> to open further bugs to discuss about other commands.
>
> Thanks for creating a new feature request that unlike these discussions
> on emacs-devel won't fall into oblivion.

This was in February, though, and the patch still hasn't been applied.  :-/

I think this addition makes sense...  was there any particular reason
it's not applied?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-08-09 11:28   ` Lars Ingebrigtsen
@ 2020-08-09 23:23     ` Juri Linkov
  2020-08-10  1:19       ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-08-10 17:00       ` Tino Calancha
  0 siblings, 2 replies; 21+ messages in thread
From: Juri Linkov @ 2020-08-09 23:23 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: spacibba, Tino Calancha, npostavs, 39512, contovob

>> Thanks for creating a new feature request that unlike these discussions
>> on emacs-devel won't fall into oblivion.
>
> This was in February, though, and the patch still hasn't been applied.  :-/
>
> I think this addition makes sense...  was there any particular reason
> it's not applied?

I really don't see a need for adding isearch-yank-region.  I think that
isearch-forward-region proposed by Ergus should be sufficient because
typing 'M-s M-.' (bound globally to isearch-forward-region) even when
isearch mode is active, will exit Isearch and restart Isearch with the
contents of the still active region added to the search string.
This will cover all cases requested here with just one new command:

  (defun isearch-forward-region ()
    "Do incremental search forward for text from the active region.
  Like ordinary incremental search except that text from the region
  is added to the search string initially if the region is active."
    (interactive)
    (isearch-forward nil 1)
    (cond
     ((use-region-p)
      (when (< (mark) (point))
        (exchange-point-and-mark))
      (isearch-yank-string
       (buffer-substring-no-properties (region-beginning) (region-end)))
      (deactivate-mark))
     (t
      (setq isearch-error "No active region")
      (isearch-push-state)
      (isearch-update))))

  (define-key search-map "\M-." 'isearch-forward-region)





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-08-09 23:23     ` Juri Linkov
@ 2020-08-10  1:19       ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-08-10 23:49         ` Juri Linkov
  2020-08-10 17:00       ` Tino Calancha
  1 sibling, 1 reply; 21+ messages in thread
From: Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-08-10  1:19 UTC (permalink / raw)
  To: Juri Linkov
  Cc: Tino Calancha, npostavs, 39512, contovob, Lars Ingebrigtsen, eliz,
	drew.adams

On Mon, Aug 10, 2020 at 02:23:59AM +0300, Juri Linkov wrote:
>>> Thanks for creating a new feature request that unlike these discussions
>>> on emacs-devel won't fall into oblivion.
>>
>> This was in February, though, and the patch still hasn't been applied.  :-/
>>
>> I think this addition makes sense...  was there any particular reason
>> it's not applied?
>
>I really don't see a need for adding isearch-yank-region.  I think that
>isearch-forward-region proposed by Ergus should be sufficient because
>typing 'M-s M-.' (bound globally to isearch-forward-region) even when
>isearch mode is active, will exit Isearch and restart Isearch with the
>contents of the still active region added to the search string.
>This will cover all cases requested here with just one new command:
>
>  (defun isearch-forward-region ()
>    "Do incremental search forward for text from the active region.
>  Like ordinary incremental search except that text from the region
>  is added to the search string initially if the region is active."
>    (interactive)
>    (isearch-forward nil 1)
>    (cond
>     ((use-region-p)
>      (when (< (mark) (point))
>        (exchange-point-and-mark))
>      (isearch-yank-string
>       (buffer-substring-no-properties (region-beginning) (region-end)))
>      (deactivate-mark))
>     (t
>      (setq isearch-error "No active region")
>      (isearch-push-state)
>      (isearch-update))))
>
>  (define-key search-map "\M-." 'isearch-forward-region)

Hi Juri:

I didn't add this command to vanilla at the end because there were many
arguments about the bindings to use. And I consider that without a
default binding it is pretty much useless for the general user.

Now I am using swiper which has an improved version for thing-at-point
(something like `thing-at-point-or-region` called `ivy-thing-at-point`)
which actually is much more useful and avoids an extra binding.

https://github.com/abo-abo/swiper/blob/c6b60d34ac37bf4d91a25f16d22e528f85e06938/ivy.el#L426

Implementing something like that in vanilla is (in my opinion) the best
default behaviors for isearch-forward-symbol-at-point. But I don't want
to go in that discussion in the mailing list because it is difficult to
get an agreement what touching old commands. But you are free to do it
if you want.

Something more or less like this should work:

(defcustom isearch-thing-at-point-use-region nil
   "isearch-forward-symbol-at-point use region when active."
   :type 'boolean)

(defun bounds-thing-at-pt-or-region ()
   "Return current 'thing-at-point' or region bounds"
   (and (not (nth 3 (syntax-ppss)))  ;; global skip
        (cond
	((and isearch-thing-at-point-use-region
	      (region-active-p))
	 (let* ((beg (region-beginning))
		(end (region-end))
		(eol (save-excursion (goto-char beg) (line-end-position))))
	   (and (< beg end)      ;; no empty region
		(<= end eol)     ;; no multiline region
		(cons beg end))))
	((find-tag-default-bounds)))))

(defun isearch-forward-symbol-at-point (&optional arg)
   "Do incremental search forward for a symbol found near point.
Like ordinary incremental search except that the symbol found at point
is added to the search string initially as a regexp surrounded
by symbol boundary constructs \\_< and \\_>.
See the command `isearch-forward-symbol' for more information.
With a prefix argument, search for ARGth symbol forward if ARG is
positive, or search for ARGth symbol backward if ARG is negative."
   (interactive "P")
   (isearch-forward-symbol nil 1)
   (let ((bounds (bounds-thing-at-pt-or-region))
         (count (and arg (prefix-numeric-value arg))))
     (cond
      (bounds
       (when (< (car bounds) (point))
	(goto-char (car bounds)))
       (isearch-yank-string
        (buffer-substring-no-properties (car bounds) (cdr bounds)))
       (when count
         (isearch-repeat-forward count)))
      (t
       (if isearch-thing-at-point-use-region
           (setq isearch-error "No symbol at point or active region.")
         (setq isearch-error "No symbol at point"))
       (isearch-push-state)
       (isearch-update)))))

Best,
Ergus





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-08-09 23:23     ` Juri Linkov
  2020-08-10  1:19       ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-08-10 17:00       ` Tino Calancha
  1 sibling, 0 replies; 21+ messages in thread
From: Tino Calancha @ 2020-08-10 17:00 UTC (permalink / raw)
  To: Juri Linkov
  Cc: spacibba, Tino Calancha, npostavs, 39512, contovob,
	Lars Ingebrigtsen



On Mon, 10 Aug 2020, Juri Linkov wrote:

>> I think this addition makes sense...  was there any particular reason
>> it's not applied?
>
> I really don't see a need for adding isearch-yank-region.  I think that
> isearch-forward-region proposed by Ergus should be sufficient because
> typing 'M-s M-.' (bound globally to isearch-forward-region) even when
> isearch mode is active, will exit Isearch and restart Isearch with the
> contents of the still active region added to the search string.
> This will cover all cases requested here with just one new command:

I am OK with this; it is a good addition.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-08-10  1:19       ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-08-10 23:49         ` Juri Linkov
  2020-08-11  1:12           ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2020-08-10 23:49 UTC (permalink / raw)
  To: Ergus; +Cc: Tino Calancha, npostavs, 39512, contovob, Lars Ingebrigtsen

> Now I am using swiper which has an improved version for thing-at-point
> (something like `thing-at-point-or-region` called `ivy-thing-at-point`)
> which actually is much more useful and avoids an extra binding.
>
> https://github.com/abo-abo/swiper/blob/c6b60d34ac37bf4d91a25f16d22e528f85e06938/ivy.el#L426
>
> Implementing something like that in vanilla is (in my opinion) the best
> default behaviors for isearch-forward-symbol-at-point. But I don't want
> to go in that discussion in the mailing list because it is difficult to
> get an agreement what touching old commands. But you are free to do it
> if you want.

Indeed there are two ways to add it: as a new command (that requires a new
keybinding) or a new option (disabled by default) for the existing command.
Maybe we could enable it by default, but I don't know how it will affect
the use cases of users - maybe someone users use isearch to extend the
active region to the next matching symbol, and don't want 'M-s .' to yank
the region.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-08-10 23:49         ` Juri Linkov
@ 2020-08-11  1:12           ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-08-11 23:13             ` Juri Linkov
  0 siblings, 1 reply; 21+ messages in thread
From: Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-08-11  1:12 UTC (permalink / raw)
  To: Juri Linkov
  Cc: Tino Calancha, npostavs, 39512, contovob, Lars Ingebrigtsen, eliz,
	drew.adams

On Tue, Aug 11, 2020 at 02:49:44AM +0300, Juri Linkov wrote:
>> Now I am using swiper which has an improved version for thing-at-point
>> (something like `thing-at-point-or-region` called `ivy-thing-at-point`)
>> which actually is much more useful and avoids an extra binding.
>>
>> https://github.com/abo-abo/swiper/blob/c6b60d34ac37bf4d91a25f16d22e528f85e06938/ivy.el#L426
>>
>> Implementing something like that in vanilla is (in my opinion) the best
>> default behaviors for isearch-forward-symbol-at-point. But I don't want
>> to go in that discussion in the mailing list because it is difficult to
>> get an agreement what touching old commands. But you are free to do it
>> if you want.
>
>Indeed there are two ways to add it: as a new command (that requires a new
>keybinding) or a new option (disabled by default) for the existing command.
>Maybe we could enable it by default, but I don't know how it will affect
>the use cases of users - maybe someone users use isearch to extend the
>active region to the next matching symbol, and don't want 'M-s .' to yank
>the region.

If the region is active but empty that functionality could make sense
(extend to the next thing at point), but when it is not empty I think it
doesn't make too much sense in general use... but let's wait for the
rest of the opinions.

Actually, probably such a function (thing-at-point-or-region) could be
added to thing-at-pt to use it in other functionalities too. (like
highlight thing at point, idle highlight thing at point or region, kill
thing at point or region and so on)

It is something I have seen re-implemented in many packages here and
there again and again, so probably it is time to provide it in vanilla.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-08-11  1:12           ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-08-11 23:13             ` Juri Linkov
  2020-08-12 17:41               ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2020-08-11 23:13 UTC (permalink / raw)
  To: Ergus; +Cc: Tino Calancha, npostavs, 39512, contovob, Lars Ingebrigtsen

> If the region is active but empty that functionality could make sense
> (extend to the next thing at point), but when it is not empty I think it
> doesn't make too much sense in general use... but let's wait for the
> rest of the opinions.

Good point.  ‘C-SPC M-s . C-s C-s ...’ means that the user wants to
extend the region to one of the next found symbols.

But still there is an unsolved problem: ‘M-s .’ activates the *symbol*
search mode, not the default non-symbol search mode.  This means that
the yanked region will match only symbols, that is useless when the
region is longer than one symbol (the symbol search is activated by
the call of ‘isearch-forward-symbol’ in ‘isearch-forward-symbol-at-point’).

This means there is still a need to add a new command that yanks the region
but doesn't activate the symbol search mode.

> Actually, probably such a function (thing-at-point-or-region) could be
> added to thing-at-pt to use it in other functionalities too. (like
> highlight thing at point, idle highlight thing at point or region, kill
> thing at point or region and so on)
>
> It is something I have seen re-implemented in many packages here and
> there again and again, so probably it is time to provide it in vanilla.

Yes, this could be added to thing-at-pt.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-08-11 23:13             ` Juri Linkov
@ 2020-08-12 17:41               ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-08-12 23:44                 ` Juri Linkov
  0 siblings, 1 reply; 21+ messages in thread
From: Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-08-12 17:41 UTC (permalink / raw)
  To: Juri Linkov
  Cc: Tino Calancha, npostavs, 39512, contovob, Lars Ingebrigtsen, eliz,
	drew.adams

On Wed, Aug 12, 2020 at 02:13:11AM +0300, Juri Linkov wrote:
>> If the region is active but empty that functionality could make sense
>> (extend to the next thing at point), but when it is not empty I think it
>> doesn't make too much sense in general use... but let's wait for the
>> rest of the opinions.
>
>Good point.  ‘C-SPC M-s . C-s C-s ...’ means that the user wants to
>extend the region to one of the next found symbols.
>
>But still there is an unsolved problem: ‘M-s .’ activates the *symbol*
>search mode, not the default non-symbol search mode.  This means that
>the yanked region will match only symbols, that is useless when the
>region is longer than one symbol (the symbol search is activated by
>the call of ‘isearch-forward-symbol’ in ‘isearch-forward-symbol-at-point’).
>
>This means there is still a need to add a new command that yanks the region
>but doesn't activate the symbol search mode.
>
You are right; but...

Does it really makes sense to call isearch-forward-symbol when the
region is active and not empty if it works in the way we are describing?

I mean; we could move the call to isearch-forward-symbol inside the cond
and use just isearch-forward or isearch-forward-regexp directly when the
bounds are from the active region.

  Because for isearch-forward-symbol there is already `M-s _`

>> Actually, probably such a function (thing-at-point-or-region) could be
>> added to thing-at-pt to use it in other functionalities too. (like
>> highlight thing at point, idle highlight thing at point or region, kill
>> thing at point or region and so on)
>>
>> It is something I have seen re-implemented in many packages here and
>> there again and again, so probably it is time to provide it in vanilla.
>
>Yes, this could be added to thing-at-pt.

If so, maybe it will be needed to return somehow the information about
the precedence of the bounds. Anything that can be used in the caller to
know if the bounds are from thing-at-pt or region or if they are a
symbol, a word or a region.

In C we could pass a variable by reference; but in elisp I don't know
whats the convectional way.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-08-12 17:41               ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-08-12 23:44                 ` Juri Linkov
  2020-08-13  3:14                   ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2020-08-12 23:44 UTC (permalink / raw)
  To: Ergus; +Cc: Tino Calancha, npostavs, 39512, contovob, Lars Ingebrigtsen

> Does it really makes sense to call isearch-forward-symbol when the
> region is active and not empty if it works in the way we are describing?
>
> I mean; we could move the call to isearch-forward-symbol inside the cond
> and use just isearch-forward or isearch-forward-regexp directly when the
> bounds are from the active region.

The problem is that the name of the command bound to 'M-s .' is
isearch-forward-symbol-at-point that implies that it has to call
isearch-forward-symbol.  So not using symbol search on the active region
will make a mess from this command.

A cleaner solution would be to add a new non-symbol command with a name like
isearch-forward-thing-at-point-or-region bound to 'M-s M-.' and based on
thing-at-point-or-region.

>>> Actually, probably such a function (thing-at-point-or-region) could be
>>> added to thing-at-pt to use it in other functionalities too. (like
>>> highlight thing at point, idle highlight thing at point or region, kill
>>> thing at point or region and so on)
>>>
>>> It is something I have seen re-implemented in many packages here and
>>> there again and again, so probably it is time to provide it in vanilla.
>>
>>Yes, this could be added to thing-at-pt.
>
> If so, maybe it will be needed to return somehow the information about
> the precedence of the bounds. Anything that can be used in the caller to
> know if the bounds are from thing-at-pt or region or if they are a
> symbol, a word or a region.

In the previous message you sent a link to `ivy-thing-at-point` that
also uses (thing-at-point 'url) and also tries to get a filename at point.
Do you think `thing-at-point-or-region` should do the same?





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-08-12 23:44                 ` Juri Linkov
@ 2020-08-13  3:14                   ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-04-15 20:52                     ` Juri Linkov
  0 siblings, 1 reply; 21+ messages in thread
From: Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-08-13  3:14 UTC (permalink / raw)
  To: Juri Linkov
  Cc: Tino Calancha, npostavs, 39512, contovob, Lars Ingebrigtsen, eliz,
	drew.adams

On Thu, Aug 13, 2020 at 02:44:33AM +0300, Juri Linkov wrote:
>
>The problem is that the name of the command bound to 'M-s .' is
>isearch-forward-symbol-at-point that implies that it has to call
>isearch-forward-symbol.  So not using symbol search on the active region
>will make a mess from this command.
>
>A cleaner solution would be to add a new non-symbol command with a name like
>isearch-forward-thing-at-point-or-region bound to 'M-s M-.' and based on
>thing-at-point-or-region.
>
I would prefer using `M-s .` for consistency (even if it requires
changing the command name). In swiper it works like that and it is very
comfortable and consistent with the "expected" experience; but I
understand that someone will complain for sure... (as usual)

So without any other alternative, `M-s M-.` will be good enough. In
general it would be better (for consistency) if we "reserve" `M-.`
"suffixes" for future thing-at-point-or-region commands right?


>In the previous message you sent a link to `ivy-thing-at-point` that
>also uses (thing-at-point 'url) and also tries to get a filename at point.
>Do you think `thing-at-point-or-region` should do the same?

With the interactive experience in mind I think this could make sense as
it does in `ivy-thing-at-point`.

In my opinion from the api point of view, the important modification
could be the method to know the "kind the thing" detected after calling
the function (region, word, symbol, url) but also a function to get the
bounds instead of the text (like bounds-of-thing-at-point-or-region).





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2020-08-13  3:14                   ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-04-15 20:52                     ` Juri Linkov
  2021-04-18 15:34                       ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2021-04-15 20:52 UTC (permalink / raw)
  To: Ergus; +Cc: Tino Calancha, npostavs, 39512, contovob, Lars Ingebrigtsen

> So without any other alternative, `M-s M-.` will be good enough. In
> general it would be better (for consistency) if we "reserve" `M-.`
> "suffixes" for future thing-at-point-or-region commands right?
>
>> In the previous message you sent a link to `ivy-thing-at-point` that
>> also uses (thing-at-point 'url) and also tries to get a filename at point.
>> Do you think `thing-at-point-or-region` should do the same?
>
> With the interactive experience in mind I think this could make sense as
> it does in `ivy-thing-at-point`.
>
> In my opinion from the api point of view, the important modification
> could be the method to know the "kind the thing" detected after calling
> the function (region, word, symbol, url) but also a function to get the
> bounds instead of the text (like bounds-of-thing-at-point-or-region).

I continued trying to do what you suggested some time ago
and immediately stumbled upon a question what "thing"
to use by default as an argument of '(thing-at-point thing)'?

Trying these priorities:

(or (thing-at-point 'region)
    (thing-at-point 'url)
    ;; (thing-at-point 'filename)
    ;; (thing-at-point 'list)
    (thing-at-point 'symbol))

has several problems:

1. There is no such "thing" as 'region'.  Maybe could be added to thingatpt.el?

2. 'url' returns nil when there is no url at point, good.  But
   'filename' returns non-nil on any string, not only on real filenames.

3. It would be nice to use (thing-at-point 'list) only when point
   is on the open/close parens.  This is how double-clicking by mouse
   selects the thing at point of mouse click.  When clicked on a paren,
   the whole list is selected by 'mouse-start-end'.
   'isearch-forward-thing-at-point' could be the same logic.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2021-04-15 20:52                     ` Juri Linkov
@ 2021-04-18 15:34                       ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-04-20 20:29                         ` Juri Linkov
  0 siblings, 1 reply; 21+ messages in thread
From: Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-04-18 15:34 UTC (permalink / raw)
  To: Juri Linkov
  Cc: Tino Calancha, npostavs, 39512, contovob, Lars Ingebrigtsen, eliz,
	drew.adams

On Thu, Apr 15, 2021 at 11:52:03PM +0300, Juri Linkov wrote:
>> So without any other alternative, `M-s M-.` will be good enough. In
>> general it would be better (for consistency) if we "reserve" `M-.`
>> "suffixes" for future thing-at-point-or-region commands right?
>>
>>> In the previous message you sent a link to `ivy-thing-at-point` that
>>> also uses (thing-at-point 'url) and also tries to get a filename at point.
>>> Do you think `thing-at-point-or-region` should do the same?
>>
>> With the interactive experience in mind I think this could make sense as
>> it does in `ivy-thing-at-point`.
>>
>> In my opinion from the api point of view, the important modification
>> could be the method to know the "kind the thing" detected after calling
>> the function (region, word, symbol, url) but also a function to get the
>> bounds instead of the text (like bounds-of-thing-at-point-or-region).
>
>I continued trying to do what you suggested some time ago
>and immediately stumbled upon a question what "thing"
>to use by default as an argument of '(thing-at-point thing)'?
>
>Trying these priorities:
>
>(or (thing-at-point 'region)
>    (thing-at-point 'url)
>    ;; (thing-at-point 'filename)
>    ;; (thing-at-point 'list)
>    (thing-at-point 'symbol))
>
>has several problems:
>
>1. There is no such "thing" as 'region'.  Maybe could be added to thingatpt.el?
>
This was actually the most important part in the request. Maybe an extra
optional parameter like use-region could be added to
bounds-of-thing-at-point that uses the active region when
region-active-p or use-region-p.

As I already mentioned; IMO the idea is to follow the same logic than here:

https://github.com/abo-abo/swiper/blob/471d644d6bdd7d5dc6ca4efb405e6a6389dff245/ivy.el#L427

where `(thing-at-point 'region)` is basically the first branch in the
cond.

>2. 'url' returns nil when there is no url at point, good.  But
>   'filename' returns non-nil on any string, not only on real filenames.
>
'filename' must match a local filename or a path... but it may be costly
to check in the filesystem if the match is an existing file... specially
when using tramp... so I don't have a solution for this. But the linked
code uses some ffap api for that.

>3. It would be nice to use (thing-at-point 'list) only when point
>   is on the open/close parens.  This is how double-clicking by mouse
>   selects the thing at point of mouse click.  When clicked on a paren,
>   the whole list is selected by 'mouse-start-end'.
>   'isearch-forward-thing-at-point' could be the same logic.

now I use C-M-SPC for this selection and then M-w to copy and C-s
C-y. If you add the command, then it will save the M-w and the C-y to
copy the region, because region will be already active... But will
require the M-s prefix any way... so not 2 but at least 1 bind will be
saved. 

You can consider this option in the future if you want... 



  





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2021-04-18 15:34                       ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-04-20 20:29                         ` Juri Linkov
  2021-04-21 20:41                           ` Juri Linkov
  0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2021-04-20 20:29 UTC (permalink / raw)
  To: Ergus; +Cc: Tino Calancha, npostavs, 39512, contovob, Lars Ingebrigtsen

[-- Attachment #1: Type: text/plain, Size: 664 bytes --]

>>(or (thing-at-point 'region)
>>    (thing-at-point 'url)
>>    ;; (thing-at-point 'filename)
>>    ;; (thing-at-point 'list)
>>    (thing-at-point 'symbol))
>>
>>has several problems:
>>
>>1. There is no such "thing" as 'region'.  Maybe could be added to thingatpt.el?
>>
> This was actually the most important part in the request. Maybe an extra
> optional parameter like use-region could be added to
> bounds-of-thing-at-point that uses the active region when
> region-active-p or use-region-p.

No need to add new arg, because the existing arg 'THING' can be used
to accept another value 'region'.

What I meant is just to add to thingatpt.el these 4 lines:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: bounds-of-thing-at-point-region.patch --]
[-- Type: text/x-diff, Size: 461 bytes --]

diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
index c52fcfcc05..b9f7c116e3 100644
--- a/lisp/thingatpt.el
+++ b/lisp/thingatpt.el
@@ -604,6 +604,10 @@ thing-at-point-email-regexp
 
 (put 'buffer 'end-op (lambda () (goto-char (point-max))))
 (put 'buffer 'beginning-op (lambda () (goto-char (point-min))))
+(put 'region 'bounds-of-thing-at-point
+     (lambda ()
+       (when (use-region-p)
+         (cons (region-beginning) (region-end)))))
 
 ;; UUID
 

[-- Attachment #3: Type: text/plain, Size: 1227 bytes --]


> As I already mentioned; IMO the idea is to follow the same logic than here:
>
> https://github.com/abo-abo/swiper/blob/471d644d6bdd7d5dc6ca4efb405e6a6389dff245/ivy.el#L427
>
> where `(thing-at-point 'region)` is basically the first branch in the
> cond.

`(thing-at-point 'region)` will work with the patch above.
Then we could add a customizable user option to specify
in what order the user wants to try things at point, e.g.

  `(region url file symbol)`

>>3. It would be nice to use (thing-at-point 'list) only when point
>>   is on the open/close parens.  This is how double-clicking by mouse
>>   selects the thing at point of mouse click.  When clicked on a paren,
>>   the whole list is selected by 'mouse-start-end'.
>>   'isearch-forward-thing-at-point' could be the same logic.
>
> now I use C-M-SPC for this selection and then M-w to copy and C-s
> C-y. If you add the command, then it will save the M-w and the C-y to
> copy the region, because region will be already active... But will
> require the M-s prefix any way... so not 2 but at least 1 bind will be
> saved. You can consider this option in the future if you want...

Yes, this is a nice way to yank the next list to the search with just
C-M-SPC M-.

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#39512: 28.0.50; Add command isearch-yank-region
  2021-04-20 20:29                         ` Juri Linkov
@ 2021-04-21 20:41                           ` Juri Linkov
  0 siblings, 0 replies; 21+ messages in thread
From: Juri Linkov @ 2021-04-21 20:41 UTC (permalink / raw)
  To: Ergus; +Cc: contovob, Lars Ingebrigtsen, Tino Calancha, npostavs, 39512

tags 39512 fixed
close 39512 28.0.50
quit

>>>(or (thing-at-point 'region)
>>>    (thing-at-point 'url)
>>>    ;; (thing-at-point 'filename)
>>>    ;; (thing-at-point 'list)
>>>    (thing-at-point 'symbol))
>>>
> What I meant is just to add to thingatpt.el these 4 lines:
>
> +(put 'region 'bounds-of-thing-at-point
> +     (lambda ()
> +       (when (use-region-p)
> +         (cons (region-beginning) (region-end)))))
>
>> As I already mentioned; IMO the idea is to follow the same logic than here:
>>
>> https://github.com/abo-abo/swiper/blob/471d644d6bdd7d5dc6ca4efb405e6a6389dff245/ivy.el#L427
>>
>> where `(thing-at-point 'region)` is basically the first branch in the
>> cond.
>
> `(thing-at-point 'region)` will work with the patch above.
> Then we could add a customizable user option to specify
> in what order the user wants to try things at point, e.g.
>
>   `(region url file symbol)`

Now pushed to master and closed.

>> now I use C-M-SPC for this selection and then M-w to copy and C-s
>> C-y. If you add the command, then it will save the M-w and the C-y to
>> copy the region, because region will be already active... But will
>> require the M-s prefix any way... so not 2 but at least 1 bind will be
>> saved. You can consider this option in the future if you want...
>
> Yes, this is a nice way to yank the next list to the search with just
> C-M-SPC M-s M-.

Now works not only with 'C-M-SPC M-s M-.' but also with just 'M-s M-.'
due to 'sexp' in the default value.





^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2021-04-21 20:41 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-08 18:04 bug#39512: 28.0.50; Add command isearch-yank-region Tino Calancha
2020-02-08 23:47 ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-02-09  0:31 ` Juri Linkov
2020-02-09 11:21   ` Tino Calancha
2020-02-09 12:38   ` Tino Calancha
2020-02-10  0:45     ` Juri Linkov
2020-02-12 22:10       ` Juri Linkov
2020-08-09 11:28   ` Lars Ingebrigtsen
2020-08-09 23:23     ` Juri Linkov
2020-08-10  1:19       ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-08-10 23:49         ` Juri Linkov
2020-08-11  1:12           ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-08-11 23:13             ` Juri Linkov
2020-08-12 17:41               ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-08-12 23:44                 ` Juri Linkov
2020-08-13  3:14                   ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-04-15 20:52                     ` Juri Linkov
2021-04-18 15:34                       ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-04-20 20:29                         ` Juri Linkov
2021-04-21 20:41                           ` Juri Linkov
2020-08-10 17:00       ` Tino Calancha

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).