atomontage / xterm-color

ANSI & xterm-256 color text property translator for Emacs
BSD 2-Clause "Simplified" License
214 stars 20 forks source link

ag functionality breaks with xterm-color compilation filter #37

Closed efroemling closed 4 years ago

efroemling commented 4 years ago

I'm really loving xterm-color; thanks for writing it!

I just wanted to note one issue I ran into (and my somewhat hacky workaround).

I've added xterm-color functionality to compilation output via the method described in the docs:

(defun my/advice-compilation-filter (f proc string)
  (funcall f proc (xterm-color-filter string)))

I'm using projectile and the ag search functionality it comes with, and I found that this breaks the ability to jump to search results in ag output buffers. It would seem that some of the filtering xterm-color is doing interferes with the filtering ag is doing to parse its output.

My hacky workaround is to only apply the filter if the buffer name starts with "*compilation"

(defun my/advice-compilation-filter (f proc string)
  (funcall f proc (if (string-prefix-p "*compilation" (buffer-name (process-buffer proc)))
                      (xterm-color-filter string) string)))

I'm not really sure if this would be considered xterm-color's problem or Ag's problem; I just wanted to mention it. Would there perhaps be an elegant way to differentiate compilation buffers kicked off by the user vs ones used by things like ag?

atomontage commented 4 years ago

I can't think of a reason as to why that'd be the case, but if you give me a minimal way to reproduce this issue (starting from Emacs -q describe all the steps I need to take including what packages I should install and how I should configure them), I can take a look.

efroemling commented 4 years ago

Ok here's a sequence I can run to recreate it; hopefully this is helpful. This is emacs 26.3 on a Mac via homebrew (the 'cask' version)

atomontage commented 4 years ago

The problem is that ag.el relies on ANSI color codes being part of the input since that's what it uses to match on. From ag-filter (ag.el:632):

(while (re-search-forward "\033\\[30;43m\\(.*?\\)\033\\[0m\033\\[K" end 1)
            (replace-match (propertize (match-string 1)
                                       'face nil 'font-lock-face 'ag-match-face)
                           t t)))
        ;; Add marker at start of line for files. This is used by the match
        ;; in `compilation-error-regexp-alist' to extract the file name.
        (when ag-group-matches
          (goto-char beg)
          (while (re-search-forward "\033\\[1;32m\\(.*\\)\033\\[0m\033\\[K" end 1)
            (replace-match
             (concat "File: " (propertize (match-string 1) 'face nil 'font-lock-face
                                          'compilation-info))
             t t)))
        ;; Delete all remaining escape sequences
        (goto-char beg)
        (while (re-search-forward "\033\\[[0-9;]*[mK]" end 1)
          (replace-match "" t t))))))

This is not going to work when the input is filtered through xterm-color.el, since that translates all ANSI escape sequences into properties. It's also a bad idea to hardcode this sort of assumption in ag.el, so I would definitely advise its developer to find a better way to do matching.

I don't use it myself and I'm not going to dig any deeper but there may be an easy way to configure it so that it doesn't rely on ANSI escapes. You should ask its developer/maintainer. I'm closing this issue.

efroemling commented 4 years ago

Ok, thanks for taking a look at that. I'll forward the info along to the ag.el maintainer.