Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in rg-filter impacting font locking #99

Closed
charignon opened this issue Oct 3, 2020 · 12 comments
Closed

Bug in rg-filter impacting font locking #99

charignon opened this issue Oct 3, 2020 · 12 comments

Comments

@charignon
Copy link

charignon commented Oct 3, 2020

When performing searches that lead to a lot of results (thousands or more), I notice that on a few lines the column number remains event when I set (setq rg-show-columns nil). This impacts the font-locking and visual of the result buffer.

If I were to guess, this may have to do with the fact that we think we are dealing with whole line in rg-filter when we are not. I know we account for line ending in there but I think there is a subtle bug that shows with search that display a large volume of data. I haven't figured out more than that so far.

@dajva
Copy link
Owner

dajva commented Oct 3, 2020

Thanks for the report. I have not seen that myself.
Which version of emacs do you use? M-x emacs-version
What rg settings do you use?
Eval the following in scratch and copy the output in the report.

(defun rg-print-settings ()
  "Print all the active customization options for this package."
  (interactive)
  (let ((settings
         (thread-last (custom-group-members 'rg nil)
           (seq-filter (lambda (item)
                         (eq (cadr item) 'custom-variable)))
           (mapcar (lambda (member)
                     (cons (car member) (symbol-value (car member))))))))
    (message "--------- RG settings ---------\n%s\n------------- END -------------"
             (mapconcat
              (lambda (setting) (format "%S: %S" (car setting) (cdr setting)))
              settings "\n"))))
(rg-print-settings)

@dajva
Copy link
Owner

dajva commented Oct 3, 2020

Can you also post a screen shot of the faulty rendering?

@charignon
Copy link
Author

"--------- RG settings ---------                                                                                                      
rg-use-transient-menu: t                                                                                                              
rg-show-columns: nil                                                                                                                  
rg-group-result: t                                                                                                                    
rg-show-header: t                                                                                                                     
rg-hide-command: t                                                                                                                    
rg-align-position-numbers: t                                                                                                          
rg-align-line-number-field-length: 4                                                                                                  
rg-align-column-number-field-length: 3                                                                                                
rg-align-line-column-separator: \" \"                                                                                                 
rg-align-position-content-separator: \" \"                                                                                            
rg-custom-type-aliases: ((\"gyp\" . \"*.gyp *.gypi\"))                                                                                
rg-executable: \"/usr/bin/rg\"                                                                                                        
rg-command-line-flags: nil                                                                                                            
rg-ignore-case: case-fold-search                                                                                                      
rg-keymap-prefix: \"^Cs\"                                                                                                             
rg-default-alias-fallback: \"all\"                                                                                                    
rg-buffer-name: \"rg\"                                                                                                                
rg-ignore-ripgreprc: t                                                                                                                
------------- END -------------"

Screenshot from 2020-10-03 16-06-47

For example searching for "defun" in "fastai"
Using emacs 27.1.50 on Manjaro with rg 12.1.1

@dajva
Copy link
Owner

dajva commented Oct 4, 2020

Hmm, this seems like a tricky one. It's a bit hard to see in the screenshot but looks like the font changes on the line of the error and all the results after.
Could you place cursor on such a faulty line, line and column number separately and run M-x describe-text-properties?
And the same for correct line after that looks to have a modified fortification.

@charignon
Copy link
Author

charignon commented Oct 4, 2020

Another example with the information that you need:
Screenshot from 2020-10-04 11-22-11
At the beginning of the green "faulty" line on "198":
Screenshot from 2020-10-04 11-22-32
At the beginning of the line right before on "194":
Screenshot from 2020-10-04 11-23-03

@dajva
Copy link
Owner

dajva commented Oct 5, 2020

Thanks, this looks like an ansi-color issue.
I thought ansi-color only integrated into comint mode which shouldn't be used by this package. Do you have some custom hook in compilation mode for ansi-color or similar?

Anyway. My guess would be (judging from the screen shots) that the output is first handled by rg in rg-filter which will filter out all SGR color codes except for an incomplete line at the end. After that the ansi-color somehow comes into play and apply color and removing color codes that were left out. When new output is inserted in the buffer rg-filter is called again but now the expected leading SGR codes at the start of the first line is missing and the line:column: is not matched.

@GambolingPangolin
Copy link

I'm running into the same problem

"--------- RG settings ---------
rg-use-transient-menu: t
rg-show-columns: t
rg-group-result: t
rg-show-header: t
rg-hide-command: t
rg-align-position-numbers: t
rg-align-line-number-field-length: 4
rg-align-column-number-field-length: 3
rg-align-line-column-separator: \" \"
rg-align-position-content-separator: \" \"
rg-custom-type-aliases: ((\"gyp\" . \"*.gyp *.gypi\"))
rg-executable: \"/usr/bin/rg\"
rg-command-line-flags: nil
rg-ignore-case: case-fold-search
rg-keymap-prefix: \"�s\"
rg-default-alias-fallback: \"all\"
rg-buffer-name: \"rg\"
rg-ignore-ripgreprc: t
------------- END -------------"

Emacs 27.1 (doom)
Arch Linux
rg 12.1.1

@dajva
Copy link
Owner

dajva commented Oct 6, 2020

For doom the problems lies here: https://github.com/hlissner/doom-emacs/blob/29b12de83e5f8ce76e9ff38549753c69bc507650/core/core-ui.el#L373
This should really be reported as a doom bug since this could break any compilation-mode downstream package. OTOH, this package, built-in grep.el and ag.el could be affected in different ways, unknown to what extent.

The config workaround for this would be something along these lines:

(defun rg-clear-doom-ansi-color-compilation-hook ()
  (make-local-variable 'compilation-filter-hook)
  (remove-hook 'compilation-filter-hook #'doom-apply-ansi-color-to-compilation-buffer-h))

(add-hook 'rg-mode-hook 'rg-clear-doom-ansi-color-compilation-hook)

I will make improvements to this package to make it less sensitive to such things but it's not possible to solve for real here.

@charignon Are your setup also doom based?

@GambolingPangolin
Copy link

Thanks for the tip! I'll see if I can get your workaround to work in my setup.

@charignon
Copy link
Author

charignon commented Oct 7, 2020

Yes, the workaround works for me, and it is not an issue with rg.el then. Thank you so much for your responsiveness and help on this @dajva 👍 🥇

@GambolingPangolin
Copy link

It works for me too. Thanks again!

@dajva dajva closed this as completed in a442f6b Oct 10, 2020
@dajva
Copy link
Owner

dajva commented Oct 10, 2020

I realised this could actually be fixed from this package so should work without the hook now.

dajva added a commit that referenced this issue Oct 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants