GNU bug report logs - #76234
[PATCH] Prefix argument implies recursive for project-remember/forget-under

Previous Next

Full log

Message #8 received at 76234 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ship Mints <shipmints <at> gmail.com>
Cc: 76234 <at> debbugs.gnu.org
Subject: Re: bug#76234: [PATCH] Prefix argument implies recursive for
 project-remember/forget-under
Date: Wed, 12 Feb 2025 21:46:15 +0200

> From: Ship Mints <shipmints <at> gmail.com>
> Date: Wed, 12 Feb 2025 12:36:58 -0500
> 
> Specifying a universal prefix argument when invoking 'project-remember-projects-under' or
> 'project-forget-projects-under' now imply recursive. This makes recursive interactive use easier.

Thanks, please wait for Dmitry to review.

> +*** 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.

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

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

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.