helm-do-grep-ag: ripgrep colours in search results

[manuel-uberti]

Expected behavior

For Helm to use the helm-grep-match face, the command-line call to grep or other backends has to specify --color=never (or equivalent). Otherwise Helm will use whatever the command-line output would use for the matching text: typically bold text in red (ANSI color 1).

Using this generally works as expected, by showing the theme's faces:

(setq helm-grep-ag-command
      "rg --color=never --smart-case --no-heading --line-number %s %s %s")

This is a screenshot showing the output of helm-do-grep-ag with dired as its input:

Actual behaviour

The same setup fails in two ways when the input is an incomplete/invalid regular expression, such as \(dired:

• It matches dired as if it were a string.
• It shows the matching lines, but not the contents.

This is a screenshot showing the output of helm-do-grep-ag with \(dired as its input:

Actual behavior (from emacs-helm.sh if possible, see note at the bottom)

Steps to reproduce (recipe)

• run emacs-helm.sh
• evaluate the following code:

(setq helm-grep-ag-command
      "rg --color=never --smart-case --no-heading --line-number %s %s %s")

• M-x helm-do-grep-ag RET \(dired

Describe versions of Helm, Emacs, operating system, etc.

• Helm: commit 9101738a4ff56f838641ea412dfc2c3d24f5be82
• Emacs: GNU Emacs 28.0.50 (f42db4b6e1598c12924cce4bbe4d67e6d86b7963)
• OS: Ubuntu 20.04 LTS

Are you using emacs-helm.sh to reproduce this bug? (yes/no):

Yes

Are you using Spacemacs? (yes/no):

No

Notes

The rg executable can parse an arbitrary colour value specified as an option for --colors (see the manpage). This uses #0030a6 as the match's background (expressed as 0x00,0x30,0xa6):

# command-line call
rg helm --color=always --colors 'match:bg:0x00,0x30,0xa6' --colors match:fg:white --colors match:style:bold --smart-case --no-heading

The interpretation of the blue #0030a6 works on the command-line, but not in the Helm buffer.

I originally reported this problem on the @protesilaos' modus-themes issue tracker, where you can find further discussion, if you want: https://gitlab.com/protesilaos/modus-themes/-/issues/49

[thierryvolpiatto]

Manuel Uberti <notifications@github.com> writes:

Expected behavior For Helm to use the helm-grep-match face, the command-line call to grep or other backends has to specify --color=never (or equivalent). Otherwise Helm will use whatever the command-line output would use for the matching text: typically bold text in red (ANSI color 1). Using this generally works as expected, by showing the theme's faces: (setq helm-grep-ag-command "rg --color=never --smart-case --no-heading --line-number %s %s %s")

No, this is a deprecated usage that is still supported for backward compatibility, it is slow, you have better time using "--color=always" which is much faster and also work as expected. However the setting of colors have changed several time along ripgrep versions, what is documented in helm is no more valid (--color-match), you have to use "--colors" like this: "--colors 'match:fg:yellow'" (replace yellow by whatever you want). See rg man page for more infos on this. I will update `helm-grep-ag-command` docstring. Once you have configured `helm-grep-ag-command` with "--colors", don't forget to configure as well `helm-grep-ag-pipe-cmd-switches`, otherwise when specifying multi patterns, only the first will be colorized in yellow (for our example). Here from my config using ripgrep 12.1.1: (setq helm-pdfgrep-default-read-command "evince --page-label=%p '%f'" helm-grep-default-command "ack-grep -Hn --color --smart-case --no-group %e %p %f" helm-grep-default-recurse-command "ack-grep -H --color --smart-case --no-group %e %p %f" helm-grep-ag-command "rg --color=always --colors 'match:fg:yellow' --smart-case --no-heading --line-number %s %s %s" helm-grep-ag-pipe-cmd-switches '("--colors 'match:fg:yellow'") helm-grep-git-grep-command "git --no-pager grep -n%cH --color=always --exclude-standard --no-index --full-name -e %p -- %f")

This is a screenshot showing the output of helm-do-grep-ag with dired as its input: 1 Actual behaviour The same setup fails in two ways when the input is an incomplete/invalid regular expression, such as \(dired: • It matches dired as if it were a string. • It shows the matching lines, but not the contents.

Yeah, not surprized, the code for colorizing matching output is now really old.

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

[manuel-uberti]

Using --colors 'match:fg:yellow' works, but what I don't understand is how using something like --colors 'match:bg:0x00,0x30,0xa6' doesn't. (See my Notes above).

[thierryvolpiatto]

Hmm, I don't know, I only know how to do basic settings, perhaps you can ask @BurntSushi the developer of ripgrep.

[thierryvolpiatto]

Is it working on command line ?

[manuel-uberti]

Yes, it is working from the command line.

[thierryvolpiatto]

Manuel Uberti <notifications@github.com> writes:

Yes, it is working from the command line.

So probably Emacs is not supporting these colors (just a guess, I am not expert on rgb, ansi etc...) But I recall enhanced ansi sequences are not supported and need to be replaced by something more basic when used in Emacs, see: https://github.com/thierryvolpiatto/emacs-tv-config/blob/master/tv-utils.el#L1136

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

[manuel-uberti]

Thanks Thierry, I think you may have found something here. With this:

(setq helm-grep-ag-command
        "rg --color=ansi --colors match:fg:32,31,32 --colors match:style:nobold --smart-case --no-heading --line-number %s %s %s")

I can see the foreground colour. @protesilaos what do you think?

[protesilaos]

Thank you both for looking into this!

@manuel-uberti Your last snippet works for the foreground. But I cannot get the expected result for the background with --colors match:fg:32,31,32. So maybe something can be done there.

That granted, I was about to comment that the values for the terminal colours is controlled by ansi-color-names-vector. Below is a proof-of-concept where I swap the original red with another colour.

Using this command:

(setq helm-grep-ag-command
      "rg helm --color=always --colors match:bg:red --colors match:fg:black --colors match:style:bold --smart-case --no-heading --line-number %s %s %s")

Original (no changes to the ansi vector)

With the demo change

Idea

So perhaps users could do something like (this does not work):

(let ((ansi-color-names-vector [black "#f8ddea" green yellow blue magenta cyan white]))
  (call-interactively 'helm-do-grep-ag))

[manuel-uberti]

Interesting. So what is the next step here? I don't think this is Helm-related now, nor rg for that matter.

@protesilaos are you going to add the change to ansi-color-names-vector to the themes?

[protesilaos]

@manuel-uberti Changing that at the theme level would probably cause more problems, because other commands are likely expecting to read from it.

The cleaner solution for a theme would be to tweak the values of a face. Absent that, I would need to experiment more with the code I posted under the "Idea" heading. Though that is not an ideal solution either.

[protesilaos]

This is the way a user may update the ANSI colour values:

(ansi-color-map-update 'ansi-color-names-vector ["#000000" "#a60000" "#005e00" "#813e00" "#0030a6" "#721045" "#00538b" "#ffffff"]

The sequence is: black, red, green, yellow, blue, magenta, cyan, white.

Demo where I change the "red" value (preview colours with M-x rainbow-mode):

I am not sure what the best options is and would like to hear from you. What I now see are these main possibilities:

1. Users could just change the ansi-color-names-vector at their config level (I can help you @manuel-uberti on the themes' issue tracker).
2. The vector could be dynamically set and reset on a per-function basis. More difficult to maintain, but unlikely to cause unforeseen issues with other aspects of Emacs.
3. A Helm face could be taking over the ANSI colour code.

Your thoughts?

[BurntSushi]

However the setting of colors have changed several time along ripgrep
versions

Could you elaborate? I don't think this has changed at all.

I'm happy to answer any ripgrep specific questions.

[thierryvolpiatto]

Andrew Gallant <notifications@github.com> writes:

However the setting of colors have changed several time along ripgrep versions Could you elaborate? I don't think this has changed at all.

IIRC At the first times there was no option at all, then it was --color-match, nowaday it is --colors, is this right?

I'm happy to answer any ripgrep specific questions.

Thanks (and thanks for ripgrep ;-)).

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

[thierryvolpiatto]

Protesilaos Stavrou <notifications@github.com> writes:

This is the way a user may update the ANSI colour values: (ansi-color-map-update 'ansi-color-names-vector ["#000000" "#a60000" "#005e00" "#813e00" "#0030a6" "#721045" "#00538b" "#ffffff"] The sequence is: black, red, green, yellow, blue, magenta, cyan, white. Demo where I change the "red" value (preview colours with M-x rainbow-mode): Screensh Screensh I am not sure what the best options is and would like to hear from you. What I now see are these main possibilities: 1. Users could just change the ansi-color-names-vector at their config level (I can help you @manuel-uberti on the themes' issue tracker). 2. The vector could be dynamically set and reset on a per-function basis. More difficult to maintain, but unlikely to cause unforeseen issues with other aspects of Emacs. 3. A Helm face could be taking over the ANSI colour code. Your thoughts?

Option 3. seems the best, PR welcome.

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

[manuel-uberti]

@protesilaos

I'd avoid option 2 because it'd mean more maintenance work for something that should not be this complicated.

Option 1 would need to be well documented, and would put the user in charge of the problem.

Option 3 seems the more reasonable: provided that everything is in place, the user would just have to install ripgrep, Helm, and a theme supporting this colour setup and nothing more would be required.

WRT a Helm face, helm-grep-match is already present. Couldn't this be re-used?

[protesilaos]

@manuel-uberti I agree that option 3 is the best. @thierryvolpiatto is welcome to a possible PR, but I do not yet know how to implement that, so as to help you in that regard.

[manuel-uberti]

I am not sure either.

ansi-color-names-vector isn't strictly related to Helm, and it is a vector, not a single specific face. A defcustom could be used to apply the needed colours to ansi-color-names-vector, but then what would the use of helm-grep-match be?

[protesilaos]

Thinking about it, messing around with that vector would be fragile. Perhaps the answer is to tweak helm-grep-highlight-match?

[manuel-uberti]

That uses helm-add-face-text-properties to apply helm-grep-match, so I guess it still is a matter of having the correct face to highlight the matches.

But then again, the problem is mixing ansi-color-names-vector and a defface.

[thierryvolpiatto]

Manuel Uberti <notifications@github.com> writes:

Using this generally works as expected, by showing the theme's faces: (setq helm-grep-ag-command "rg --color=never --smart-case --no-heading --line-number %s %s %s") This is a screenshot showing the output of helm-do-grep-ag with dired as its input:

The reason why with these settings nothing is printed is that the transformer is using "\(dired" as an elisp regexp whereas rg is using this same regexp as a pcre regexp. I fixed this but I wonder if it wouldn't be better to just drop support for --color=never, will see.

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

[manuel-uberti]

Just tried latest commit (4071dca7f80b590a9d089ff790fb54224fad110d), and now I am getting the desired effect with:

(setq helm-grep-ag-command
        "rg --color=never --smart-case --no-heading --line-number %s %s %s")

@protesilaos could you give it a try? I am pretty sure we can close both this ticket and the one on your issue tracker now.

Thank you Thierry once again for the amazing support.

[thierryvolpiatto]

Manuel Uberti <notifications@github.com> writes:

That uses helm-add-face-text-properties to apply helm-grep-match, so I guess it still is a matter to have the correct face to highlight the matches.

No, helm-grep-highlight-match is never used when ansi is in use i.e. when using "--color=always".

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

[manuel-uberti]

Oh I see, thanks for the clarification. :)

[BurntSushi]

However the setting of colors have changed several time along ripgrep versions Could you elaborate? I don't think this has changed at all.
IIRC At the first times there was no option at all, then it was --color-match, nowaday it is --colors, is this right?

Ah nope, it was originally introduced as --colors way back in ripgrep 0.3.0, which was released about 3.5 years ago. (It was only a few months after ripgrep's original release.) It has stayed the same since then, so there has been very little churn in this feature from my perspective.

I'm happy to answer any ripgrep specific questions.
Thanks (and thanks for ripgrep ;-)).

:-)

[thierryvolpiatto]

Andrew Gallant <notifications@github.com> writes:

However the setting of colors have changed several time along ripgrep versions Could you elaborate? I don't think this has changed at all. IIRC At the first times there was no option at all, then it was --color-match, nowaday it is --colors, is this right? Ah nope, it was originally introduced as --colors way back in ripgrep 0.3.0, which was released about 3.5 years ago. (It was only a few months after ripgrep's original release.) It has stayed the same since then, so there has been very little churn in this feature from my perspective.

Ah yes, I mix up with AG which have --color-match, sorry for confusion. Thanks.

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

[jagrg]

A few things I noticed (using ./emacs-helm.sh) with:

(setq helm-grep-ag-command
      "rg --color=never --smart-case --no-heading --line-number %s %s %s")

If I type defun helm str, "defun" is not highlighted.

With --color=always, "defun" is red while "helm" and "str" are green.

Also, if I set nohighlight to nil in helm-grep-ag-class, the three words are red which is what I'd expect, and is what I see with C-x C-f C-s. Maybe the nohighlight slot should be set conditionally. Anyhow, if this is an issue with the ripgrep version I'm using (11.0.2), what is the correct way of changing the value of nohighlight? The code below does nothing AFAICT.

(defmethod helm-setup-user-source ((source helm-grep-ag-class))
  (setf (slot-value source 'nohighlight) nil))

Thank you all.

[thierryvolpiatto]

Jonathan Gregory <notifications@github.com> writes:

A few things I noticed (using ./emacs-helm.sh) with: (setq helm-grep-ag-command "rg --color=never --smart-case --no-heading --line-number %s %s %s") If I type defun helm str, "defun" is not highlighted.

Will look at this tomorrow, too late now.

With --color=always, "defun" is red while "helm" and "str" are green.

Read helm-grep-ag-command docstring, it is explained.

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

[jagrg]

On 15 Jun 2020, Thierry Volpiatto ***@***.***> wrote: Read helm-grep-ag-command docstring, it is explained.

Ok. I see now. I missed the `helm-grep-ag-pipe-cmd-switches` variable. Anyhow, I also had to add the --no-config flag to both `helm-grep-ag-command` and `helm-grep-ag-pipe-cmd-switches` variables. It may be worth mentioning this flag in the documentation. I think the colors set in the helm variables won't work if colors are also set in the $RIPGREP_CONFIG_PATH file using a 256-color palette (e.g. --colors=match:bg:24).

…

-- Jonathan

[jagrg]

Thierry, it's working now. Thanks! Here's what I'm using: (setq helm-grep-ag-command "rg --no-config --hidden --color=always \ --colors 'match:fg:yellow' --colors 'match:style:nobold' \ --no-heading -S -n %s %s %s" helm-grep-ag-pipe-cmd-switches '("--no-config --colors 'match:fg:yellow' --colors 'match:style:nobold'")) BTW I also think we should set `helm-grep-input-idle-delay` to a lower value (e.g. 0.1) despite the flickering. This is the type of command which I think users may favour speed. WDYT?

[thierryvolpiatto]

Jonathan Gregory <notifications@github.com> writes:

Thierry, it's working now. Thanks!

Great!

BTW I also think we should set `helm-grep-input-idle-delay` to a lower value (e.g. 0.1) despite the flickering. This is the type of command which I think users may favour speed. WDYT?

Done. Thanks.

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997