Re: [PATCH 02/12] Convert starts_with() to skip_prefix() for option parsing
- From
Johannes Sixt <j.sixt@viscovery.net>
- Date
- Dec 20, 2013, 06:51 UTC
- Message-ID
- <52B3E8D4.1030805@viscovery.net>
- In-Reply-To
- <1387378437-20646-3-git-send-email-pclouds@gmail.com>
Am 12/18/2013 15:53, schrieb Nguyễn Thái Ngọc Duy:
Show 41 quoted lines
> 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@gmail.com>
> ---
> 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