On Wed, Feb 12, 2025 at 2:46 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Ship Mints <shipmints@gmail.com>
> > Date: Wed, 12 Feb 2025 12:36:58 -0500
> >> +*** Prefix argument implies recursive for remember/forget under.
>
> +Specifying a universal prefix argument when invoking
> > +'project-remember-projects-under' or 'project-forget-projects-under' now
> > +imply recursive.
>
> The natural way of describing this kind of changes is the other way
> around: starting with the command names, not with the prefix argument.
> Like this:
>
>   *** 'project-remember/forget-projects-under' can now work recursively.
>   The commands 'project-remember-projects-under' and
>   'project-forget-projects-under' now find projects in subdirectories,
>   recursively, if invoked with a prefix argument.
>

I'll improve the language. They already did work recursively, but only when
invoked programmatically.

>  (defun project-remember-projects-under (dir &optional recursive)
> >    "Index all projects below a directory DIR.
> > -If RECURSIVE is non-nil, recurse into all subdirectories to find
> > -more projects.  After finishing, a message is printed summarizing
> > -the progress.  The function returns the number of detected
> > -projects."
> > +If RECURSIVE is non-nil, or with a \\[universal-argument] prefix when
> > +called, recurse into all subdirectories to find more projects.  After
> > +finishing, a message is printed summarizing the progress.  The function
> > +returns the number of detected projects."
>
> This doc string "needs work", and uses passive voice unnecessarily.
> Here's a rewording that should be closer to our conventions:
>
>     Remember (i.e., index) projects in directory DIR.
>   Interactively, prompt for DIR.
>   Optional argument RECURSIVE, if non -nil (interactively, the
>   prefix argument) means recurse into subdirectories of DIR to find
>   more projects.
>   Display a message at the end summarizing what was indexed.
>   Return the number of indexed projects.
>

Will do. I'd merely added the interactive detail to what was already there
vs. a wholesale rewrite.

>  (defun project-forget-projects-under (dir &optional recursive)
> >    "Forget all known projects below a directory DIR.
> > -If RECURSIVE is non-nil, recurse into all subdirectories to
> > -remove all known projects.  After finishing, a message is printed
> > -summarizing the progress.  The function returns the number of
> > -forgotten projects."
> > +If RECURSIVE is non-nil, or with a \\[universal-argument] prefix when
> > +called, recurse into all subdirectories to remove all known projects.
> > +After finishing, a message is printed summarizing the progress.  The
> > +function returns the number of forgotten projects."
>
> Similarly here.
>

Ditto.

>    (interactive "DDirectory: \nP")
> >    (let ((count 0))
> > -    (if recursive
> > +    (if (or recursive (consp current-prefix-arg))
>
> Any reason why you use consp here (and in the other function)?  AFAIU,
> the interactive form will automatically take care of setting the
> second argument to the value of the raw prefix arg, so why would you
> need to look at the value of current-prefix-arg and discard it if it
> is not a cons cell?
>

Habit. I'll change it to merely check for non-nil.

Thank you for the thoughtful feedback.

-Stephane