Re: Add M-x occur to the menu-bar

[Ted Zlatanov]

On 07 Apr 2004, address@hidden wrote:

>>        (select-window outwin)
>>        (goto-char (point-max))))
>>      ;; Make it so the next C-x ` will use this buffer.
>> -    (setq compilation-last-buffer outbuf)))
>> +    (setq compilation-last-buffer outbuf)
>        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> duplicate with the one below.

Fixed.

+    (setq next-error-last-buffer outbuf)
+    (setq compilation-last-buffer outbuf)
+    (with-current-buffer outbuf
+      ;; note that compilation-next-error-function is for interfacing
+      ;; with the next-error function in simple.el, and it's only
+      ;; coincidentally named similarly to compilation-next-error
+      (setq next-error-function 'compilation-next-error-function))))
> 
> Shouldn't we get rid of compilation-last-buffer?

I think that would be OK.  Any dissenters? :)

> And (setq next-error-function 'compilation-next-error-function)
> should be in compilation-setup.

Done.
>> +;;; test if a buffer is a compilation buffer, using 
>> compilation-buffer-internal-p
>>  (defsubst compilation-buffer-p (buffer)
>> -  (local-variable-p 'compilation-locs buffer))
>> +  "Test if BUFFER is a compilation buffer."
>> +  (with-current-buffer buffer
>> +    (compilation-buffer-internal-p)))
>> +
>> +;;; test if a buffer is a compilation buffer, assuming we're in the buffer
>> +(defsubst compilation-buffer-internal-p ()
>> +  "Test if inside a compilation buffer."
>> +  (local-variable-p 'compilation-locs (current-buffer)))
> 
> Huh?  This looks like a complicated way to do thing.  Just make
> the BUFFER argument optional instead (local-variable-p will
> DTRT).

Done, thanks.

> 
>> +(defun compilation-next-error-function (argp &optional reset)
> 
> I'd use a special value like `first' for ARGP instead of adding
> a RESET argument.  BTW, what does `argp' stand for and wouldn't
> you have to update the body since it still presumably refers to the old
> N argument name instead?

With RESET, the function acts as if first-error was called and
then next-error N was called.  Good catch on ARGP, that was my
mistake.

> 
>> +          (setq next-error-function 'occur-next-error))
> 
> This should be placed in occur-mode.

It's in replace.el, where occur-mode is defined.

>> +(defvar next-error-last-buffer nil
>> +(defvar next-error-function nil
>> +(make-variable-buffer-local 'next-error-function)
>> +(defsubst next-error-buffer-p (buffer &optional extra-test)
>> +(defun next-error-find-buffer (&optional other-buffer extra-test)
>> +(defun next-error (argp &optional reset)
>> +(defalias 'goto-next-locus 'next-error)
>> +(defalias 'next-match 'next-error)
>> +(define-key ctl-x-map "`" 'next-error)
>> +(defun previous-error (n)
>> +(defun first-error (n)
> 
> Should we also move next-error-no-select and previous-error-no-select ?

Done.

Thank you very much for the thorough review.  All the changes are
in the latest patch I just posted.

Ted