bug#41766: Make it possible to change regexp to identify and highlight grep matches via customization

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#41766: Make it possible to change regexp to identify and highlight grep matches via customization

Simon Lang-2


> Am 13.06.2020 um 23:59 schrieb Juri Linkov <[hidden email]>:
>
> 
>>
>> 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



Reply | Threaded
Open this post in threaded view
|

bug#41766: Make it possible to change regexp to identify and highlight grep matches via customization

Juri Linkov-2
>> 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.



Reply | Threaded
Open this post in threaded view
|

bug#41766: Make it possible to change regexp to identify and highlight grep matches via customization

Simon Lang
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 <[hidden email]>
Sent: 15 June 2020 00:08
To: Simon Lang
Cc: Simon Lang; Dmitry Gutov; Basil L. Contovounesios; [hidden email]
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.



Reply | Threaded
Open this post in threaded view
|

bug#41766: Make it possible to change regexp to identify and highlight grep matches via customization

Juri Linkov-2
> 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")