bug-patch
[Top][All Lists]
Advanced

[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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]