git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: Nguyễn Thái Ngọc DuyNext: Jeff King
Message 6 of 31 in “Hard coded string length cleanup”
  1. 00/12 Hard coded string length cleanupNguyễn Thái Ngọc Duy, Dec 18, 2013
  2. 01/12 Make starts_with() a wrapper of skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  3. Junio C HamanoDec 18, 2013
  4. Junio C HamanoDec 18, 2013
  5. 02/12 Convert starts_with() to skip_prefix() for option parsingNguyễn Thái Ngọc Duy, Dec 18, 2013
  6. Johannes SixtDec 20, 2013
  7. Jeff KingDec 20, 2013
  8. Christian CouderDec 20, 2013
  9. René ScharfeDec 20, 2013
  10. Junio C HamanoDec 20, 2013
  11. Duy NguyenDec 21, 2013
  12. Junio C HamanoDec 26, 2013
  13. Jeff KingDec 28, 2013
  14. 03/12 Add and use skip_prefix_defval()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  15. Kent R. SpillnerDec 18, 2013
  16. Junio C HamanoDec 18, 2013
  17. 04/12 Replace some use of starts_with() with skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  18. 05/12 Convert a lot of starts_with() to skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  19. 06/12 fetch.c: replace some use of starts_with() with skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  20. 07/12 connect.c: replace some use of starts_with() with skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  21. 08/12 refs.c: replace some use of starts_with() with skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  22. 09/12 diff.c: reduce code duplication in --stat-xxx parsingNguyễn Thái Ngọc Duy, Dec 18, 2013
  23. 10/12 environment.c: replace starts_with() in strip_namespace() with skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  24. 11/12 diff.c: convert diff_scoreopt_parse to use skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  25. 12/12 refs.c: use skip_prefix() in prune_ref()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  26. Junio C HamanoDec 18, 2013
  27. René ScharfeDec 19, 2013
  28. Duy NguyenDec 19, 2013
  29. René ScharfeDec 20, 2013
  30. Duy NguyenDec 20, 2013
  31. Junio C HamanoDec 20, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.