Re: how-many/count-matches for non-interactive use

[Richard Stallman]

The code in allout.el uses interactive-p in a rather confused way.

The first use of interactive-p in allout is clearly a bug.
That command would fail in a macro.  The right way to right
that code is to read the argument inside the interactive spec.

Another use does call-interactively only if (interactive-p) is true.
That also is clearly wrong.

Many calls to interactive-p appear in functions that are not commands.
The only way interactive-p can return non-nil is when these functions
are called from commands run from macros.  It doesn't seem right for
allout commands to move the cursor differently when they are run from
macros.  So I am pretty sure these are bugs.

I think this is the right way to fix these bugs, but I don't use
allout.el.  Could someone who uses it please check this?


*** allout.el   04 Sep 2004 13:19:28 -0400      1.50
--- allout.el   23 Oct 2004 13:42:44 -0400      
***************
*** 954,973 ****
  \(require 'allout)
  \(allout-init t)"
  
!   (interactive)
!   (if (interactive-p)
!       (progn
!       (setq mode
!             (completing-read
!              (concat "Select outline auto setup mode "
!                      "(empty for report, ? for options) ")
!              '(("nil")("full")("activate")("deactivate")
!                ("ask") ("report") (""))
!              nil
!              t))
!       (if (string= mode "")
!           (setq mode 'report)
!         (setq mode (intern-soft mode)))))
    (let
        ;; convenience aliases, for consistent ref to respective vars:
        ((hook 'allout-find-file-hook)
--- 954,969 ----
  \(require 'allout)
  \(allout-init t)"
  
!   (interactive
!    (let ((m (completing-read
!            (concat "Select outline auto setup mode "
!                    "(empty for report, ? for options) ")
!            '(("nil")("full")("activate")("deactivate")
!              ("ask") ("report") (""))
!            nil
!            t)))
!      (if (string= m "") 'report
!        (intern-soft m))))
    (let
        ;; convenience aliases, for consistent ref to respective vars:
        ((hook 'allout-find-file-hook)
***************
*** 1904,1917 ****
                     depth)
            (goto-char last-good)
            nil))
!     (if (interactive-p) (allout-end-of-prefix))))
  ;;;_   > allout-ascend ()
  (defun allout-ascend ()
    "Ascend one level, returning t if successful, nil if not."
    (prog1
        (if (allout-beginning-of-level)
          (allout-previous-heading))
!     (if (interactive-p) (allout-end-of-prefix))))
  ;;;_   > allout-descend-to-depth (depth)
  (defun allout-descend-to-depth (depth)
    "Descend to depth DEPTH within current topic.
--- 1900,1913 ----
                     depth)
            (goto-char last-good)
            nil))
!     (allout-end-of-prefix)))
  ;;;_   > allout-ascend ()
  (defun allout-ascend ()
    "Ascend one level, returning t if successful, nil if not."
    (prog1
        (if (allout-beginning-of-level)
          (allout-previous-heading))
!     (allout-end-of-prefix)))
  ;;;_   > allout-descend-to-depth (depth)
  (defun allout-descend-to-depth (depth)
    "Descend to depth DEPTH within current topic.
***************
*** 1931,1943 ****
        nil))
    )
  ;;;_   > allout-up-current-level (arg &optional dont-complain)
! (defun allout-up-current-level (arg &optional dont-complain)
    "Move out ARG levels from current visible topic.
  
  Positions on heading line of containing topic.  Error if unable to
  ascend that far, or nil if unable to ascend but optional arg
  DONT-COMPLAIN is non-nil."
!   (interactive "p")
    (allout-back-to-current-heading)
    (let ((present-level (allout-recent-depth))
        (last-good (point))
--- 1927,1939 ----
        nil))
    )
  ;;;_   > allout-up-current-level (arg &optional dont-complain)
! (defun allout-up-current-level (arg &optional dont-complain interactive)
    "Move out ARG levels from current visible topic.
  
  Positions on heading line of containing topic.  Error if unable to
  ascend that far, or nil if unable to ascend but optional arg
  DONT-COMPLAIN is non-nil."
!   (interactive "p\ni\np")
    (allout-back-to-current-heading)
    (let ((present-level (allout-recent-depth))
        (last-good (point))
***************
*** 1958,1969 ****
      (if (or failed
            (> arg 0))
        (progn (goto-char last-good)
!              (if (interactive-p) (allout-end-of-prefix))
               (if (not dont-complain)
                   (error "Can't ascend past outermost level")
!                (if (interactive-p) (allout-end-of-prefix))
                 nil))
!       (if (interactive-p) (allout-end-of-prefix))
        allout-recent-prefix-beginning)))
  
  ;;;_  - Linear
--- 1954,1965 ----
      (if (or failed
            (> arg 0))
        (progn (goto-char last-good)
!              (if interactive (allout-end-of-prefix))
               (if (not dont-complain)
                   (error "Can't ascend past outermost level")
!                (if interactive (allout-end-of-prefix))
                 nil))
!       (if interactive (allout-end-of-prefix))
        allout-recent-prefix-beginning)))
  
  ;;;_  - Linear
***************
*** 2029,2035 ****
    (let ((depth (allout-depth)))
      (while (allout-previous-sibling depth nil))
      (prog1 (allout-recent-depth)
!       (if (interactive-p) (allout-end-of-prefix)))))
  ;;;_   > allout-next-visible-heading (arg)
  (defun allout-next-visible-heading (arg)
    "Move to the next ARG'th visible heading line, backward if arg is negative.
--- 2025,2031 ----
    (let ((depth (allout-depth)))
      (while (allout-previous-sibling depth nil))
      (prog1 (allout-recent-depth)
!       (allout-end-of-prefix))))
  ;;;_   > allout-next-visible-heading (arg)
  (defun allout-next-visible-heading (arg)
    "Move to the next ARG'th visible heading line, backward if arg is negative.
***************
*** 2067,2079 ****
    (interactive "p")
    (allout-next-visible-heading (- arg)))
  ;;;_   > allout-forward-current-level (arg)
! (defun allout-forward-current-level (arg)
    "Position point at the next heading of the same level.
  
  Takes optional repeat-count, goes backward if count is negative.
  
  Returns resulting position, else nil if none found."
!   (interactive "p")
    (let ((start-depth (allout-current-depth))
        (start-point (point))
        (start-arg arg)
--- 2063,2075 ----
    (interactive "p")
    (allout-next-visible-heading (- arg)))
  ;;;_   > allout-forward-current-level (arg)
! (defun allout-forward-current-level (arg &optional interactive)
    "Position point at the next heading of the same level.
  
  Takes optional repeat-count, goes backward if count is negative.
  
  Returns resulting position, else nil if none found."
!   (interactive "p\np")
    (let ((start-depth (allout-current-depth))
        (start-point (point))
        (start-arg arg)
***************
*** 2101,2107 ****
                  (= (allout-recent-depth) start-depth)))
        allout-recent-prefix-beginning
        (goto-char last-good)
!       (if (not (interactive-p))
          nil
        (allout-end-of-prefix)
        (error "Hit %s level %d topic, traversed %d of %d requested"
--- 2097,2103 ----
                  (= (allout-recent-depth) start-depth)))
        allout-recent-prefix-beginning
        (goto-char last-good)
!       (if (not interactive)
          nil
        (allout-end-of-prefix)
        (error "Hit %s level %d topic, traversed %d of %d requested"
***************
*** 2110,2119 ****
               (- (abs start-arg) arg)
               (abs start-arg))))))
  ;;;_   > allout-backward-current-level (arg)
! (defun allout-backward-current-level (arg)
    "Inverse of `allout-forward-current-level'."
!   (interactive "p")
!   (if (interactive-p)
        (let ((current-prefix-arg (* -1 arg)))
        (call-interactively 'allout-forward-current-level))
      (allout-forward-current-level (* -1 arg))))
--- 2106,2115 ----
               (- (abs start-arg) arg)
               (abs start-arg))))))
  ;;;_   > allout-backward-current-level (arg)
! (defun allout-backward-current-level (arg &optional interactive)
    "Inverse of `allout-forward-current-level'."
!   (interactive "p\np")
!   (if interactive
        (let ((current-prefix-arg (* -1 arg)))
        (call-interactively 'allout-forward-current-level))
      (allout-forward-current-level (* -1 arg))))