{"thread":{"id":"35803","subject":"[PATCH 0/3] Add a function skip_prefix_if_present()","startedAt":"2014-02-05T06:25:14Z","lastAt":"2014-02-06T12:19:06Z","messageCount":7,"participants":["Michael Haggerty","Jeff King","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"234269","messageId":"1391581517-8586-1-git-send-email-mhagger@alum.mit.edu","threadId":"35803","inReplyTo":null,"subject":"[PATCH 0/3] Add a function skip_prefix_if_present()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-02-05T06:25:14Z","receivedAt":"2014-02-05T06:25:14Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"These patches apply on top of gitster/nd/more-skip-prefix.\n\nI noticed that Duy's new function skip_prefix_defval() was mostly\nbeing called with its first and third arguments the same. So\nintroduce a function skip_prefix_if_present() that implements this\npattern.\n\nMichael Haggerty (3):\n Add and use a function skip_prefix_if_present()\n diff_scoreopt_parse(): use skip_prefix_if_present()\n rev_is_head(): use skip_prefix()\n\n builtin/checkout.c | 4 ++--\n builtin/fast-export.c | 2 +-\n builtin/merge.c | 2 +-\n builtin/show-branch.c | 15 ++++++++-------\n diff.c | 6 +++---\n git-compat-util.h | 5 +++++\n git.c | 2 +-\n notes.c | 4 ++--\n refs.c | 2 +-\n wt-status.c | 4 ++--\n 10 files changed, 26 insertions(+), 20 deletions(-)\n\n-- \n1.8.5.3\n"},{"id":"234272","messageId":"1391581517-8586-2-git-send-email-mhagger@alum.mit.edu","threadId":"35803","inReplyTo":"1391581517-8586-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 1/3] Add and use a function skip_prefix_if_present()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-02-05T06:25:15Z","receivedAt":"2014-02-05T06:25:15Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Most of the calls to skip_prefix_defval() had equal first and third\narguments, with the effect of skipping the prefix if present, but\notherwise returning the original string. So define a new function\nthat does exactly that.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/checkout.c | 4 ++--\n builtin/fast-export.c | 2 +-\n builtin/merge.c | 2 +-\n builtin/show-branch.c | 6 +++---\n git-compat-util.h | 5 +++++\n git.c | 2 +-\n notes.c | 4 ++--\n refs.c | 2 +-\n wt-status.c | 4 ++--\n 9 files changed, 18 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 6531ed4..84682f1 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1151,8 +1151,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tconst char *argv0 = argv[0];\n \t\tif (!argc || !strcmp(argv0, \"--\"))\n \t\t\tdie (_(\"--track needs a branch name\"));\n-\t\targv0 = skip_prefix_defval(argv0, \"refs/\", argv0);\n-\t\targv0 = skip_prefix_defval(argv0, \"remotes/\", argv0);\n+\t\targv0 = skip_prefix_if_present(argv0, \"refs/\");\n+\t\targv0 = skip_prefix_if_present(argv0, \"remotes/\");\n \t\targv0 = strchr(argv0, '/');\n \t\tif (!argv0 || !argv0[1])\n \t\t\tdie (_(\"Missing branch name; try -b\"));\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex cd0a302..c87c7ea 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -476,7 +476,7 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\t}\n \t}\n \n-\tname = skip_prefix_defval(name, \"refs/tags/\", name);\n+\tname = skip_prefix_if_present(name, \"refs/tags/\");\n \tprintf(\"tag %s\\nfrom :%d\\n%.*s%sdata %d\\n%.*s\\n\",\n \t name, tagged_mark,\n \t (int)(tagger_end - tagger), tagger,\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 603f80a..7b01dcf 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1109,7 +1109,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t */\n \tbranch = branch_to_free = resolve_refdup(\"HEAD\", head_sha1, 0, &flag);\n \tif (branch)\n-\t\tbranch = skip_prefix_defval(branch, \"refs/heads/\", branch);\n+\t\tbranch = skip_prefix_if_present(branch, \"refs/heads/\");\n \tif (!branch || is_null_sha1(head_sha1))\n \t\thead_commit = NULL;\n \telse\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex 6078132..f2c3b19 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -284,7 +284,7 @@ static void show_one_commit(struct commit *commit, int no_name)\n \t\tpp_commit_easy(CMIT_FMT_ONELINE, commit, &pretty);\n \t\tpretty_str = pretty.buf;\n \t}\n-\tpretty_str = skip_prefix_defval(pretty_str, \"[PATCH] \", pretty_str);\n+\tpretty_str = skip_prefix_if_present(pretty_str, \"[PATCH] \");\n \n \tif (!no_name) {\n \t\tif (name && name->head_name) {\n@@ -478,7 +478,7 @@ static int rev_is_head(const char *head, int headlen, const char *name,\n \tif ((!head[0]) ||\n \t (head_sha1 && sha1 && hashcmp(head_sha1, sha1)))\n \t\treturn 0;\n-\thead = skip_prefix_defval(head, \"refs/heads/\", head);\n+\thead = skip_prefix_if_present(head, \"refs/heads/\");\n \tif (starts_with(name, \"refs/heads/\"))\n \t\tname += 11;\n \telse if (starts_with(name, \"heads/\"))\n@@ -810,7 +810,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t\thas_head++;\n \t\t}\n \t\tif (!has_head)\n-\t\t\tappend_one_rev(skip_prefix_defval(head, \"refs/heads/\", head));\n+\t\t\tappend_one_rev(skip_prefix_if_present(head, \"refs/heads/\"));\n \t}\n \n \tif (!ref_name_cnt) {\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 59265af..cff946c 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -368,6 +368,11 @@ static inline const char *skip_prefix(const char *str, const char *prefix)\n \treturn skip_prefix_defval(str, prefix, NULL);\n }\n \n+static inline const char *skip_prefix_if_present(const char *str, const char *prefix)\n+{\n+\treturn skip_prefix_defval(str, prefix, str);\n+}\n+\n static inline int starts_with(const char *str, const char *prefix)\n {\n \treturn skip_prefix(str, prefix) != NULL;\ndiff --git a/git.c b/git.c\nindex 321ae81..f3357d8 100644\n--- a/git.c\n+++ b/git.c\n@@ -579,7 +579,7 @@ int main(int argc, char **av)\n \targc--;\n \thandle_options(&argv, &argc, NULL);\n \tif (argc > 0) {\n-\t\targv[0] = skip_prefix_defval(argv[0], \"--\", argv[0]);\n+\t\targv[0] = skip_prefix_if_present(argv[0], \"--\");\n \t} else {\n \t\t/* The user didn't specify a command; give them help */\n \t\tcommit_pager_choice();\ndiff --git a/notes.c b/notes.c\nindex 31f513b..15c49d8 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -1243,8 +1243,8 @@ static void format_note(struct notes_tree *t, const unsigned char *object_sha1,\n \t\tif (!ref || !strcmp(ref, GIT_NOTES_DEFAULT_REF)) {\n \t\t\tstrbuf_addstr(sb, \"\\nNotes:\\n\");\n \t\t} else {\n-\t\t\tref = skip_prefix_defval(ref, \"refs/\", ref);\n-\t\t\tref = skip_prefix_defval(ref, \"notes/\", ref);\n+\t\t\tref = skip_prefix_if_present(ref, \"refs/\");\n+\t\t\tref = skip_prefix_if_present(ref, \"notes/\");\n \t\t\tstrbuf_addf(sb, \"\\nNotes (%s):\\n\", ref);\n \t\t}\n \t}\ndiff --git a/refs.c b/refs.c\nindex 217093f..808985d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2318,7 +2318,7 @@ static void try_remove_empty_parents(char *name)\n /* make sure nobody touched the ref, and unlink */\n static void prune_ref(struct ref_to_prune *r)\n {\n-\tconst char *name = skip_prefix_defval(r->name, \"refs/\", r->name);\n+\tconst char *name = skip_prefix_if_present(r->name, \"refs/\");\n \tstruct ref_lock *lock = lock_ref_sha1(name, r->sha1);\n \n \tif (lock) {\ndiff --git a/wt-status.c b/wt-status.c\nindex 185fa81..e7fab5c 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1179,8 +1179,8 @@ static void wt_status_get_detached_from(struct wt_status_state *state)\n \t ((commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&\n \t !hashcmp(cb.nsha1, commit->object.sha1)))) {\n \t\tconst char *p;\n-\t\tif ((p = skip_prefix_defval(ref, \"refs/tags/\", ref)) == ref)\n-\t\t\tp = skip_prefix_defval(ref, \"refs/remotes/\", ref);\n+\t\tif (!(p = skip_prefix(ref, \"refs/tags/\")))\n+\t\t\tp = skip_prefix_if_present(ref, \"refs/remotes/\");\n \t\tstate->detached_from = xstrdup(p);\n \t} else\n \t\tstate->detached_from =\n-- \n1.8.5.3\n"},{"id":"234271","messageId":"1391581517-8586-3-git-send-email-mhagger@alum.mit.edu","threadId":"35803","inReplyTo":"1391581517-8586-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 2/3] diff_scoreopt_parse(): use skip_prefix_if_present()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-02-05T06:25:16Z","receivedAt":"2014-02-05T06:25:16Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Whenever skip_prefix_defval() was called, opt was still equal to\n*opt_p. So we can use skip_prefix_if_present() instead.\n\nSigned-off-by: Michael Haggerty \n---\n diff.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d629cc5..a07fe9d 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3890,13 +3890,13 @@ static int diff_scoreopt_parse(const char **opt_p)\n \t*opt_p = opt;\n \tif (cmd == '-') {\n \t\t/* convert the long-form arguments into short-form versions */\n-\t\tif ((opt = skip_prefix_defval(opt, \"break-rewrites\", *opt_p)) != *opt_p) {\n+\t\tif ((opt = skip_prefix_if_present(opt, \"break-rewrites\")) != *opt_p) {\n \t\t\tif (*opt == 0 || *opt++ == '=')\n \t\t\t\tcmd = 'B';\n-\t\t} else if ((opt = skip_prefix_defval(opt, \"find-copies\", *opt_p)) != *opt_p) {\n+\t\t} else if ((opt = skip_prefix_if_present(opt, \"find-copies\")) != *opt_p) {\n \t\t\tif (*opt == 0 || *opt++ == '=')\n \t\t\t\tcmd = 'C';\n-\t\t} else if ((opt = skip_prefix_defval(opt, \"find-renames\", *opt_p)) != *opt_p) {\n+\t\t} else if ((opt = skip_prefix_if_present(opt, \"find-renames\")) != *opt_p) {\n \t\t\tif (*opt == 0 || *opt++ == '=')\n \t\t\t\tcmd = 'M';\n \t\t}\n-- \n1.8.5.3\n"},{"id":"234270","messageId":"1391581517-8586-4-git-send-email-mhagger@alum.mit.edu","threadId":"35803","inReplyTo":"1391581517-8586-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 3/3] rev_is_head(): use skip_prefix()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-02-05T06:25:17Z","receivedAt":"2014-02-05T06:25:17Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Change the logic a little bit so that we can use skip_prefix() instead\nof doing pointer arithmetic with hardcoded numbers.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/show-branch.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex f2c3b19..aee7fe5 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -475,14 +475,15 @@ static void snarf_refs(int head, int remotes)\n static int rev_is_head(const char *head, int headlen, const char *name,\n \t\t unsigned char *head_sha1, unsigned char *sha1)\n {\n+\tconst char *short_name;\n+\n \tif ((!head[0]) ||\n \t (head_sha1 && sha1 && hashcmp(head_sha1, sha1)))\n \t\treturn 0;\n \thead = skip_prefix_if_present(head, \"refs/heads/\");\n-\tif (starts_with(name, \"refs/heads/\"))\n-\t\tname += 11;\n-\telse if (starts_with(name, \"heads/\"))\n-\t\tname += 6;\n+\tif ((short_name = skip_prefix(name, \"refs/heads/\")) ||\n+\t (short_name = skip_prefix(name, \"heads/\")))\n+\t\treturn !strcmp(head, short_name);\n \treturn !strcmp(head, name);\n }\n \n-- \n1.8.5.3\n"},{"id":"234273","messageId":"52F1E04D.3090208@alum.mit.edu","threadId":"35803","inReplyTo":"1391581517-8586-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH 0/3] Add a function skip_prefix_if_present()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-02-05T06:55:09Z","receivedAt":"2014-02-05T06:55:09Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/05/2014 07:25 AM, Michael Haggerty wrote:\n> These patches apply on top of gitster/nd/more-skip-prefix.\n> \n> I noticed that Duy's new function skip_prefix_defval() was mostly\n> being called with its first and third arguments the same. So\n> introduce a function skip_prefix_if_present() that implements this\n> pattern.\n\nI see I should have read the whole previous thread [1] before firing off\nthis patch series. What I learned when I read it just now:\n\n* Johannes Sixt didn't think changes like the following improve\n readability:\n\n> -\t\tif (starts_with(arg, \"--upload-pack=\")) {\n> -\t\t\targs.uploadpack = arg + 14;\n> +\t\tif ((optarg = skip_prefix(arg, \"--upload-pack=\")) != NULL) {\n> +\t\t\targs.uploadpack = optarg;\n\n* René Scharfe submitted a patch to use a function parse_prefix()\n (originally suggested by Peff) instead of Duy's suggested approach:\n\n http://article.gmane.org/gmane.comp.version-control.git/239569\n\n His patch appears to have been overlooked.\n\n* Duy seemed to offer to rewrite his patch series, but I don't think\n that it has happened yet.\n\nAnd then the conversation was drowned by Christmas eggnog.\n\nI don't have a strong feeling about (Duy's proposal plus my patches) vs.\n(René's parse_prefix() approach). But I definitely *do* like the idea\nof getting rid of all those awkward magic numbers everywhere.\n\nMichael\n\n[1]\nhttp://thread.gmane.org/gmane.comp.version-control.git/239438/focus=239569\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"234290","messageId":"20140205172651.GC7268@sigill.intra.peff.net","threadId":"35803","inReplyTo":"52F1E04D.3090208@alum.mit.edu","subject":"Re: [PATCH 0/3] Add a function skip_prefix_if_present()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T17:26:51Z","receivedAt":"2014-02-05T17:26:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 05, 2014 at 07:55:09AM +0100, Michael Haggerty wrote:\n\n> * René Scharfe submitted a patch to use a function parse_prefix()\n> (originally suggested by Peff) instead of Duy's suggested approach:\n> \n> http://article.gmane.org/gmane.comp.version-control.git/239569\n> \n> His patch appears to have been overlooked.\n> \n> * Duy seemed to offer to rewrite his patch series, but I don't think\n> that it has happened yet.\n> \n> And then the conversation was drowned by Christmas eggnog.\n> \n> I don't have a strong feeling about (Duy's proposal plus my patches) vs.\n> (René's parse_prefix() approach). But I definitely *do* like the idea\n> of getting rid of all those awkward magic numbers everywhere.\n\nFWIW, after coming down off my eggnog bender, I think I still prefer\nRené's (my?) approach.\n\n-Peff\n"},{"id":"234361","messageId":"CACsJy8Dd8jD_mCGWsNfm0HJ=F5Ax17dqjd+jJ-RAMotjOU6tzw@mail.gmail.com","threadId":"35803","inReplyTo":"52F1E04D.3090208@alum.mit.edu","subject":"Re: [PATCH 0/3] Add a function skip_prefix_if_present()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-06T12:19:06Z","receivedAt":"2014-02-06T12:19:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Feb 5, 2014 at 1:55 PM, Michael Haggerty wrote:\n> * Duy seemed to offer to rewrite his patch series, but I don't think\n> that it has happened yet.\n\nIt's really low in my todo list. So if you want to pick it up, please do.\n-- \nDuy\n"}]}