Re: [PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result
On Thu, Nov 7, 2013 at 1:12 AM, Junio C Hamano gits...@pobox.com wrote: Jonathan Nieder jrnie...@gmail.com writes: The old name followed the pattern anything-cmp(), which suggests a general comparison function suitable for e.g. sorting objects. But this was not the case for suffixcmp(). It's not clear to me that prefixcmp() is usable for sorting objects, either. Shouldn't it get the same treatment? Sounds like a plan for a good follow-up series. Ok, I will have a look. If some day we invent a type for 4-byte-aligned object names, it might make sense to do something similar to hashcmp, distinguishing between hashcmp for use where ordering is important and something like hash_eq when checking for equality (since I suspect the latter can be made faster). Interesting. Yeah, but I will pass on this one. Thanks, Christian. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result
Hi, Christian Couder wrote: Now has_suffix() returns 1 when the suffix is present and 0 otherwise. Ok. My only worry is that the function is less discoverable since its name is so different from prefixcmp(), which might cause someone to invent yet another postfixcmp. The old name followed the pattern anything-cmp(), which suggests a general comparison function suitable for e.g. sorting objects. But this was not the case for suffixcmp(). It's not clear to me that prefixcmp() is usable for sorting objects, either. Shouldn't it get the same treatment? Except for that concern, the patch looks good. If some day we invent a type for 4-byte-aligned object names, it might make sense to do something similar to hashcmp, distinguishing between hashcmp for use where ordering is important and something like hash_eq when checking for equality (since I suspect the latter can be made faster). Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result
On 06.11.2013, at 23:17, Jonathan Nieder jrnie...@gmail.com wrote: Hi, Christian Couder wrote: Now has_suffix() returns 1 when the suffix is present and 0 otherwise. Ok. My only worry is that the function is less discoverable since its name is so different from prefixcmp(), which might cause someone to invent yet another postfixcmp. Well, that can always happen, no matter what, can't it? Though personally I wouldn't mind if there was an has_prefix instead or in parallel to prefixcmp. The old name followed the pattern anything-cmp(), which suggests a general comparison function suitable for e.g. sorting objects. But this was not the case for suffixcmp(). It's not clear to me that prefixcmp() is usable for sorting objects, either. Shouldn't it get the same treatment? Well, unlike suffixcmp, it is transitive, so it could be used for sorting. Whether doing that would be sensible is another question, though :-). For clarity, it might indeed be better to also change prefixcmp to has_prefix(), and if some code pops up in the future that needs something like the current prefixcmp, it can still be added back. signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result
Max Horn wrote: Well, unlike suffixcmp, it is transitive, so it could be used for sorting. It is not antisymmetric. prefixcmp(foo, foobar) 0 prefixcmp(foobar, foo) == 0 I can see how it's possible to care about the sign of the return value, but it's equally possible to care about the sign from suffixcmp. Neither is suitable as a function for passing to qsort. Hoping that clarifies, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result
Am 07.11.2013 um 00:28 schrieb Jonathan Nieder jrnie...@gmail.com: Max Horn wrote: Well, unlike suffixcmp, it is transitive, so it could be used for sorting. It is not antisymmetric. prefixcmp(foo, foobar) 0 prefixcmp(foobar, foo) == 0 Right! I wasn't thinkinh :-( I can see how it's possible to care about the sign of the return value, but it's equally possible to care about the sign from suffixcmp. Neither is suitable as a function for passing to qsort. Yeah, so then I'd be for changing rhis one to has_prefix. Hoping that clarifies, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result
Jonathan Nieder jrnie...@gmail.com writes: The old name followed the pattern anything-cmp(), which suggests a general comparison function suitable for e.g. sorting objects. But this was not the case for suffixcmp(). It's not clear to me that prefixcmp() is usable for sorting objects, either. Shouldn't it get the same treatment? Sounds like a plan for a good follow-up series. If some day we invent a type for 4-byte-aligned object names, it might make sense to do something similar to hashcmp, distinguishing between hashcmp for use where ordering is important and something like hash_eq when checking for equality (since I suspect the latter can be made faster). Interesting. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result
Now has_suffix() returns 1 when the suffix is present and 0 otherwise.
The old name followed the pattern anything-cmp(), which suggests
a general comparison function suitable for e.g. sorting objects.
But this was not the case for suffixcmp().
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/clone.c | 2 +-
builtin/fetch.c | 2 +-
builtin/merge-recursive.c | 2 +-
builtin/remote.c | 6 +++---
builtin/repack.c | 2 +-
connected.c | 2 +-
git-compat-util.h | 2 +-
strbuf.c | 6 +++---
8 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 874e0fd..84fb1bd 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -510,7 +510,7 @@ static void write_followtags(const struct ref *refs, const
char *msg)
for (ref = refs; ref; ref = ref-next) {
if (prefixcmp(ref-name, refs/tags/))
continue;
- if (!suffixcmp(ref-name, ^{}))
+ if (has_suffix(ref-name, ^{}))
continue;
if (!has_sha1_file(ref-old_sha1))
continue;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index bd7a101..8eb6cd0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -653,7 +653,7 @@ static void find_non_local_tags(struct transport *transport,
* to fetch then we can mark the ref entry in the list
* as one to ignore by setting util to NULL.
*/
- if (!suffixcmp(ref-name, ^{})) {
+ if (has_suffix(ref-name, ^{})) {
if (item !has_sha1_file(ref-old_sha1)
!will_fetch(head, ref-old_sha1)
!has_sha1_file(item-util)
diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
index 3a64f5d..e7f1a39 100644
--- a/builtin/merge-recursive.c
+++ b/builtin/merge-recursive.c
@@ -29,7 +29,7 @@ int cmd_merge_recursive(int argc, const char **argv, const
char *prefix)
struct commit *result;
init_merge_options(o);
- if (argv[0] !suffixcmp(argv[0], -subtree))
+ if (argv[0] has_suffix(argv[0], -subtree))
o.subtree_shift = ;
if (argc 4)
diff --git a/builtin/remote.c b/builtin/remote.c
index 9b3a98e..b9a1024 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -269,13 +269,13 @@ static int config_read_branches(const char *key, const
char *value, void *cb)
enum { REMOTE, MERGE, REBASE } type;
key += 7;
- if (!suffixcmp(key, .remote)) {
+ if (has_suffix(key, .remote)) {
name = xstrndup(key, strlen(key) - 7);
type = REMOTE;
- } else if (!suffixcmp(key, .merge)) {
+ } else if (has_suffix(key, .merge)) {
name = xstrndup(key, strlen(key) - 6);
type = MERGE;
- } else if (!suffixcmp(key, .rebase)) {
+ } else if (has_suffix(key, .rebase)) {
name = xstrndup(key, strlen(key) - 7);
type = REBASE;
} else
diff --git a/builtin/repack.c b/builtin/repack.c
index a0ff5c7..9ef518d 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -78,7 +78,7 @@ static void get_non_kept_pack_filenames(struct string_list
*fname_list)
return;
while ((e = readdir(dir)) != NULL) {
- if (suffixcmp(e-d_name, .pack))
+ if (!has_suffix(e-d_name, .pack))
continue;
len = strlen(e-d_name) - strlen(.pack);
diff --git a/connected.c b/connected.c
index fae8d64..be9304e 100644
--- a/connected.c
+++ b/connected.c
@@ -38,7 +38,7 @@ int check_everything_connected_with_transport(sha1_iterate_fn
fn,
if (transport transport-smart_options
transport-smart_options-self_contained_and_connected
transport-pack_lockfile
- !suffixcmp(transport-pack_lockfile, .keep)) {
+ has_suffix(transport-pack_lockfile, .keep)) {
struct strbuf idx_file = STRBUF_INIT;
strbuf_addstr(idx_file, transport-pack_lockfile);
strbuf_setlen(idx_file, idx_file.len - 5); /* .keep */
diff --git a/git-compat-util.h b/git-compat-util.h
index 7776f12..0f6a31e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -351,7 +351,7 @@ extern void set_error_routine(void (*routine)(const char
*err, va_list params));
extern void set_die_is_recursing_routine(int (*routine)(void));
extern int prefixcmp(const char *str, const char *prefix);
-extern int suffixcmp(const char *str, const char *suffix);
+extern int has_suffix(const char *str, const char *suffix);
static inline const char *skip_prefix(const char *str, const char *prefix)
{
diff --git a/strbuf.c b/strbuf.c
index
