[Top][All Lists]
[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
From: |
Bruno Haible |
Subject: |
[bug-patch] Re: [PATCH] Addition of --rm-command to specify the remove command |
Date: |
Sat, 20 Jun 2009 10:07:53 +0200 |
User-agent: |
KMail/1.9.9 |
Hi,
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?
Bruno