* bug#9438: grep regressions
@ 2011-09-05 8:34 Juri Linkov
2011-09-07 1:22 ` Stefan Monnier
0 siblings, 1 reply; 4+ messages in thread
From: Juri Linkov @ 2011-09-05 8:34 UTC (permalink / raw)
To: 9438
grep used to navigate directly to the correct column and highlight the
match instantly with `next-error-highlight'. But now grep.el doesn't do
that because escape sequences are processed earlier than font-locking.
One way to fix this regression is to find matches highligted by
`grep-filter' and take their column positions.
Unfortunately, I can't find a simpler way to fix it than this patch:
=== modified file 'lisp/progmodes/grep.el'
--- lisp/progmodes/grep.el 2011-08-22 09:54:38 +0000
+++ lisp/progmodes/grep.el 2011-09-05 08:30:35 +0000
@@ -344,7 +344,27 @@ (defvar grep-last-buffer nil
;;;###autoload
(defconst grep-regexp-alist
'(("^\\(.+?\\)\\(:[ \t]*\\)\\([1-9][0-9]*\\)\\2"
- 1 3)
+ 1 3
+ ;; Calculate column positions (col . end-col) of first grep match on a line
+ ((lambda ()
+ (when grep-highlight-matches
+ (setq compilation-error-screen-columns nil)
+ (save-excursion
+ (let* ((beg (progn (goto-char (match-end 0)) (point)))
+ (end (line-end-position))
+ (mbeg (text-property-any beg end 'font-lock-face 'match)))
+ (when mbeg
+ (- mbeg beg))))))
+ .
+ (lambda ()
+ (when grep-highlight-matches
+ (save-excursion
+ (let* ((beg (progn (goto-char (match-end 0)) (point)))
+ (end (line-end-position))
+ (mbeg (text-property-any beg end 'font-lock-face 'match))
+ (mend (and mbeg (next-single-property-change mbeg 'font-lock-face nil end))))
+ (when mend
+ (- mend beg))))))))
;; Rule to match column numbers is commented out since no known grep
;; produces them
;; ("^\\(.+?\\)\\(:[ \t]*\\)\\([0-9]+\\)\\2\\(?:\\([0-9]+\\)\\(?:-\\([0-9]+\\)\\)?\\2\\)?"
@@ -353,17 +373,6 @@ (defconst grep-regexp-alist
;; handle weird file names (with colons in them) as well as possible.
;; E.g. we use [1-9][0-9]* rather than [0-9]+ so as to accept ":034:" in
;; file names.
- ("^\\(\\(.+?\\):\\([1-9][0-9]*\\):\\).*?\
-\\(\033\\[01;31m\\(?:\033\\[K\\)?\\)\\(.*?\\)\\(\033\\[[0-9]*m\\)"
- 2 3
- ;; Calculate column positions (beg . end) of first grep match on a line
- ((lambda ()
- (setq compilation-error-screen-columns nil)
- (- (match-beginning 4) (match-end 1)))
- .
- (lambda () (- (match-end 5) (match-end 1)
- (- (match-end 4) (match-beginning 4)))))
- nil 1)
("^Binary file \\(.+\\) matches$" 1 nil nil 0 1))
"Regexp used to match grep hits. See `compilation-error-regexp-alist'.")
^ permalink raw reply [flat|nested] 4+ messages in thread* bug#9438: grep regressions
2011-09-05 8:34 bug#9438: grep regressions Juri Linkov
@ 2011-09-07 1:22 ` Stefan Monnier
2011-09-07 12:09 ` Juri Linkov
0 siblings, 1 reply; 4+ messages in thread
From: Stefan Monnier @ 2011-09-07 1:22 UTC (permalink / raw)
To: Juri Linkov; +Cc: 9438
> Unfortunately, I can't find a simpler way to fix it than this patch:
It doesn't look too terrible.
> + ;; Calculate column positions (col . end-col) of first grep match on a line
> + ((lambda ()
> + (when grep-highlight-matches
> + (setq compilation-error-screen-columns nil)
Why set it here? I know it used to be set similarly, but I think it
should be set once and for all in grep-mode instead.
> + (save-excursion
> + (let* ((beg (progn (goto-char (match-end 0)) (point)))
Why not
(save-excursion
(goto-char (match-end 0))
(let* ((beg (point))
Or better yet:
(let* ((beg (match-end 0))
(end (save-excursion (goto-char beg) (line-end-position)))
-- Stefan
^ permalink raw reply [flat|nested] 4+ messages in thread* bug#9438: grep regressions
2011-09-07 1:22 ` Stefan Monnier
@ 2011-09-07 12:09 ` Juri Linkov
2011-09-08 20:18 ` Juri Linkov
0 siblings, 1 reply; 4+ messages in thread
From: Juri Linkov @ 2011-09-07 12:09 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 9438
>> + (when grep-highlight-matches
>> + (setq compilation-error-screen-columns nil)
>
> Why set it here? I know it used to be set similarly, but I think it
> should be set once and for all in grep-mode instead.
I just discovered `grep-error-screen-columns', a defcustom
added in 2004. Its default value is nil, so I suppose
its purpose was to override `compilation-error-screen-columns'.
It's currently unused because it misses the following lines
in `grep-mode':
(set (make-local-variable 'compilation-error-screen-columns)
grep-error-screen-columns)
I added that to `grep-mode'.
> Or better yet:
>
> (let* ((beg (match-end 0))
> (end (save-excursion (goto-char beg) (line-end-position)))
Yes, this is better. I installed these changes.
Beside `grep-error-screen-columns', I discovered two more unused defcustoms
in grep.el:
(defcustom grep-window-height nil
"*Number of lines in a grep window. If nil, use `compilation-window-height'."
:type '(choice (const :tag "Default" nil)
integer)
:version "22.1"
:group 'grep)
(defcustom grep-scroll-output nil
"*Non-nil to scroll the *grep* buffer window as output appears.
Setting it causes the grep commands to put point at the end of their
output window so that the end of the output is always visible rather
than the begining."
:type 'boolean
:version "22.1"
:group 'grep)
IIUC, since the docstring says "If nil, use `compilation-window-height'"
they should be used in `grep-mode' only when the value is non-nil like:
(when grep-window-height
(set (make-local-variable 'compilation-window-height)
grep-window-height))
(when grep-scroll-output
(set (make-local-variable 'compilation-scroll-output)
grep-scroll-output))
^ permalink raw reply [flat|nested] 4+ messages in thread* bug#9438: grep regressions
2011-09-07 12:09 ` Juri Linkov
@ 2011-09-08 20:18 ` Juri Linkov
0 siblings, 0 replies; 4+ messages in thread
From: Juri Linkov @ 2011-09-08 20:18 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 9438
> Beside `grep-error-screen-columns', I discovered two more unused defcustoms
> in grep.el:
>
> (defcustom grep-window-height nil
> "*Number of lines in a grep window. If nil, use `compilation-window-height'."
> :type '(choice (const :tag "Default" nil)
> integer)
> :version "22.1"
> :group 'grep)
>
> (defcustom grep-scroll-output nil
> "*Non-nil to scroll the *grep* buffer window as output appears.
> Setting it causes the grep commands to put point at the end of their
> output window so that the end of the output is always visible rather
> than the begining."
> :type 'boolean
> :version "22.1"
> :group 'grep)
>
> IIUC, since the docstring says "If nil, use `compilation-window-height'"
> they should be used in `grep-mode' only when the value is non-nil like:
>
> (when grep-window-height
> (set (make-local-variable 'compilation-window-height)
> grep-window-height))
> (when grep-scroll-output
> (set (make-local-variable 'compilation-scroll-output)
> grep-scroll-output))
According to bzr logs, before revno:54335 this used to be:
(if grep-window-height
(set (make-local-variable 'compilation-window-height)
grep-window-height))
(set (make-local-variable 'compilation-scroll-output)
grep-scroll-output)
So either we should restore this code in `grep-process-setup' to use
the customized values of defcustoms, or remove unused defcustoms.
I have no settled opinion what would be better to do.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-09-08 20:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-05 8:34 bug#9438: grep regressions Juri Linkov
2011-09-07 1:22 ` Stefan Monnier
2011-09-07 12:09 ` Juri Linkov
2011-09-08 20:18 ` Juri Linkov
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).