Updated patch attached with improved docs, etc.

On Wed, Feb 12, 2025 at 2:55 PM Ship Mints <shipmints@gmail.com> wrote:

> 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
>