* bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line @ 2011-10-06 6:14 ` Ari Roponen 2011-10-06 15:27 ` Juri Linkov [not found] ` <handler.9679.C.13363577086087.notifdonectrl.0@debbugs.gnu.org> 0 siblings, 2 replies; 13+ messages in thread From: Ari Roponen @ 2011-10-06 6:14 UTC (permalink / raw) To: 9679 Sometimes next-error goes to the wrong line when stepping through rgrep matches. Using git bisect and the code below, I found the commit that causes the problem (rev 105677). Reverting that (and before that a commit that depended on it) makes the problem disappear. rev 105685: (grep-regexp-alist): Move dangling comment to the previous rule. rev 105677: progmodes/grep.el (grep-regexp-alist): Calculate column positions I used the following test case to find the problematic commit. It creates a file with two lines matching "mapcar" and then rgreps for it. After that it calls next-error two times. The second call goes into the line starting with " (list 'face ...". ^- here --- code begins ---- (let ((dir (expand-file-name (format "tmp%d" (emacs-pid)) temporary-file-directory))) (ignore-errors (make-directory dir)) (with-temp-file (expand-file-name "test.el" dir) (insert "(defun my-diary-lib--date-to-string (date) (destructuring-bind (month day year) (mapcar 'number-to-string date) (concat day \".\" month \".\" year))) \(defun my-diary-lib--string-to-date (datestr) (destructuring-bind (day month year) (mapcar #'string-to-number (split-string datestr \"\\\\.\")) (list month day year))) \(defun my-diary-lib-add-dayname (datestr) (list 'face 'font-lock-comment-face 'display (concat datestr \" \" (calendar-day-name (my-diary-lib--string-to-date datestr))))) ")) (grep-compute-defaults) (rgrep "mapcar" "test.el" dir) (run-with-idle-timer 0.5 nil #'next-error 1) (run-with-idle-timer 1.0 nil #'next-error 1)) --- code ends --- In GNU Emacs 24.0.90.7 (x86_64-unknown-linux-gnu, GTK+ Version 3.2.0) of 2011-10-06 on arirop Windowing system distributor `Fedora Project', version 11.0.11000000 configured using `configure '--with-x-toolkit=gtk3'' -- Ari Roponen ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line 2011-10-06 6:14 ` bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line Ari Roponen @ 2011-10-06 15:27 ` Juri Linkov 2011-10-07 5:15 ` Ari Roponen [not found] ` <handler.9679.C.13363577086087.notifdonectrl.0@debbugs.gnu.org> 1 sibling, 1 reply; 13+ messages in thread From: Juri Linkov @ 2011-10-06 15:27 UTC (permalink / raw) To: Ari Roponen; +Cc: 9679 > Sometimes next-error goes to the wrong line when stepping through rgrep > matches. > > Using git bisect and the code below, I found the commit that causes the > problem (rev 105677). Reverting that (and before that a commit that > depended on it) makes the problem disappear. > > rev 105685: (grep-regexp-alist): Move dangling comment to the previous rule. > rev 105677: progmodes/grep.el (grep-regexp-alist): Calculate column positions > > I used the following test case to find the problematic commit. It > creates a file with two lines matching "mapcar" and then rgreps for it. > After that it calls next-error two times. The second call goes into the > line starting with " (list 'face ...". > ^- here Thanks for the comprehensive test case. I tried it but the second call goes into the line starting with "(mapcar #'string-to-number ...". ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line 2011-10-06 15:27 ` Juri Linkov @ 2011-10-07 5:15 ` Ari Roponen 2011-10-07 8:41 ` Ari Roponen 2011-10-07 15:20 ` Juri Linkov 0 siblings, 2 replies; 13+ messages in thread From: Ari Roponen @ 2011-10-07 5:15 UTC (permalink / raw) To: Juri Linkov; +Cc: 9679 Juri Linkov <juri@jurta.org> writes: > Thanks for the comprehensive test case. I tried it but the second call > goes into the line starting with "(mapcar #'string-to-number ...". Hi, I compared the behavior between the version with commits reverted and not reverted. In the reverted commits case, typing M-g M-p and M-g M-n goes to the correct line and column, but highlights the whole line. In the non-reverted commits case, typing those commands goes to the wrong line but right column. The highlight seems to start at the end of correct match and end at the cursor position. I found something that may be relevant to the problem. These are the values of *grep*-buffer's variable compilation-locs in both cases: * Reverted commits case: Value: #s(hash-table size 65 test equal weakness value rehash-size 1.5 rehash-threshold 0.8 data ( ("./test.el") (("./test.el" nil) nil (8 (nil 8 #1 #<marker at 251 in test.el> nil . t)) (3 (nil 3 #1 #<marker at 89 in test.el> nil . t))))) * Non-reverted commits case: Value: #s(hash-table size 65 test equal weakness value rehash-size 1.5 rehash-threshold 0.8 data (("Grep started at Fri Oct 7 07") (("Grep started at Fri Oct 7 07" nil) nil (31 (nil 31 #1 nil nil))) ("./test.el") (("./test.el" nil) nil (8 (13 8 #1 #<marker at 258 in test.el> nil) (7 8 #1 #<marker at 387 in test.el> nil . t)) (3 (13 3 #1 #<marker at 96 in test.el> nil) (7 3 #1 #<marker at 90 in test.el> nil . t))) ("Grep finished (matches found) at Fri Oct 7 07") (("Grep finished (matches found) at Fri Oct 7 07" nil) nil (31 (nil 31 #1 nil nil))))) In the non-reverted commits case, The marker pairs seem to delimit the matched text. The first pair (from 258 to 387) starts at the end of correct match and goes too far. The second pair (from 90 to 96) highlight correctly the first match. I noticed that killing the buffer the markers use seems to fix the problem: 1. Run the test case from the original bug report. 2. Kill the "test.el"-buffer 3. Type M-g M-p M-g M-n Now the cursor is at the beginning of the match and the match is highlighted correctly. The value of compilation-locs in *grep*-buffer is also correct: Value: #s(hash-table size 65 test equal weakness value rehash-size 1.5 rehash-threshold 0.8 data (("Grep started at Fri Oct 7 07") (("Grep started at Fri Oct 7 07" nil) nil (56 (nil 56 #1 nil nil))) ("./test.el") (("./test.el" nil) nil (8 (13 8 #1 #<marker at 258 in test.el> nil) (7 8 #1 #<marker at 252 in test.el> nil . t)) (3 (13 3 #1 #<marker at 96 in test.el> nil) (7 3 #1 #<marker at 90 in test.el> nil . t))) ("Grep finished (matches found) at Fri Oct 7 07") (("Grep finished (matches found) at Fri Oct 7 07" nil) nil (56 (nil 56 #1 nil nil))))) I guess the problem has something to do with those markers. -- Ari Roponen ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line 2011-10-07 5:15 ` Ari Roponen @ 2011-10-07 8:41 ` Ari Roponen 2011-10-07 15:20 ` Juri Linkov 1 sibling, 0 replies; 13+ messages in thread From: Ari Roponen @ 2011-10-07 8:41 UTC (permalink / raw) To: Juri Linkov; +Cc: 9679 Ari Roponen <ari.roponen@gmail.com> writes: > I guess the problem has something to do with those markers. I found a way to fix the problem locally without reverting any commits. The function `compilation-next-error-function' contains this: ;; If loc contains no marker, no error in that file has been visited. ;; If the marker is invalid the buffer has been killed. ;; So, recalculate all markers for that file. (unless (and (compilation--loc->marker loc) (marker-buffer (compilation--loc->marker loc)) ;; FIXME-omake: For "omake -P", which automatically recompiles ;; when the file is modified, the line numbers of new output ;; may not be related to line numbers from earlier output When I add "nil" after "and", the problem goes away. -- Ari Roponen ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line 2011-10-07 5:15 ` Ari Roponen 2011-10-07 8:41 ` Ari Roponen @ 2011-10-07 15:20 ` Juri Linkov 2011-10-07 15:51 ` Ari Roponen 1 sibling, 1 reply; 13+ messages in thread From: Juri Linkov @ 2011-10-07 15:20 UTC (permalink / raw) To: Ari Roponen; +Cc: 9679 > ("./test.el") > (("./test.el" nil) > nil > (8 > (13 8 #1 #<marker at 258 in test.el> nil) > (7 8 #1 #<marker at 387 in test.el> nil . t)) > (3 > (13 3 #1 #<marker at 96 in test.el> nil) > (7 3 #1 #<marker at 90 in test.el> nil . t))) This is odd, I have in the non-reverted commits case after visiting the second match: #s(hash-table size 65 test equal weakness value rehash-size 1.5 rehash-threshold 0.8 data (("./test.el") (("./test.el" nil) nil (8 (13 8 #1 #<marker at 258 in test.el> nil) (7 8 #1 #<marker at 252 in test.el> nil . t)) (3 (13 3 #1 #<marker at 96 in test.el> nil) (7 3 #1 #<marker at 90 in test.el> nil . t))))) So unfortunately I can't reproduce this bug. I suspect it may be related to screen columns because somehow `compilation-move-to-column' in `compilation-next-error-function' goes to the wrong line and column, i.e. to the position 387. What values of `compilation-error-screen-columns' and `grep-error-screen-columns' do you have in the *grep* buffer? I have both of them equal to `nil'. Could you please get the values of arguments that `compilation-move-to-column' receives here by debugging or adding (message "...") and seeing what values it prints in the *Message* buffer? ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line 2011-10-07 15:20 ` Juri Linkov @ 2011-10-07 15:51 ` Ari Roponen 2011-10-07 16:36 ` Juri Linkov 0 siblings, 1 reply; 13+ messages in thread From: Ari Roponen @ 2011-10-07 15:51 UTC (permalink / raw) To: Juri Linkov; +Cc: 9679 Juri Linkov <juri@jurta.org> writes: > I suspect it may be related to screen columns because somehow > `compilation-move-to-column' in `compilation-next-error-function' > goes to the wrong line and column, i.e. to the position 387. > What values of `compilation-error-screen-columns' and > `grep-error-screen-columns' do you have in the *grep* buffer? > I have both of them equal to `nil'. I run the test case with "emacs -Q -l bug.el", so the variables use their default values: compilation-error-screen-columns is a variable defined in `compile.el'. Its value is nil Original value was t Local in buffer *grep*; global value is t grep-error-screen-columns is a variable defined in `grep.el'. Its value is nil > > Could you please get the values of arguments that > `compilation-move-to-column' receives here by debugging > or adding (message "...") and seeing what values it prints > in the *Message* buffer? (defun compilation-move-to-column (col screen) "Go to column COL on the current line. If SCREEN is non-nil, columns are screen columns, otherwise, they are just char-counts." (message "col: %S, screen %S" col screen) ; Display parameters. (if screen (move-to-column (max col 0)) (goto-char (min (+ (line-beginning-position) col) (line-end-position))))) => Grep finished (matches found) col: 13, screen nil col: 7, screen nil col: 13, screen t col: 7, screen t I get the same values also with the simpler "fix" that I mentioned in the other post. -- Ari Roponen ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line 2011-10-07 15:51 ` Ari Roponen @ 2011-10-07 16:36 ` Juri Linkov 2011-10-07 17:48 ` Ari Roponen 0 siblings, 1 reply; 13+ messages in thread From: Juri Linkov @ 2011-10-07 16:36 UTC (permalink / raw) To: Ari Roponen; +Cc: 9679 > col: 13, screen nil > col: 7, screen nil > col: 13, screen t > col: 7, screen t Something is completely broken in your test case, because I get: col: 13, screen nil col: 7, screen nil col: 13, screen nil col: 7, screen nil i.e. `nil' is what's expected for the second match. I don't understand how `t' is possible. It looks like `compilation-next-error-function' is not in `grep-mode' at that time. Could you please add the following `message' to `compilation-next-error-function': (defun compilation-next-error-function (n &optional reset) "Advance to the next error message and visit the file where the error was. This is the value of `next-error-function' in Compilation buffers." (interactive "p") (when reset (setq compilation-current-error nil)) (message "mode: %S, buffer: %S" major-mode (current-buffer)) ... I get the following output: mode: grep-mode, buffer: #<buffer *grep*> col: 13, screen nil col: 7, screen nil col: 13, screen nil col: 7, screen nil mode: grep-mode, buffer: #<buffer *grep*> ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line 2011-10-07 16:36 ` Juri Linkov @ 2011-10-07 17:48 ` Ari Roponen 2011-10-08 16:30 ` Juri Linkov 0 siblings, 1 reply; 13+ messages in thread From: Ari Roponen @ 2011-10-07 17:48 UTC (permalink / raw) To: Juri Linkov; +Cc: 9679 Juri Linkov <juri@jurta.org> writes: > I don't understand how `t' is possible. It looks like > `compilation-next-error-function' is not in `grep-mode' at that time. > > Could you please add the following `message' to `compilation-next-error-function': > > (defun compilation-next-error-function (n &optional reset) > "Advance to the next error message and visit the file where the error was. > This is the value of `next-error-function' in Compilation buffers." > (interactive "p") > (when reset > (setq compilation-current-error nil)) > (message "mode: %S, buffer: %S" major-mode (current-buffer)) > ... > > I get the following output: > > mode: grep-mode, buffer: #<buffer *grep*> > col: 13, screen nil > col: 7, screen nil > col: 13, screen nil > col: 7, screen nil > mode: grep-mode, buffer: #<buffer *grep*> Grep finished (matches found) mode: grep-mode, buffer: #<buffer *grep*> col: 13, screen nil col: 7, screen nil col: 13, screen t col: 7, screen t mode: grep-mode, buffer: #<buffer *grep*> -- Ari Roponen ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line 2011-10-07 17:48 ` Ari Roponen @ 2011-10-08 16:30 ` Juri Linkov 2011-10-09 5:38 ` Ari Roponen 0 siblings, 1 reply; 13+ messages in thread From: Juri Linkov @ 2011-10-08 16:30 UTC (permalink / raw) To: Ari Roponen; +Cc: 9679 >> mode: grep-mode, buffer: #<buffer *grep*> >> col: 13, screen nil >> col: 7, screen nil >> col: 13, screen nil >> col: 7, screen nil >> mode: grep-mode, buffer: #<buffer *grep*> > > Grep finished (matches found) > mode: grep-mode, buffer: #<buffer *grep*> > col: 13, screen nil > col: 7, screen nil > col: 13, screen t > col: 7, screen t > mode: grep-mode, buffer: #<buffer *grep*> Thanks, I can now reproduce this bug. Its occurrence depends on the frame's size. When frame's dimensions are 80x43 with 43 lines or more, so the first grep hit is visible in the *grep* buffer, then font-lock kicks in, calculates absolute column positions, after that `compilation-next-error-function' calculates relative column positions with markers for BOTH grep hits, where the final position of the second grep hit is correct. But when frame's dimensions are 80x42 with 42 lines or less, so the first grep hit is not visible in the *grep* buffer, then fontification doesn't start, and when you call `next-error', `compilation-next-error-function' calculates column positions with markers only for the FIRST grep hit, because the second grep hit is not fontified yet in the *grep* buffer. The second grep hit gets processes later with `jit-lock-fontify-now' that calls `compilation-internal-error-properties' and uses `compilation-move-to-column' to calculate columns. The problem is that `compilation-internal-error-properties' calculates column marker positions differently than `compilation-next-error-function'. Its markers have wrong positions as a result. `compilation-next-error-function' is always called in the *grep* buffer where it gets the buffer-local value of `compilation-error-screen-columns' and keeps its value locally. After that it visits the source buffer and correctly calculates column positions using `compilation-move-to-column'. `compilation-internal-error-properties' tries to do the same, but uses a different algorithm. It gets the global value of `compilation-error-screen-columns' instead of buffer-local from the *grep* buffer. Another difference that contributes to the wrong position is using: (beginning-of-line (if end-line (- line end-line -1) (- loc marker-line -1))) Contrary to this, `compilation-next-error-function' doesn't call `beginning-of-line' after `compilation-move-to-column' moves to the correct column. Unfortunately, I can't fix this, because I don't understand why there are two implementations of column position calculations. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line 2011-10-08 16:30 ` Juri Linkov @ 2011-10-09 5:38 ` Ari Roponen 2012-05-06 6:23 ` Ari Roponen 0 siblings, 1 reply; 13+ messages in thread From: Ari Roponen @ 2011-10-09 5:38 UTC (permalink / raw) To: Juri Linkov; +Cc: 9679 Juri Linkov <juri@jurta.org> writes: > Thanks, I can now reproduce this bug. Thank you for looking at this problem. > > Unfortunately, I can't fix this, because I don't understand > why there are two implementations of column position calculations. No worries. As they say, a known bug is better than an unknown regression. :-) -- Ari Roponen ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line 2011-10-09 5:38 ` Ari Roponen @ 2012-05-06 6:23 ` Ari Roponen 0 siblings, 0 replies; 13+ messages in thread From: Ari Roponen @ 2012-05-06 6:23 UTC (permalink / raw) To: Juri Linkov; +Cc: 9679 Ari Roponen <ari.roponen@gmail.com> writes: > Juri Linkov <juri@jurta.org> writes: > >> Thanks, I can now reproduce this bug. > > Thank you for looking at this problem. > >> >> Unfortunately, I can't fix this, because I don't understand >> why there are two implementations of column position calculations. > > No worries. As they say, a known bug is better than an unknown > regression. :-) Hi, I can't reproduce this bug any more with todays checkout. It seems that this commit fixed the bug: revno: 108138 fixes bug(s): http://debbugs.gnu.org/11382 author: Troels Nielsen <bn.troels@gmail.com> committer: Chong Yidong <cyd@gnu.org> branch nick: trunk timestamp: Sun 2012-05-06 12:52:58 +0800 message: Fix match highlighting in compilation buffers. * progmodes/compile.el (compilation-internal-error-properties): Calculate start position correctly when end-col is set but end-line is not. -- Ari Roponen ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <handler.9679.C.13363577086087.notifdonectrl.0@debbugs.gnu.org>]
* bug#9679: acknowledged by developer (close 9679) [not found] ` <handler.9679.C.13363577086087.notifdonectrl.0@debbugs.gnu.org> @ 2012-05-13 6:39 ` Ari Roponen 2012-05-13 9:18 ` Chong Yidong 0 siblings, 1 reply; 13+ messages in thread From: Ari Roponen @ 2012-05-13 6:39 UTC (permalink / raw) To: 9679 This bug ("24.0.90; After rgrep, next-error goes to the wrong line") was fixed in trunk, but it still happens in emacs-24 branch. Maybe the patch from the following revision could be backported/applied into the emacs-24 branch. I just tested it locally, and it indeed fixed the problem there, too. revno: 108138 fixes bug(s): http://debbugs.gnu.org/11382 author: Troels Nielsen <bn.troels@gmail.com> committer: Chong Yidong <cyd@gnu.org> branch nick: trunk timestamp: Sun 2012-05-06 12:52:58 +0800 message: Fix match highlighting in compilation buffers. * progmodes/compile.el (compilation-internal-error-properties): Calculate start position correctly when end-col is set but end-line is not. -- Ari Roponen ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#9679: acknowledged by developer (close 9679) 2012-05-13 6:39 ` bug#9679: acknowledged by developer (close 9679) Ari Roponen @ 2012-05-13 9:18 ` Chong Yidong 0 siblings, 0 replies; 13+ messages in thread From: Chong Yidong @ 2012-05-13 9:18 UTC (permalink / raw) To: Ari Roponen; +Cc: 9679 Ari Roponen <ari.roponen@gmail.com> writes: > This bug ("24.0.90; After rgrep, next-error goes to the wrong line") was > fixed in trunk, but it still happens in emacs-24 branch. > > Maybe the patch from the following revision could be backported/applied > into the emacs-24 branch. I just tested it locally, and it indeed fixed > the problem there, too. Ah, I didn't realize this was a regression against Emacs 23. I've backported the fix the emacs-24 branch; thanks for the heads-up. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-05-13 9:18 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <87r4uwybeh.fsf@gnu.org> 2011-10-06 6:14 ` bug#9679: 24.0.90; After rgrep, next-error goes to the wrong line Ari Roponen 2011-10-06 15:27 ` Juri Linkov 2011-10-07 5:15 ` Ari Roponen 2011-10-07 8:41 ` Ari Roponen 2011-10-07 15:20 ` Juri Linkov 2011-10-07 15:51 ` Ari Roponen 2011-10-07 16:36 ` Juri Linkov 2011-10-07 17:48 ` Ari Roponen 2011-10-08 16:30 ` Juri Linkov 2011-10-09 5:38 ` Ari Roponen 2012-05-06 6:23 ` Ari Roponen [not found] ` <handler.9679.C.13363577086087.notifdonectrl.0@debbugs.gnu.org> 2012-05-13 6:39 ` bug#9679: acknowledged by developer (close 9679) Ari Roponen 2012-05-13 9:18 ` Chong Yidong
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).