Am 12/18/2013 15:53, schrieb Nguyễn Thái Ngọc Duy:
> The code that's not converted to use parse_options() often does
>
> if (!starts_with(arg, "foo=")) {
> value = atoi(arg + 4);
> }
>
> This patch removes those magic numbers with skip_prefix()
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> builtin/fetch-pack.c | 13 +++++----
> builtin/index-pack.c | 17 +++++------
> builtin/ls-remote.c | 9 +++---
> builtin/mailinfo.c | 5 ++--
> builtin/reflog.c | 9 +++---
> builtin/rev-parse.c | 41 +++++++++++++-------------
> builtin/send-pack.c | 18 ++++++------
> builtin/unpack-objects.c | 5 ++--
> builtin/update-ref.c | 21 +++++++-------
> daemon.c | 75 ++++++++++++++++++++++++------------------------
> diff.c | 49 +++++++++++++++----------------
> git.c | 13 +++++----
> merge-recursive.c | 13 +++++----
> revision.c | 60 +++++++++++++++++++-------------------
> upload-pack.c | 5 ++--
> 15 files changed, 182 insertions(+), 171 deletions(-)
>
> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
> index 8b8978a2..2df1423 100644
> --- a/builtin/fetch-pack.c
> +++ b/builtin/fetch-pack.c
> @@ -47,13 +47,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
>
> for (i = 1; i < argc && *argv[i] == '-'; i++) {
> const char *arg = argv[i];
> + const char *optarg;
>
> - if (starts_with(arg, "--upload-pack=")) {
> - args.uploadpack = arg + 14;
> + if ((optarg = skip_prefix(arg, "--upload-pack=")) != NULL) {
> + args.uploadpack = optarg;
Quite frankly, I do not think this is an improvement. The old code is
*MUCH* easier to understand because "starts_with" is clearly a predicate
that is either true or false, but the code with "skip_prefix" is much
heavier on the eye with its extra level of parenthesis. That it removes a
hard-coded constant does not count much IMHO because it is very clear
where the value comes from.
-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html