GNU bug report logs - #27341
eww-search-words should prompt if region inactive

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 27341 in the body.
You can then email your comments to 27341 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Alex Branham <branham <at> utexas.edu>
To: "" <bug-gnu-emacs <at> gnu.org>
Subject: eww-search-words should prompt if region inactive
Date: Mon, 12 Jun 2017 13:21:15 +0200

[Message part 1 (text/plain, inline)]

eww-search-words (M-S M-w) searches for the active region and doesn't do anything useful if the region isn't active. This patch has it prompt the user for a search string if the region isn't active.

Alex

[0001-eww-earch-prefix-prompt-for-query-if-region-inactive.patch (text/x-diff, attachment)]

Message #8 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: Alex Branham <branham <at> utexas.edu>
To: 27341 <at> debbugs.gnu.org
Subject: updated patch to fix docstring
Date: Tue, 13 Jun 2017 11:24:25 +0200

[Message part 1 (text/plain, inline)]

Hi -

I realized I forgot to update the docstring; this patch incorporates that change as well

Alex




From 4545b2fb1d6f047b05cbb1e19a50fb09809d5a31 Mon Sep 17 00:00:00 2001
From: Alex Branham <branham <at> utexas.edu>
Date: Tue, 13 Jun 2017 11:21:35 +0200
Subject: [PATCH] eww-search-words prompts user for search if region inactive

---
 lisp/net/eww.el | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index fe31657914..aa358dce0f 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -313,10 +313,13 @@ word(s) will be searched for via `eww-search-prefix'."

 ;;;###autoload
 (defun eww-search-words (&optional beg end)
-  "Search the web for the text between BEG and END.
+  "If region is active, search the web for the text between BEG and END.
+Else, prompt the user for a search string.
 See the `eww-search-prefix' variable for the search engine used."
   (interactive "r")
-  (eww (buffer-substring beg end)))
+  (if (region-active-p)
+      (eww (buffer-substring beg end))
+    (eww (read-string "Query: "))))

 (defun eww-open-in-new-buffer ()
   "Fetch link at point in a new EWW buffer."
--
2.13.1

[0002-eww-search-words-prompt-user-for-search-if-region.patch (text/x-diff, attachment)]

Message #11 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Alex Branham <branham <at> utexas.edu>
Cc: 27341 <at> debbugs.gnu.org
Subject: Re: bug#27341: updated patch to fix docstring
Date: Sun, 25 Jun 2017 10:29:22 -0400

Alex Branham <branham <at> utexas.edu> writes:

> -  (eww (buffer-substring beg end)))
> +  (if (region-active-p)

If think we should use `use-region-p' instead.  Or perhaps just check
directly if region is empty?  I guess searching for an empty string
doesn't make much sense even if `use-empty-active-region' is t.

Message #14 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: Alex Branham <branham <at> utexas.edu>
To: npostavs <at> users.sourceforge.net
Cc: 27341 <at> debbugs.gnu.org
Subject: Re: bug#27341: updated patch to fix docstring
Date: Mon, 26 Jun 2017 16:00:46 +0200

[Message part 1 (text/plain, inline)]

Here's a new patch that checks the region directly, ignoring `use-empty-active-region'.

From c3babb73d8f29ff3f27aacbac99c7226ef4b32a0 Mon Sep 17 00:00:00 2001
From: Alex Branham <branham <at> utexas.edu>
Date: Mon, 26 Jun 2017 15:57:15 +0200
Subject: [PATCH] eww-search-words prompt user for query if region active and
 not empty

---
 lisp/net/eww.el | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index fe31657914..cf46530d35 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -314,9 +314,15 @@ word(s) will be searched for via `eww-search-prefix'."
 ;;;###autoload
 (defun eww-search-words (&optional beg end)
   "Search the web for the text between BEG and END.
-See the `eww-search-prefix' variable for the search engine used."
+  If region is active (and not whitespace), search the web for
+the text between BEG and END. Else, prompt the user for a search
+string. See the `eww-search-prefix' variable for the search
+engine used."
   (interactive "r")
-  (eww (buffer-substring beg end)))
+  (let ((region-string (buffer-substring beg end)))
+    (if (not (string-match-p "\\`[ \n\t\r\v\f]*\\'" region-string))
+        (eww region-string)
+      (eww (read-string "Query: ")))))

 (defun eww-open-in-new-buffer ()
   "Fetch link at point in a new EWW buffer."
--
2.13.1


On Sun 25 Jun 2017 at 14:29, npostavs <at> users.sourceforge.net wrote:

> Alex Branham <branham <at> utexas.edu> writes:
>
>> -  (eww (buffer-substring beg end)))
>> +  (if (region-active-p)
>
> If think we should use `use-region-p' instead.  Or perhaps just check
> directly if region is empty?  I guess searching for an empty string
> doesn't make much sense even if `use-empty-active-region' is t.

[0003-eww-search-words-prompt-user-for-query-if-region-act.patch (text/x-diff, attachment)]

Message #17 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Alex Branham <branham <at> utexas.edu>
Cc: 27341 <at> debbugs.gnu.org
Subject: Re: bug#27341: updated patch to fix docstring
Date: Mon, 26 Jun 2017 19:50:45 -0400

Alex Branham <branham <at> utexas.edu> writes:

> Here's a new patch that checks the region directly, ignoring `use-empty-active-region'.

Looks good, I'll push in a few days.  You haven't assigned copyright,
have you?

Message #20 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: Alex Branham <branham <at> utexas.edu>
To: npostavs <at> users.sourceforge.net
Cc: 27341 <at> debbugs.gnu.org
Subject: Re: bug#27341: updated patch to fix docstring
Date: Tue, 27 Jun 2017 07:32:06 +0200

[Message part 1 (text/plain, inline)]

No, I haven't. Do I need to for something this small? Happy to do so if
it's necessary.

Alex

On Jun 27, 2017 1:49 AM, <npostavs <at> users.sourceforge.net> wrote:

> Alex Branham <branham <at> utexas.edu> writes:
>
> > Here's a new patch that checks the region directly, ignoring
> `use-empty-active-region'.
>
> Looks good, I'll push in a few days.  You haven't assigned copyright,
> have you?
>

[Message part 2 (text/html, inline)]

Message #23 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: Alex Branham <branham <at> utexas.edu>
To: npostavs <at> users.sourceforge.net
Cc: 27341 <at> debbugs.gnu.org
Subject: Re: bug#27341: updated patch to fix docstring
Date: Tue, 27 Jun 2017 15:23:17 +0200

[Message part 1 (text/plain, inline)]

Actually, we need to check (use-region-p) anyway in case the mark hasn't been activated yet in the buffer:


From 685f3d0d2bfea7b3d6aad03d46a247fac25702d0 Mon Sep 17 00:00:00 2001
From: Alex Branham <branham <at> utexas.edu>
Date: Tue, 27 Jun 2017 15:20:27 +0200
Subject: [PATCH] eww-search-words prompt for query if region inactive or blank

---
 lisp/net/eww.el | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index fe31657914..a90caa7fc4 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -314,9 +314,16 @@ word(s) will be searched for via `eww-search-prefix'."
 ;;;###autoload
 (defun eww-search-words (&optional beg end)
   "Search the web for the text between BEG and END.
-See the `eww-search-prefix' variable for the search engine used."
+    If region is active (and not whitespace), search the web for
+  the text between BEG and END. Else, prompt the user for a search
+  string. See the `eww-search-prefix' variable for the search
+  engine used."
   (interactive "r")
-  (eww (buffer-substring beg end)))
+  (if (use-region-p)
+      (let ((region-string (buffer-substring beg end)))
+        (if (not (string-match-p "\\`[ \n\t\r\v\f]*\\'" region-string))
+            (eww region-string)))
+    (eww (read-string "Query: "))))

 (defun eww-open-in-new-buffer ()
   "Fetch link at point in a new EWW buffer."
--
2.13.2



On Tue 27 Jun 2017 at 05:32, Alex Branham <branham <at> utexas.edu> wrote:

> No, I haven't. Do I need to for something this small? Happy to do so if
> it's necessary.
>
> Alex
>
> On Jun 27, 2017 1:49 AM, <npostavs <at> users.sourceforge.net> wrote:
>
>> Alex Branham <branham <at> utexas.edu> writes:
>>
>> > Here's a new patch that checks the region directly, ignoring
>> `use-empty-active-region'.
>>
>> Looks good, I'll push in a few days.  You haven't assigned copyright,
>> have you?
>>

[0004-eww-search-words-prompt-user-for-query-if-region-inactive.patch (text/x-diff, attachment)]

Message #26 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex Branham <branham <at> utexas.edu>
Cc: 27341 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: bug#27341: updated patch to fix docstring
Date: Tue, 27 Jun 2017 17:40:55 +0300

> From: Alex Branham <branham <at> utexas.edu>
> Date: Tue, 27 Jun 2017 07:32:06 +0200
> Cc: 27341 <at> debbugs.gnu.org
> 
> No, I haven't. Do I need to for something this small? Happy to do so if it's necessary. 

No, you don't.  But it's good to start the ball rolling anyway, so
that we could accept your future contributions without limitations.

Thanks.

Message #29 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Alex Branham <branham <at> utexas.edu>
Cc: 27341 <at> debbugs.gnu.org
Subject: Re: bug#27341: updated patch to fix docstring
Date: Thu, 29 Jun 2017 20:54:02 -0400

Alex Branham <branham <at> utexas.edu> writes:

> Actually, we need to check (use-region-p) anyway in case the mark
> hasn't been activated yet in the buffer:

Oops, good catch, I hadn't thought of that.

> Subject: [PATCH] eww-search-words prompt for query if region inactive or blank
>
> ---

Could you add a ChangeLog entry to the commit message as described in
CONTRIBUTE, please?

>  (defun eww-search-words (&optional beg end)
>    "Search the web for the text between BEG and END.
> -See the `eww-search-prefix' variable for the search engine used."
> +    If region is active (and not whitespace), search the web for
> +  the text between BEG and END. Else, prompt the user for a search
> +  string. See the `eww-search-prefix' variable for the search
> +  engine used."

Doc strings shouldn't be indented like that, and there should be a
double space between sentences.

>    (interactive "r")
> -  (eww (buffer-substring beg end)))
> +  (if (use-region-p)
> +      (let ((region-string (buffer-substring beg end)))
> +        (if (not (string-match-p "\\`[ \n\t\r\v\f]*\\'" region-string))
> +            (eww region-string)))
> +    (eww (read-string "Query: "))))

This silently does nothing if the region is active but just blanks, did
you mean to fallback to querying in that case?

Message #32 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: Alex Branham <branham <at> utexas.edu>
To: npostavs <at> users.sourceforge.net
Cc: 27341 <at> debbugs.gnu.org
Subject: Re: bug#27341: updated patch to fix docstring
Date: Fri, 30 Jun 2017 08:25:58 +0200

[Message part 1 (text/plain, inline)]

On Fri 30 Jun 2017 at 00:54, npostavs <at> users.sourceforge.net wrote:

> Could you add a ChangeLog entry to the commit message as described in
> CONTRIBUTE, please?

Yes, I have done so. Let me know if it's not right.

>
>>  (defun eww-search-words (&optional beg end)
>>    "Search the web for the text between BEG and END.
>> -See the `eww-search-prefix' variable for the search engine used."
>> +    If region is active (and not whitespace), search the web for
>> +  the text between BEG and END. Else, prompt the user for a search
>> +  string. See the `eww-search-prefix' variable for the search
>> +  engine used."
>
> Doc strings shouldn't be indented like that, and there should be a
> double space between sentences.

Fixed.

>
>>    (interactive "r")
>> -  (eww (buffer-substring beg end)))
>> +  (if (use-region-p)
>> +      (let ((region-string (buffer-substring beg end)))
>> +        (if (not (string-match-p "\\`[ \n\t\r\v\f]*\\'" region-string))
>> +            (eww region-string)))
>> +    (eww (read-string "Query: "))))
>
> This silently does nothing if the region is active but just blanks, did
> you mean to fallback to querying in that case?

Fixed, thanks

[0005-Make-eww-search-words-prompt-for-query-if-nothing-se.patch (text/x-diff, attachment)]

Message #35 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Alex Branham <branham <at> utexas.edu>
Cc: 27341 <at> debbugs.gnu.org
Subject: Re: bug#27341: updated patch to fix docstring
Date: Sun, 02 Jul 2017 14:32:50 -0400

Alex Branham <branham <at> utexas.edu> writes:

> On Fri 30 Jun 2017 at 00:54, npostavs <at> users.sourceforge.net wrote:
>
>> Could you add a ChangeLog entry to the commit message as described in
>> CONTRIBUTE, please?
>
> Yes, I have done so. Let me know if it's not right.

Looks good, thanks.

>> Doc strings shouldn't be indented like that, and there should be a
>> double space between sentences.
>
> Fixed.

You missed the double spacing before the "Else,".

> Actually, we need to check (use-region-p) anyway in case the mark
> hasn't been activated yet in the buffer:

Hmm, I just tried this and it doesn't quite do the job.  I believe the
problem is that the (interactive "r") form throws an error when the mark
hasn't been activated yet.  To handle this case correctly you need to
replace the "r" with some lisp code that produces the argument list.

Message #38 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: Alex Branham <branham <at> utexas.edu>
To: npostavs <at> users.sourceforge.net
Cc: 27341 <at> debbugs.gnu.org
Subject: Re: bug#27341: updated patch to fix docstring
Date: Sun, 02 Jul 2017 20:45:49 +0200

[Message part 1 (text/plain, inline)]

On Sun 02 Jul 2017 at 18:32, npostavs <at> users.sourceforge.net wrote:

> Alex Branham <branham <at> utexas.edu> writes:
>
>> On Fri 30 Jun 2017 at 00:54, npostavs <at> users.sourceforge.net wrote:
>
>>> Doc strings shouldn't be indented like that, and there should be a
>>> double space between sentences.
>>
>> Fixed.
>
> You missed the double spacing before the "Else,".

D'oh! Fixed this time, sorry!

>> Actually, we need to check (use-region-p) anyway in case the mark
>> hasn't been activated yet in the buffer:
>
> Hmm, I just tried this and it doesn't quite do the job.  I believe the
> problem is that the (interactive "r") form throws an error when the mark
> hasn't been activated yet.  To handle this case correctly you need to
> replace the "r" with some lisp code that produces the argument list.

Ok, now I don't use "r" and just get the region directly. I think that'll do the trick?

Thanks,
Alex

[0006-Make-eww-search-words-prompt-for-query-if-nothing-se.patch (text/x-diff, attachment)]

Message #41 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Alex Branham <branham <at> utexas.edu>
Cc: 27341 <at> debbugs.gnu.org
Subject: Re: bug#27341: updated patch to fix docstring
Date: Sun, 02 Jul 2017 15:13:29 -0400

Alex Branham <branham <at> utexas.edu> writes:

>> You missed the double spacing before the "Else,".
>
> D'oh! Fixed this time, sorry!

Oh, the funny indentation came back, do you have some Emacs setting
that's doing that?

>>> Actually, we need to check (use-region-p) anyway in case the mark
>>> hasn't been activated yet in the buffer:
>>
>> Hmm, I just tried this and it doesn't quite do the job.  I believe the
>> problem is that the (interactive "r") form throws an error when the mark
>> hasn't been activated yet.  To handle this case correctly you need to
>> replace the "r" with some lisp code that produces the argument list.
>
> Ok, now I don't use "r" and just get the region directly. I think that'll do the trick?

Oh yeah, that works.  We should try not to add compiler warnings though:

  ELC      net/eww.elc

In toplevel form:
../../emacs-master/lisp/net/eww.el:315:1:Warning: Unused lexical argument
    ‘end’
../../emacs-master/lisp/net/eww.el:315:1:Warning: Unused lexical argument
    ‘beg’

I think we can just drop the arguments as well, lisp programs can always
do (eww (buffer-substring BEG END)) instead.

Do you think the prompt string should be a bit more detailed?  "Query"
seems a bit vague (e.g., consider if you hit M-s M-w by accident).
Maybe we should just do (call-interactively 'eww) and then we'll get the
prompt from there.

Message #44 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: Alex Branham <branham <at> utexas.edu>
To: npostavs <at> users.sourceforge.net
Cc: 27341 <at> debbugs.gnu.org
Subject: Re: bug#27341: updated patch to fix docstring
Date: Sun, 02 Jul 2017 21:25:56 +0200

[Message part 1 (text/plain, inline)]

On Sun 02 Jul 2017 at 19:13, npostavs <at> users.sourceforge.net wrote:

> Alex Branham <branham <at> utexas.edu> writes:
>
>>> You missed the double spacing before the "Else,".
>>
>> D'oh! Fixed this time, sorry!
>
> Oh, the funny indentation came back, do you have some Emacs setting
> that's doing that?

Ugh. Fixed. I think it has to do with how I'm copy/pasting from my own init file.

>> Ok, now I don't use "r" and just get the region directly. I think that'll do the trick?
>
> Oh yeah, that works.  We should try not to add compiler warnings though:
>
>   ELC      net/eww.elc
>
> In toplevel form:
> ../../emacs-master/lisp/net/eww.el:315:1:Warning: Unused lexical argument
>     ‘end’
> ../../emacs-master/lisp/net/eww.el:315:1:Warning: Unused lexical argument
>     ‘beg’
>
> I think we can just drop the arguments as well, lisp programs can always
> do (eww (buffer-substring BEG END)) instead.

Dropped.

> Do you think the prompt string should be a bit more detailed?  "Query"
> seems a bit vague (e.g., consider if you hit M-s M-w by accident).
> Maybe we should just do (call-interactively 'eww) and then we'll get the
> prompt from there.

Yes, that's much better. Done, thanks.

[0007-Make-eww-search-words-prompt-for-query-if-nothing-se.patch (text/x-diff, attachment)]

Message #47 received at 27341 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Alex Branham <branham <at> utexas.edu>
Cc: 27341 <at> debbugs.gnu.org
Subject: Re: bug#27341: updated patch to fix docstring
Date: Sun, 02 Jul 2017 16:23:37 -0400

tags 27341 fixed
close 27341 26.1
quit

Alex Branham <branham <at> utexas.edu> writes:
>
>> Do you think the prompt string should be a bit more detailed?  "Query"
>> seems a bit vague (e.g., consider if you hit M-s M-w by accident).
>> Maybe we should just do (call-interactively 'eww) and then we'll get the
>> prompt from there.
>
> Yes, that's much better. Done, thanks.

Okay, pushed to master [1: 1fd6ca40fd], thanks for hanging in there! :)

[1: 1fd6ca40fd]: 2017-07-02 16:19:57 -0400
  Make eww-search-words prompt for query if nothing selected
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=1fd6ca40fd50989b8f82b287c04a5079a051ed09

Previous Next

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