{"thread":{"id":"35278","subject":"[PATCH] Rename suffixcmp() to has_suffix()","startedAt":"2013-11-04T21:45:11Z","lastAt":"2013-11-04T22:06:50Z","messageCount":2,"participants":["Christian Couder","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"230094","messageId":"20131104214512.19757.74215.chriscool@tuxfamily.org","threadId":"35278","inReplyTo":null,"subject":"[PATCH] Rename suffixcmp() to has_suffix()","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2013-11-04T21:45:11Z","receivedAt":"2013-11-04T21:45:11Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"As suffixcmp() should not be used as an ordering comparison function,\nand anything-cmp() ought to be usable as an ordering comparison function,\nsuffixcmp() should be renamed to something that doesn't end with \"cmp\".\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\nHi Junio,\n\nThis is a patch, following your last suggestion, that should be added or\nsquashed into the 2 patch series I sent one hour ago.\n\nThanks,\nChristian.\n\n builtin/clone.c           | 2 +-\n builtin/fetch.c           | 2 +-\n builtin/merge-recursive.c | 2 +-\n builtin/remote.c          | 6 +++---\n builtin/repack.c          | 2 +-\n connected.c               | 2 +-\n git-compat-util.h         | 2 +-\n strbuf.c                  | 2 +-\n 8 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 874e0fd..4b8308e 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -510,7 +510,7 @@ static void write_followtags(const struct ref *refs, const char *msg)\n \tfor (ref = refs; ref; ref = ref->next) {\n \t\tif (prefixcmp(ref->name, \"refs/tags/\"))\n \t\t\tcontinue;\n-\t\tif (!suffixcmp(ref->name, \"^{}\"))\n+\t\tif (!has_suffix(ref->name, \"^{}\"))\n \t\t\tcontinue;\n \t\tif (!has_sha1_file(ref->old_sha1))\n \t\t\tcontinue;\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex bd7a101..a26b0e5 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -653,7 +653,7 @@ static void find_non_local_tags(struct transport *transport,\n \t\t * to fetch then we can mark the ref entry in the list\n \t\t * as one to ignore by setting util to NULL.\n \t\t */\n-\t\tif (!suffixcmp(ref->name, \"^{}\")) {\n+\t\tif (!has_suffix(ref->name, \"^{}\")) {\n \t\t\tif (item && !has_sha1_file(ref->old_sha1) &&\n \t\t\t    !will_fetch(head, ref->old_sha1) &&\n \t\t\t    !has_sha1_file(item->util) &&\ndiff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c\nindex 3a64f5d..6ba30aa 100644\n--- a/builtin/merge-recursive.c\n+++ b/builtin/merge-recursive.c\n@@ -29,7 +29,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \tstruct commit *result;\n \n \tinit_merge_options(&o);\n-\tif (argv[0] && !suffixcmp(argv[0], \"-subtree\"))\n+\tif (argv[0] && !has_suffix(argv[0], \"-subtree\"))\n \t\to.subtree_shift = \"\";\n \n \tif (argc < 4)\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 9b3a98e..c04fd7d 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -269,13 +269,13 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n \t\tenum { REMOTE, MERGE, REBASE } type;\n \n \t\tkey += 7;\n-\t\tif (!suffixcmp(key, \".remote\")) {\n+\t\tif (!has_suffix(key, \".remote\")) {\n \t\t\tname = xstrndup(key, strlen(key) - 7);\n \t\t\ttype = REMOTE;\n-\t\t} else if (!suffixcmp(key, \".merge\")) {\n+\t\t} else if (!has_suffix(key, \".merge\")) {\n \t\t\tname = xstrndup(key, strlen(key) - 6);\n \t\t\ttype = MERGE;\n-\t\t} else if (!suffixcmp(key, \".rebase\")) {\n+\t\t} else if (!has_suffix(key, \".rebase\")) {\n \t\t\tname = xstrndup(key, strlen(key) - 7);\n \t\t\ttype = REBASE;\n \t\t} else\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex a0ff5c7..79b14bf 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -78,7 +78,7 @@ static void get_non_kept_pack_filenames(struct string_list *fname_list)\n \t\treturn;\n \n \twhile ((e = readdir(dir)) != NULL) {\n-\t\tif (suffixcmp(e->d_name, \".pack\"))\n+\t\tif (has_suffix(e->d_name, \".pack\"))\n \t\t\tcontinue;\n \n \t\tlen = strlen(e->d_name) - strlen(\".pack\");\ndiff --git a/connected.c b/connected.c\nindex fae8d64..6a1629c 100644\n--- a/connected.c\n+++ b/connected.c\n@@ -38,7 +38,7 @@ int check_everything_connected_with_transport(sha1_iterate_fn fn,\n \tif (transport && transport->smart_options &&\n \t    transport->smart_options->self_contained_and_connected &&\n \t    transport->pack_lockfile &&\n-\t    !suffixcmp(transport->pack_lockfile, \".keep\")) {\n+\t    !has_suffix(transport->pack_lockfile, \".keep\")) {\n \t\tstruct strbuf idx_file = STRBUF_INIT;\n \t\tstrbuf_addstr(&idx_file, transport->pack_lockfile);\n \t\tstrbuf_setlen(&idx_file, idx_file.len - 5); /* \".keep\" */\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 7776f12..0f6a31e 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -351,7 +351,7 @@ extern void set_error_routine(void (*routine)(const char *err, va_list params));\n extern void set_die_is_recursing_routine(int (*routine)(void));\n \n extern int prefixcmp(const char *str, const char *prefix);\n-extern int suffixcmp(const char *str, const char *suffix);\n+extern int has_suffix(const char *str, const char *suffix);\n \n static inline const char *skip_prefix(const char *str, const char *prefix)\n {\ndiff --git a/strbuf.c b/strbuf.c\nindex 9ba50de..5a82735 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -10,7 +10,7 @@ int prefixcmp(const char *str, const char *prefix)\n \t\t\treturn (unsigned char)*prefix - (unsigned char)*str;\n }\n \n-int suffixcmp(const char *str, const char *suffix)\n+int has_suffix(const char *str, const char *suffix)\n {\n \tint len = strlen(str), suflen = strlen(suffix);\n \tif (len < suflen)\n-- \n1.8.4.1.561.g186b3da.dirty\n"},{"id":"230096","messageId":"20131104220650.GA26636@sigill.intra.peff.net","threadId":"35278","inReplyTo":"20131104214512.19757.74215.chriscool@tuxfamily.org","subject":"Re: [PATCH] Rename suffixcmp() to has_suffix()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-11-04T22:06:50Z","receivedAt":"2013-11-04T22:06:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 04, 2013 at 10:45:11PM +0100, Christian Couder wrote:\n\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 874e0fd..4b8308e 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -510,7 +510,7 @@ static void write_followtags(const struct ref *refs, const char *msg)\n>  \tfor (ref = refs; ref; ref = ref->next) {\n>  \t\tif (prefixcmp(ref->name, \"refs/tags/\"))\n>  \t\t\tcontinue;\n> -\t\tif (!suffixcmp(ref->name, \"^{}\"))\n> +\t\tif (!has_suffix(ref->name, \"^{}\"))\n\nI think the intent of your patch is reasonable, but doesn't it flip the\nlogic of the return value? That is, shouldn't this become:\n\n  if (has_suffix(...))\n\n?\n\n-Peff\n"}]}