[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug-patch] Re: [PATCH] Addition of --rm-command to specify the remove c
[bug-patch] Re: [PATCH] Addition of --rm-command to specify the remove command
Sat, 20 Jun 2009 10:07:53 +0200
I'm not the 'patch' maintainer, but can't avoid to comment.
Daniel Gutson wrote:
> + strcpy (cmd, rm_command);
> + strcat (cmd, " ");
> + strcat (cmd, to);
> + if (systemic (cmd) == -1)
This will remove the wrong files when 'to' contains spaces or other special
characters. It is mandatory to use quote_system_arg here.
> the attached patch adds the --rm-command command line option, used to
> specify a different 'rm' command, useful to use in source directories
> controlled by
> revision control systems (such as svn).
Your patch uses a completely different approach than the one already in use.
Namely, the file src/util.c contains two function version_controller and
version_get that return command strings. Obviously these functions suffer
from bitrot, since they don't know about cvs, svn, git, hg.
Also, I think that asking the user to think about a specific --rm-command is
too specific. Let the user decide whether he wants to use a simple remove or
a version-controlled remove. The 'patch' program can then detect by itself
which versioning system is in use. Maybe the '-g' option can be used for
this purpose? Or maybe the --version-control option needs to be extended?