Hi Ingo,
Thanks for taking the time to review this.
On Sun, Jun 25, 2017 at 03:12:26PM +0200, Ingo Schwarze wrote:
> Job Snijders wrote on Sun, Jun 25, 2017 at 02:06:16PM +0200:
>
> > This patch adds a '-v' option to rm(1) for more verbose output.
>
> Do not add new options to standard utilities, unless you can show that
> they are unusually useful in practice *and* practically every other
> system out there has them, with a compatible user interface across
> practically all systems.
I checked a number of systems: busybox, HP-UX, and SunOS don't have -v.
FreeBSD, NetBSD, DragonFlyBSD, Slackware, Minix, Debian, CentOS, Suse,
and Darwin do have a -v option to report which files were deleted.
> Adding -v to rm(1) probably wouldn't make the very high bar against
> adding non-standard options even if almost everybody else had it
> (which i didn't check, to not waste time) because it's completely
> useless.
I appreciate the bar is high, and I doubted whether I should send the
patch at all. However, without some proposed code on the table we would
not be able to have this conversation.
> If you are really unsure, study the output of
>
> $ find *
>
> first, before typing
>
> $ rm -rf *
>
> No non-standard option is needed at all for this.
The 'find *' will show you what existed at the moment of executing the
'find' command, where on the other hand 'rm -rfv *' gives a report of
which files actually were succesfully deleted through the execution of
the 'rm' command.
> It's also strongly in violation of the UNIX philosophy (each utility,
> do one thing, and do it well). rm(1) removes files, and does so well.
> rm(1) does *not* trespass on ls(1) and find(1) territory to list
> files.
I'm not sure I agree with you on this point. To me ls(1) and rm(1) have
different uses, rm(1) being able to tell which files it deleted, is not
at all the same as requesting a listing of files.
> Besides, they way your proposed rm(1) extension lists files is not
> doing it well at all. The output is awful.
Yes, the output could be simplified, below a version of the patch to
align with how freebsd and netbsd do it:
$ touch f; mkdir d; touch d/f
$ rm -rfv *
d/f
d
f
Sticking to the standards is a strong argument, and I understand that
changes to core utilities like rm must be thoroughly vetted. Please
don't take offense in me attempting to propose a change.
Kind regards,
Job
diff --git bin/rm/rm.1 bin/rm/rm.1
index 5c8aefaab7d..edc68b60001 100644
--- bin/rm/rm.1
+++ bin/rm/rm.1
@@ -95,6 +95,8 @@ that directory is skipped.
.It Fl r
Equivalent to
.Fl R .
+.It Fl v
+Explain what is being done.
.El
.Pp
The
@@ -148,7 +150,7 @@ utility is compliant with the
specification.
.Pp
The flags
-.Op Fl dP
+.Op Fl dPv
are extensions to that specification.
.Sh HISTORY
An
diff --git bin/rm/rm.c bin/rm/rm.c
index 3242ef5f410..fc0904d0325 100644
--- bin/rm/rm.c
+++ bin/rm/rm.c
@@ -50,7 +50,7 @@
extern char *__progname;
-int dflag, eval, fflag, iflag, Pflag, stdin_ok;
+int dflag, eval, fflag, iflag, Pflag, vflag, stdin_ok;
int check(char *, char *, struct stat *);
void checkdot(char **);
@@ -73,7 +73,7 @@ main(int argc, char *argv[])
int ch, rflag;
Pflag = rflag = 0;
- while ((ch = getopt(argc, argv, "dfiPRr")) != -1)
+ while ((ch = getopt(argc, argv, "dfiPRrv")) != -1)
switch(ch) {
case 'd':
dflag = 1;
@@ -93,6 +93,9 @@ main(int argc, char *argv[])
case 'r': /* Compatibility. */
rflag = 1;
break;
+ case 'v':
+ vflag = 1;
+ break;
default:
usage();
}
@@ -201,8 +204,11 @@ rm_tree(char **argv)
case FTS_DP:
case FTS_DNR:
if (!rmdir(p->fts_accpath) ||
- (fflag && errno == ENOENT))
+ (fflag && errno == ENOENT)) {
+ if (vflag)
+ (void)fprintf(stdout, "%s\n",
p->fts_path);
continue;
+ }
break;
case FTS_F:
@@ -213,8 +219,11 @@ rm_tree(char **argv)
/* FALLTHROUGH */
default:
if (!unlink(p->fts_accpath) ||
- (fflag && errno == ENOENT))
+ (fflag && errno == ENOENT)) {
+ if (vflag)
+ (void)fprintf(stdout, "%s\n",
p->fts_path);
continue;
+ }
}
warn("%s", p->fts_path);
eval = 1;
@@ -262,7 +271,8 @@ rm_file(char **argv)
if (rval && (!fflag || errno != ENOENT)) {
warn("%s", f);
eval = 1;
- }
+ } else if (vflag)
+ (void)fprintf(stdout, "%s\n", f);
}
}
@@ -430,6 +440,6 @@ skip:
void
usage(void)
{
- (void)fprintf(stderr, "usage: %s [-dfiPRr] file ...\n", __progname);
+ (void)fprintf(stderr, "usage: %s [-dfiPRrv] file ...\n", __progname);
exit(1);
}
