GNU bug report logs - #41766
Make it possible to change regexp to identify and highlight grep matches via customization

Previous Next

Package: emacs;

Reported by: Simon Lang <simon.lang <at> outlook.com>

Date: Mon, 8 Jun 2020 21:32:02 UTC

Severity: normal

Fixed in version 28.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Mon, 08 Jun 2020 21:32:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Simon Lang <simon.lang <at> outlook.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Mon, 08 Jun 2020 21:32:02 GMT) Full text and rfc822 format available.

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

From: Simon Lang <simon.lang <at> outlook.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: Make it possible to change regexp to identify and highlight grep
 matches via customization 
Date: Mon, 8 Jun 2020 20:25:06 +0000
[Message part 1 (text/plain, inline)]
When changing grep-command or grep-find-command to e.g. ripgrep matches are not highlighted in the grep buffer. This patches makes the regexp that is used to identify matches customizable and hence possible to adapt it to potential grep replacements.

For example:

change  grep command to 

"rg -n -H -S --no-heading --color always -e "

and grep-match-regexp to

"\033\\[[0-9]*m\033\\[[0-9]*1m\033\\[[0-9]*1m\\(.*?\\)\033\\[[0-9]*0m"

to get correct highlighting with ripgrep.


[0001-Make-regexp-used-to-highlight-grep-matches-customiza.patch (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Mon, 08 Jun 2020 23:39:02 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Simon Lang <simon.lang <at> outlook.com>
Cc: 41766 <at> debbugs.gnu.org
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Tue, 09 Jun 2020 02:18:47 +0300
> For example:
>
> change  grep command to
>
> "rg -n -H -S --no-heading --color always -e "
>
> and grep-match-regexp to

Thanks, grep-match-regexp is a good name, but your patch uses some less suitable
name grep-regexp-match.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Tue, 09 Jun 2020 00:45:02 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Simon Lang <simon.lang <at> outlook.com>, 41766 <at> debbugs.gnu.org
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Tue, 9 Jun 2020 03:44:47 +0300
On 08.06.2020 23:25, Simon Lang wrote:
> For example:
> 
> change  grep command to
> 
> "rg -n -H -S --no-heading --color always -e"

I wonder if we can use a similar customization more generally. For 
instance, in my testing Grep searches the full Emacs checkout in ~230ms, 
whereas RipGrep does that in ~40ms. The difference is perceptible.

The obvious idea is to use grep-template, but we're passing -s and -E to 
it. rg would interpret these options differently.

Here's a perftest patch if someone was personally curious about the 
difference:

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 5b5fb4bc47..2c6ed4da7d 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -1262,7 +1262,7 @@ xref-matches-in-files
        (dir (file-name-directory (car files)))
        (remote-id (file-remote-p dir))
        ;; 'git ls-files' can output broken symlinks.
-       (command (format "xargs -0 grep %s -snHE -e %s"
+       (command (format "xargs -0 rg %s -nH -e %s"
                         (if (and case-fold-search
                                  (isearch-no-upper-case-p regexp t))
                             "-i"
@@ -1275,6 +1275,7 @@ xref-matches-in-files
                        #'tramp-file-local-name
                        #'file-local-name)
                    files)))
+    (setq tt (time-to-seconds))
     (with-current-buffer output
       (erase-buffer)
       (with-temp-buffer
@@ -1289,6 +1290,7 @@ xref-matches-in-files
                                             shell-command-switch
                                             command)))
       (goto-char (point-min))
+      (message "%s" (- (time-to-seconds) tt))
       (when (and (/= (point-min) (point-max))
                  (not (looking-at grep-re))
                  ;; TODO: Show these matches as well somehow?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Tue, 09 Jun 2020 09:24:02 GMT) Full text and rfc822 format available.

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

From: Simon Lang <Simon.lang <at> outlook.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>, "41766 <at> debbugs.gnu.org"
 <41766 <at> debbugs.gnu.org>, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Tue, 9 Jun 2020 07:58:06 +0000
[Message part 1 (text/plain, inline)]
Hi,

I changed grep-regexp-match to grep-match-regexp.

Pls note that ripgrep knows about ignore files etc. hence the more fair comparison would probably be git grep (e.g. vc-git-grep). But ripgrep is still considerably faster and does not only work for git repositories.

Thanks!

________________________________________
From: DG <raaahh <at> gmail.com> on behalf of Dmitry Gutov <dgutov <at> yandex.ru>
Sent: 09 June 2020 01:44
To: Simon Lang; 41766 <at> debbugs.gnu.org
Subject: Re: bug#41766: Make it possible to change regexp to identify and highlight grep matches via customization

On 08.06.2020 23:25, Simon Lang wrote:
> For example:
>
> change  grep command to
>
> "rg -n -H -S --no-heading --color always -e"

I wonder if we can use a similar customization more generally. For
instance, in my testing Grep searches the full Emacs checkout in ~230ms,
whereas RipGrep does that in ~40ms. The difference is perceptible.

The obvious idea is to use grep-template, but we're passing -s and -E to
it. rg would interpret these options differently.

Here's a perftest patch if someone was personally curious about the
difference:

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 5b5fb4bc47..2c6ed4da7d 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -1262,7 +1262,7 @@ xref-matches-in-files
         (dir (file-name-directory (car files)))
         (remote-id (file-remote-p dir))
         ;; 'git ls-files' can output broken symlinks.
-       (command (format "xargs -0 grep %s -snHE -e %s"
+       (command (format "xargs -0 rg %s -nH -e %s"
                          (if (and case-fold-search
                                   (isearch-no-upper-case-p regexp t))
                              "-i"
@@ -1275,6 +1275,7 @@ xref-matches-in-files
                         #'tramp-file-local-name
                         #'file-local-name)
                     files)))
+    (setq tt (time-to-seconds))
      (with-current-buffer output
        (erase-buffer)
        (with-temp-buffer
@@ -1289,6 +1290,7 @@ xref-matches-in-files
                                              shell-command-switch
                                              command)))
        (goto-char (point-min))
+      (message "%s" (- (time-to-seconds) tt))
        (when (and (/= (point-min) (point-max))
                   (not (looking-at grep-re))
                   ;; TODO: Show these matches as well somehow?
[0001-Make-regexp-used-to-highlight-grep-matches-customiza.patch (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Tue, 09 Jun 2020 11:56:02 GMT) Full text and rfc822 format available.

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Simon Lang <Simon.lang <at> outlook.com>
Cc: Juri Linkov <juri <at> linkov.net>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Tue, 09 Jun 2020 12:55:25 +0100
Simon Lang <Simon.lang <at> outlook.com> writes:

> From bc9b736ff20a03e831bc5110283ccf9241127773 Mon Sep 17 00:00:00 2001
> From: Simon Lang <simon.lang <at> outlook.com>
> Date: Mon, 8 Jun 2020 20:47:08 +0100
> Subject: [PATCH] Make regexp used to highlight grep matches customizable
>
> * lisp/progmodes/grep.el
> ---
>  lisp/progmodes/grep.el | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
> index 7731be5965..3f0c99f6dc 100644
> --- a/lisp/progmodes/grep.el
> +++ b/lisp/progmodes/grep.el
> @@ -100,6 +100,9 @@ To change the default value, use \\[customize] or call the function
>    :set #'grep-apply-setting
>    :version "22.1")
>  
> +(defcustom grep-match-regexp "\033\\[0?1;31m\\(.*?\\)\033\\[[0-9]*m"
> +  "Regex definition to identify grep markers to highlight matches.")

Nit: How about "Regular expression matching grep markers to highlight."

Every defcustom also needs:

  :type 'regexp
  :version "28.1"

as well as possibly being announced in etc/NEWS.

I wonder, though: the default value matches some quite obscure codes
which aren't (and maybe shouldn't be) documented, so is a defcustom
really suitable for this?  Or would a defvar suffice?  (I don't have
strong feelings either way.)

Thanks,

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Tue, 09 Jun 2020 12:17:01 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Simon Lang <Simon.lang <at> outlook.com>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Tue, 9 Jun 2020 15:15:53 +0300
On 09.06.2020 10:58, Simon Lang wrote:
> Pls note that ripgrep knows about ignore files etc.

But not when passed specific file names on the command line, right? 
Which is what xargs does.

Sorry, it's a tangent from your patch's discussion. It looks good to me, 
but I'd rather defer the review to someone else.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Tue, 09 Jun 2020 14:09:03 GMT) Full text and rfc822 format available.

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

From: Simon Lang <Simon.lang <at> outlook.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>, "41766 <at> debbugs.gnu.org"
 <41766 <at> debbugs.gnu.org>, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Tue, 9 Jun 2020 12:41:21 +0000
True, I think you're right.

Cheers,

Simon

________________________________________
From: DG <raaahh <at> gmail.com> on behalf of Dmitry Gutov <dgutov <at> yandex.ru>
Sent: 09 June 2020 13:15
To: Simon Lang; 41766 <at> debbugs.gnu.org; Juri Linkov
Subject: Re: bug#41766: Make it possible to change regexp to identify and highlight grep matches via customization

On 09.06.2020 10:58, Simon Lang wrote:
> Pls note that ripgrep knows about ignore files etc.

But not when passed specific file names on the command line, right?
Which is what xargs does.

Sorry, it's a tangent from your patch's discussion. It looks good to me,
but I'd rather defer the review to someone else.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Tue, 09 Jun 2020 14:09:03 GMT) Full text and rfc822 format available.

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

From: Simon Lang <Simon.lang <at> outlook.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Juri Linkov <juri <at> linkov.net>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Tue, 9 Jun 2020 12:45:18 +0000
[Message part 1 (text/plain, inline)]
I am happy with defvar instead (attached - I changed the commit message accordingly). I admit it would not be straight forward to customize anyway without doing some research.

Thanks!


________________________________________
From: Basil L. Contovounesios <contovob <at> tcd.ie>
Sent: 09 June 2020 12:55
To: Simon Lang
Cc: Dmitry Gutov; 41766 <at> debbugs.gnu.org; Juri Linkov
Subject: Re: bug#41766: Make it possible to change regexp to identify and highlight grep matches via customization

Simon Lang <Simon.lang <at> outlook.com> writes:

> From bc9b736ff20a03e831bc5110283ccf9241127773 Mon Sep 17 00:00:00 2001
> From: Simon Lang <simon.lang <at> outlook.com>
> Date: Mon, 8 Jun 2020 20:47:08 +0100
> Subject: [PATCH] Make regexp used to highlight grep matches customizable
>
> * lisp/progmodes/grep.el
> ---
>  lisp/progmodes/grep.el | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
> index 7731be5965..3f0c99f6dc 100644
> --- a/lisp/progmodes/grep.el
> +++ b/lisp/progmodes/grep.el
> @@ -100,6 +100,9 @@ To change the default value, use \\[customize] or call the function
>    :set #'grep-apply-setting
>    :version "22.1")
>
> +(defcustom grep-match-regexp "\033\\[0?1;31m\\(.*?\\)\033\\[[0-9]*m"
> +  "Regex definition to identify grep markers to highlight matches.")

Nit: How about "Regular expression matching grep markers to highlight."

Every defcustom also needs:

  :type 'regexp
  :version "28.1"

as well as possibly being announced in etc/NEWS.

I wonder, though: the default value matches some quite obscure codes
which aren't (and maybe shouldn't be) documented, so is a defcustom
really suitable for this?  Or would a defvar suffice?  (I don't have
strong feelings either way.)

Thanks,

--
Basil
[0001-Remove-hardcoding-of-regexp-used-to-highlight-grep-m.patch (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Tue, 09 Jun 2020 14:25:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Simon Lang <simon.lang <at> outlook.com>
Cc: 41766 <at> debbugs.gnu.org
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Tue, 09 Jun 2020 17:24:27 +0300
> From: Simon Lang <simon.lang <at> outlook.com>
> Date: Mon, 8 Jun 2020 20:25:06 +0000
> 
> +(defcustom grep-regexp-match "\033\\[0?1;31m\\(.*?\\)\033\\[[0-9]*m"
> +  "Regex definition to identify grep markers to highlight matches.")

Please don't forget the :version tag and the NEWS and the manual
updates.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Tue, 09 Jun 2020 14:33:02 GMT) Full text and rfc822 format available.

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Simon Lang <Simon.lang <at> outlook.com>
Cc: Juri Linkov <juri <at> linkov.net>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Tue, 09 Jun 2020 15:32:01 +0100
Simon Lang <Simon.lang <at> outlook.com> writes:

> I am happy with defvar instead (attached - I changed the commit message
> accordingly). I admit it would not be straight forward to customize anyway
> without doing some research.

Thanks, just some convention tips from me.

> From 724d83a211aae922b719a28cc4f9cc75d88cc4ee Mon Sep 17 00:00:00 2001
> From: Simon Lang <simon.lang <at> outlook.com>
> Date: Tue, 9 Jun 2020 13:38:07 +0100
> Subject: [PATCH] Remove hardcoding of regexp used to highlight grep matches.
>
> * lisp/progmodes/grep.el

See etc/CONTRIBUTE for commit message conventions.
In particular, it should list the definitions being changed, e.g.:

* lisp/progmodes/grep.el (grep-match-regexp): New variable.
(grep-filter): Use it.

> ---
>  lisp/progmodes/grep.el | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
> index 7731be5965..618c2935f0 100644
> --- a/lisp/progmodes/grep.el
> +++ b/lisp/progmodes/grep.el
> @@ -353,6 +353,10 @@ Notice that using \\[next-error] or \\[compile-goto-error] modifies
>  (defvar grep-match-face	'match
>    "Face name to use for grep matches.")
>  
> +(defvar grep-match-regexp "\033\\[0?1;31m\\(.*?\\)\033\\[[0-9]*m"
> +  "Regular expression matching grep markers to highlight, used
> +in `grep-filter'.")

The first sentence of every docstring should fit on one line;
see (info "(elisp) Documentation Tips").  So you could move the mention
of grep-filter into a sentence of its own.

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Tue, 09 Jun 2020 14:45:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: juri <at> linkov.net, 41766 <at> debbugs.gnu.org, Simon.lang <at> outlook.com,
 dgutov <at> yandex.ru
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Tue, 09 Jun 2020 17:43:43 +0300
> From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
> Date: Tue, 09 Jun 2020 12:55:25 +0100
> Cc: "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
>  Dmitry Gutov <dgutov <at> yandex.ru>, Juri Linkov <juri <at> linkov.net>
> 
> I wonder, though: the default value matches some quite obscure codes
> which aren't (and maybe shouldn't be) documented, so is a defcustom
> really suitable for this?

They are SGR escape sequences that Grep emits to color its output.  We
should probably mention that in the doc string.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Wed, 10 Jun 2020 21:12:01 GMT) Full text and rfc822 format available.

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

From: Simon Lang <Simon.lang <at> outlook.com>
To: Eli Zaretskii <eliz <at> gnu.org>, "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: "juri <at> linkov.net" <juri <at> linkov.net>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 "dgutov <at> yandex.ru" <dgutov <at> yandex.ru>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Wed, 10 Jun 2020 21:11:09 +0000
[Message part 1 (text/plain, inline)]
Hi,

I now something to the manual, NEWS and changed the doc string + commit msg as advised. I decided on defcustom in the end because this way it is easy for the user to figure out what goes wrong in case he/she modifies grep-command and highlighting is missing. Hope that is fine.

Pls let me know if there is anything else I need to change.

Thanks!

________________________________________
From: Eli Zaretskii <eliz <at> gnu.org>
Sent: 09 June 2020 15:43
To: Basil L. Contovounesios
Cc: Simon.lang <at> outlook.com; 41766 <at> debbugs.gnu.org; dgutov <at> yandex.ru; juri <at> linkov.net
Subject: Re: bug#41766: Make it possible to change regexp to identify and highlight grep matches via customization

> From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
> Date: Tue, 09 Jun 2020 12:55:25 +0100
> Cc: "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
>  Dmitry Gutov <dgutov <at> yandex.ru>, Juri Linkov <juri <at> linkov.net>
>
> I wonder, though: the default value matches some quite obscure codes
> which aren't (and maybe shouldn't be) documented, so is a defcustom
> really suitable for this?

They are SGR escape sequences that Grep emits to color its output.  We
should probably mention that in the doc string.
[0001-lisp-progmodes-grep.el-grep-match-regexp-New-variabl.patch (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Wed, 10 Jun 2020 21:57:02 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Simon Lang <Simon.lang <at> outlook.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, Eli Zaretskii <eliz <at> gnu.org>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 "dgutov <at> yandex.ru" <dgutov <at> yandex.ru>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Thu, 11 Jun 2020 00:52:28 +0300
> I now something to the manual, NEWS and changed the doc string + commit msg
> as advised. I decided on defcustom in the end because this way it is easy
> for the user to figure out what goes wrong in case he/she modifies
> grep-command and highlighting is missing. Hope that is fine.

In the long run what should be customizable is not escape sequences,
but a choice of the grep program with a list of such options:

  "GNU grep"
  "ripgrep"
  ...

i.e. the same customization as for the selection of the web browser
in browse-url.el.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Wed, 10 Jun 2020 22:15:01 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>, Simon Lang <Simon.lang <at> outlook.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Thu, 11 Jun 2020 01:14:20 +0300
On 11.06.2020 00:52, Juri Linkov wrote:
> In the long run what should be customizable is not escape sequences,
> but a choice of the grep program with a list of such options:
> 
>    "GNU grep"
>    "ripgrep"
>    ...

That could simply be done by adding :tag to each option value, right?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Wed, 10 Jun 2020 23:21:02 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 Simon Lang <Simon.lang <at> outlook.com>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Thu, 11 Jun 2020 02:10:53 +0300
>> In the long run what should be customizable is not escape sequences,
>> but a choice of the grep program with a list of such options:
>>    "GNU grep"
>>    "ripgrep"
>>    ...
>
> That could simply be done by adding :tag to each option value, right?

Indeed, the same way as it's already used in

(defcustom grep-find-use-xargs nil
  "How to invoke find and grep.
If `exec', use `find -exec {} ;'.
If `exec-plus' use `find -exec {} +'.
If `gnu', use `find -print0' and `xargs -0'.
If `gnu-sort', use `find -print0', `sort -z' and `xargs -0'.
Any other value means to use `find -print' and `xargs'.

This variable's value takes effect when `grep-compute-defaults' is called."
  :type '(choice (const :tag "find -exec {} ;" exec)
                 (const :tag "find -exec {} +" exec-plus)
                 (const :tag "find -print0 | xargs -0" gnu)
                 (const :tag "find -print0 | sort -z | xargs -0'" gnu-sort)
                 string
		 (const :tag "Not Set" nil))
  :set #'grep-apply-setting
  :version "27.1")




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Wed, 10 Jun 2020 23:25:02 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 Simon Lang <Simon.lang <at> outlook.com>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Thu, 11 Jun 2020 02:24:11 +0300
On 11.06.2020 02:10, Juri Linkov wrote:
>>> In the long run what should be customizable is not escape sequences,
>>> but a choice of the grep program with a list of such options:
>>>     "GNU grep"
>>>     "ripgrep"
>>>     ...
>> That could simply be done by adding :tag to each option value, right?
> Indeed, the same way as it's already used in

Then the main thing missing is the alternative value(s).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Sat, 13 Jun 2020 06:51:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Simon Lang <Simon.lang <at> outlook.com>
Cc: contovob <at> tcd.ie, juri <at> linkov.net, 41766 <at> debbugs.gnu.org, dgutov <at> yandex.ru
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Sat, 13 Jun 2020 09:50:16 +0300
> From: Simon Lang <Simon.lang <at> outlook.com>
> CC: "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>, "dgutov <at> yandex.ru"
> 	<dgutov <at> yandex.ru>, "juri <at> linkov.net" <juri <at> linkov.net>
> Date: Wed, 10 Jun 2020 21:11:09 +0000
> 
> I now something to the manual, NEWS and changed the doc string + commit msg as advised. I decided on defcustom in the end because this way it is easy for the user to figure out what goes wrong in case he/she modifies grep-command and highlighting is missing. Hope that is fine.
> 
> Pls let me know if there is anything else I need to change.

Thanks, a few minor comments below.

> +by grep.  The matching of the sequences is controlled by
> +@code{grep-match-regexp}, which can be customize to accommodate
> +different grep programs.               ^^^^^^^^^

Typo: should be "customized".

Also, "grep" should be "Grep".

> +** Grep changes:
> +
> +*** New variable 'grep-match-regexp' matches grep markers to highlight.
> +Grep emits SGR ANSI escape sequences to color its output. The new variable
> +'grep-match-regexp' holds the regular expression to match the appropriate
> +markers in order to provide highlighting in the source buffer. The variable
> +can be customized to accommodate other grep-like tools.

Please leave 2 spaces between sentences, per our conventions.
> +(defcustom grep-match-regexp "\033\\[0?1;31m\\(.*?\\)\033\\[[0-9]*m"
> +  "Regular expression matching grep markers to highlight.
> +It matches SGR ANSI escape sequences which are emitted by grep to
> +color its output. This variable is used in `grep-filter'."

Likewise here.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Sat, 13 Jun 2020 09:49:01 GMT) Full text and rfc822 format available.

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

From: Simon Lang <Simon.lang <at> outlook.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "contovob <at> tcd.ie" <contovob <at> tcd.ie>, "juri <at> linkov.net" <juri <at> linkov.net>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 "dgutov <at> yandex.ru" <dgutov <at> yandex.ru>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Sat, 13 Jun 2020 09:48:13 +0000
[Message part 1 (text/plain, inline)]
Thank you!

Attached pls find the revised patch.

Simon

________________________________________
From: Eli Zaretskii <eliz <at> gnu.org>
Sent: 13 June 2020 07:50
To: Simon Lang
Cc: contovob <at> tcd.ie; 41766 <at> debbugs.gnu.org; dgutov <at> yandex.ru; juri <at> linkov.net
Subject: Re: bug#41766: Make it possible to change regexp to identify and highlight grep matches via customization

> From: Simon Lang <Simon.lang <at> outlook.com>
> CC: "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>, "dgutov <at> yandex.ru"
>       <dgutov <at> yandex.ru>, "juri <at> linkov.net" <juri <at> linkov.net>
> Date: Wed, 10 Jun 2020 21:11:09 +0000
>
> I now something to the manual, NEWS and changed the doc string + commit msg as advised. I decided on defcustom in the end because this way it is easy for the user to figure out what goes wrong in case he/she modifies grep-command and highlighting is missing. Hope that is fine.
>
> Pls let me know if there is anything else I need to change.

Thanks, a few minor comments below.

> +by grep.  The matching of the sequences is controlled by
> +@code{grep-match-regexp}, which can be customize to accommodate
> +different grep programs.               ^^^^^^^^^

Typo: should be "customized".

Also, "grep" should be "Grep".

> +** Grep changes:
> +
> +*** New variable 'grep-match-regexp' matches grep markers to highlight.
> +Grep emits SGR ANSI escape sequences to color its output. The new variable
> +'grep-match-regexp' holds the regular expression to match the appropriate
> +markers in order to provide highlighting in the source buffer. The variable
> +can be customized to accommodate other grep-like tools.

Please leave 2 spaces between sentences, per our conventions.
> +(defcustom grep-match-regexp "\033\\[0?1;31m\\(.*?\\)\033\\[[0-9]*m"
> +  "Regular expression matching grep markers to highlight.
> +It matches SGR ANSI escape sequences which are emitted by grep to
> +color its output. This variable is used in `grep-filter'."

Likewise here.
[0001-lisp-progmodes-grep.el-grep-match-regexp-New-variabl.patch (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Sat, 13 Jun 2020 09:52:01 GMT) Full text and rfc822 format available.

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

From: Simon Lang <Simon.lang <at> outlook.com>
To: Juri Linkov <juri <at> linkov.net>, Dmitry Gutov <dgutov <at> yandex.ru>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Sat, 13 Jun 2020 09:51:29 +0000
Maybe also a way to easily register new search tools? Otherwise one might be locked into the available options again - and if there is a new tool the interface might be not that stable, so there is the danger that it breaks until there is a new emacs release.

Simon

________________________________________
From: Juri Linkov <juri <at> linkov.net>
Sent: 11 June 2020 00:10
To: Dmitry Gutov
Cc: Simon Lang; Basil L. Contovounesios; 41766 <at> debbugs.gnu.org
Subject: Re: bug#41766: Make it possible to change regexp to identify and highlight grep matches via customization

>> In the long run what should be customizable is not escape sequences,
>> but a choice of the grep program with a list of such options:
>>    "GNU grep"
>>    "ripgrep"
>>    ...
>
> That could simply be done by adding :tag to each option value, right?

Indeed, the same way as it's already used in

(defcustom grep-find-use-xargs nil
  "How to invoke find and grep.
If `exec', use `find -exec {} ;'.
If `exec-plus' use `find -exec {} +'.
If `gnu', use `find -print0' and `xargs -0'.
If `gnu-sort', use `find -print0', `sort -z' and `xargs -0'.
Any other value means to use `find -print' and `xargs'.

This variable's value takes effect when `grep-compute-defaults' is called."
  :type '(choice (const :tag "find -exec {} ;" exec)
                 (const :tag "find -exec {} +" exec-plus)
                 (const :tag "find -print0 | xargs -0" gnu)
                 (const :tag "find -print0 | sort -z | xargs -0'" gnu-sort)
                 string
                 (const :tag "Not Set" nil))
  :set #'grep-apply-setting
  :version "27.1")




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Sat, 13 Jun 2020 23:00:03 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Simon Lang <Simon.lang <at> outlook.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Sun, 14 Jun 2020 01:50:49 +0300
> Maybe also a way to easily register new search tools? Otherwise one might
> be locked into the available options again - and if there is a new tool the
> interface might be not that stable, so there is the danger that it breaks
> until there is a new emacs release.

No problem, you can even dynamically add an option available only
when a grep program is installed:

(defcustom grep-program nil
  "The default grep program for `grep-command' and `grep-find-command'.
This variable's value takes effect when `grep-compute-defaults' is called."
  :type `(choice (const :tag "GNU grep" (purecopy "grep"))
                 ,@(if (executable-find "rg") '((const :tag "ripgrep" "rg")))
                 (string :tag "Other grep program")
                 (const :tag "Not Set" nil))
  :version "28.1")

Or for a completely new tool:

(nconc (get 'grep-program 'custom-type) '((const :tag "ripgrep" "rg")))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Sun, 14 Jun 2020 11:27:02 GMT) Full text and rfc822 format available.

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

From: Simon Lang <simon.lang <at> yellowfrog.io>
To: Juri Linkov <juri <at> linkov.net>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 Simon Lang <simon.lang <at> outlook.com>, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Sun, 14 Jun 2020 10:12:03 +0100

> Am 13.06.2020 um 23:59 schrieb Juri Linkov <juri <at> linkov.net>:
> 
> 
>> 
>> Maybe also a way to easily register new search tools? Otherwise one might
>> be locked into the available options again - and if there is a new tool the
>> interface might be not that stable, so there is the danger that it breaks
>> until there is a new emacs release.
> 
> No problem, you can even dynamically add an option available only
> when a grep program is installed:
> 
> (defcustom grep-program nil
>  "The default grep program for `grep-command' and `grep-find-command'.
> This variable's value takes effect when `grep-compute-defaults' is called."
>  :type `(choice (const :tag "GNU grep" (purecopy "grep"))
>                 ,@(if (executable-find "rg") '((const :tag "ripgrep" "rg")))
>                 (string :tag "Other grep program")
>                 (const :tag "Not Set" nil))
>  :version "28.1")
> 
> Or for a completely new tool:
> 
> (nconc (get 'grep-program 'custom-type) '((const :tag "ripgrep" "rg")))

Maybe still fine to merge my patch now and look at this later? One would not contradict the other, I would think? 

@Eli: sorry I hope my revised patch was not buried below this other discussion now. 

Thank you! 

Simon




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Sun, 14 Jun 2020 23:56:03 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Simon Lang <simon.lang <at> yellowfrog.io>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 Simon Lang <simon.lang <at> outlook.com>, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Mon, 15 Jun 2020 02:08:12 +0300
>> Or for a completely new tool:
>>
>> (nconc (get 'grep-program 'custom-type) '((const :tag "ripgrep" "rg")))
>
> Maybe still fine to merge my patch now and look at this later?
> One would not contradict the other, I would think?

Yes, your patch is a good starting point, thanks.

A minor worry is that you put back defcustom instead of leaving defvar.
I can't imagine a user who might want to customize it using literal
escape characters in the customization input field.

Maybe this is fine for now, but in the long run it would be better to
allow an alist of mappings from the grep program name to its escape sequences
like

(defvar grep-match-regexp
 '(("grep" . "\033\\[0?1;31m\\(.*?\\)\033\\[[0-9]*m")
   ("ripgrep" . ""\033\\[[0-9]*m\033\\[[0-9]*1m\033\\[[0-9]*1m\\(.*?\\)\033\\[[0-9]*0m"")))

Then you can easily switch between grep and ripgrep without losing
information about their escape sequences.

But of course this could be improved after installing your current patch.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Mon, 22 Jun 2020 19:10:02 GMT) Full text and rfc822 format available.

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

From: Simon Lang <Simon.lang <at> outlook.com>
To: Juri Linkov <juri <at> linkov.net>, Simon Lang <simon.lang <at> yellowfrog.io>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Mon, 22 Jun 2020 19:09:12 +0000
I changed it to defcustom because this is how I currently use it and no one had strong feelings about it. 
Do you want me to change it to devfar again? Or is it fine as it is? 

Thanks,

Simon

________________________________________
From: Juri Linkov <juri <at> linkov.net>
Sent: 15 June 2020 00:08
To: Simon Lang
Cc: Simon Lang; Dmitry Gutov; Basil L. Contovounesios; 41766 <at> debbugs.gnu.org
Subject: Re: bug#41766: Make it possible to change regexp to identify and highlight grep matches via customization

>> Or for a completely new tool:
>>
>> (nconc (get 'grep-program 'custom-type) '((const :tag "ripgrep" "rg")))
>
> Maybe still fine to merge my patch now and look at this later?
> One would not contradict the other, I would think?

Yes, your patch is a good starting point, thanks.

A minor worry is that you put back defcustom instead of leaving defvar.
I can't imagine a user who might want to customize it using literal
escape characters in the customization input field.

Maybe this is fine for now, but in the long run it would be better to
allow an alist of mappings from the grep program name to its escape sequences
like

(defvar grep-match-regexp
 '(("grep" . "\033\\[0?1;31m\\(.*?\\)\033\\[[0-9]*m")
   ("ripgrep" . ""\033\\[[0-9]*m\033\\[[0-9]*1m\033\\[[0-9]*1m\\(.*?\\)\033\\[[0-9]*0m"")))

Then you can easily switch between grep and ripgrep without losing
information about their escape sequences.

But of course this could be improved after installing your current patch.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Tue, 23 Jun 2020 00:16:01 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Simon Lang <Simon.lang <at> outlook.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 Simon Lang <simon.lang <at> yellowfrog.io>, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Tue, 23 Jun 2020 02:50:35 +0300
> I changed it to defcustom because this is how I currently use it and
> no one had strong feelings about it.
> Do you want me to change it to devfar again? Or is it fine as it is?

If you don't want to change it to devfar in your patch, then for users
there should be an easy way to revert their customization to the default
value.  This would be possible by using such menu of possible values:

(defcustom grep-match-regexp "\033\\[0?1;31m\\(.*?\\)\033\\[[0-9]*m"
  "Regular expression matching grep markers to highlight.
It matches SGR ANSI escape sequences which are emitted by grep to
color its output.  This variable is used in `grep-filter'."
  :type '(choice (regexp :tag "GNU grep" "\033\\[0?1;31m\\(.*?\\)\033\\[[0-9]*m")
		 (regexp :tag "ripgrep" "\033\\[[0-9]*m\033\\[[0-9]*1m\033\\[[0-9]*1m\\(.*?\\)\033\\[[0-9]*0m")
                 (regexp :tag "Other grep programs"))
  :version "28.1")




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#41766; Package emacs. (Sun, 27 Sep 2020 12:58:02 GMT) Full text and rfc822 format available.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Simon Lang <Simon.lang <at> outlook.com>
Cc: "contovob <at> tcd.ie" <contovob <at> tcd.ie>, Eli Zaretskii <eliz <at> gnu.org>,
 "41766 <at> debbugs.gnu.org" <41766 <at> debbugs.gnu.org>,
 "dgutov <at> yandex.ru" <dgutov <at> yandex.ru>, "juri <at> linkov.net" <juri <at> linkov.net>
Subject: Re: bug#41766: Make it possible to change regexp to identify and
 highlight grep matches via customization
Date: Sun, 27 Sep 2020 14:56:56 +0200
Simon Lang <Simon.lang <at> outlook.com> writes:

> Attached pls find the revised patch.

Thanks.  It looks like everybody agreed that this was a good change, but
then nobody applied your patch, so I did that now (to Emacs 28).

There was then some followup discussion about possible tweaks, and those
can now be done, and I'm closing this bug report.

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




bug marked as fixed in version 28.1, send any further explanations to 41766 <at> debbugs.gnu.org and Simon Lang <simon.lang <at> outlook.com> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Sun, 27 Sep 2020 12:58:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 26 Oct 2020 11:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 183 days ago.

Previous Next


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