Re: [PATCH v14 01/11] trailer: add data structures and basic functions
On Wed, Sep 17, 2014 at 9:58 AM, Jeff King wrote: > On Tue, Sep 16, 2014 at 10:01:11AM +0200, Christian Couder wrote: > >> On Mon, Sep 15, 2014 at 10:39 PM, Junio C Hamano wrote: >> > Christian Couder writes: >> > >> >> +/* Get the length of buf from its beginning until its last alphanumeric >> >> character */ >> > >> > That makes it sound as if feeding "abc%de#f@" to the function returns >> > 3 for "abc", but >> >> For me the last alphanumeric character in "abc%de#f@" is "f", so it is >> the length from the beginning to "f" so it should return 8. > > FWIW, I parsed the comment as you intended, but I do think it is a bit > unclear (especially given the name, as it is skipping over more than > just alnums). From reading the calling code, it looks like the intent is > to take a token string like "Signed-off-by:" and find that the ":" is > part of the ending punctuation, but that the "-" are retained as > internal punctuation. > > Would it make sense as: > > /* >* Return the length of the string not including any final >* punctuation. E.g., the input "Signed-off-by:" would return >* 13, stripping the trailing punctuation but retaining >* internal punctuation. >*/ > int token_len_without_separator(const char *token) > ... > > The name is a bit clunky, but hopefully it is more clear what the point > is. Ok, I will use that in the next version. 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 v14 01/11] trailer: add data structures and basic functions
On Tue, Sep 16, 2014 at 10:01:11AM +0200, Christian Couder wrote: > On Mon, Sep 15, 2014 at 10:39 PM, Junio C Hamano wrote: > > Christian Couder writes: > > > >> +/* Get the length of buf from its beginning until its last alphanumeric > >> character */ > > > > That makes it sound as if feeding "abc%de#f@" to the function returns > > 3 for "abc", but > > For me the last alphanumeric character in "abc%de#f@" is "f", so it is > the length from the beginning to "f" so it should return 8. FWIW, I parsed the comment as you intended, but I do think it is a bit unclear (especially given the name, as it is skipping over more than just alnums). From reading the calling code, it looks like the intent is to take a token string like "Signed-off-by:" and find that the ":" is part of the ending punctuation, but that the "-" are retained as internal punctuation. Would it make sense as: /* * Return the length of the string not including any final * punctuation. E.g., the input "Signed-off-by:" would return * 13, stripping the trailing punctuation but retaining * internal punctuation. */ int token_len_without_separator(const char *token) ... The name is a bit clunky, but hopefully it is more clear what the point is. -Peff -- 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 v14 01/11] trailer: add data structures and basic functions
On Mon, Sep 15, 2014 at 10:39 PM, Junio C Hamano wrote:
> Christian Couder writes:
>
>> +/* Get the length of buf from its beginning until its last alphanumeric
>> character */
>
> That makes it sound as if feeding "abc%de#f@" to the function returns
> 3 for "abc", but
For me the last alphanumeric character in "abc%de#f@" is "f", so it is
the length from the beginning to "f" so it should return 8.
>> +static size_t alnum_len(const char *buf, size_t len)
>> +{
>> + while (len > 0 && !isalnum(buf[len - 1]))
>> + len--;
>> + return len;
>> +}
>
> doesn't it look at '@', be unhappy and decrement, look at 'f' and
> break out to return the length of "abc%de#f"?
Yeah, that's the expected behavior.
> Perhaps that behaviour _is_ what you want, but then the comment is
> lying, no?
I don't think so, but maybe you are parsing the comment in a different
way than I am.
What would you suggest instead?
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 v14 01/11] trailer: add data structures and basic functions
Christian Couder writes:
> +/* Get the length of buf from its beginning until its last alphanumeric
> character */
That makes it sound as if feeding "abc%de#f@" to the function returns
3 for "abc", but
> +static size_t alnum_len(const char *buf, size_t len)
> +{
> + while (len > 0 && !isalnum(buf[len - 1]))
> + len--;
> + return len;
> +}
doesn't it look at '@', be unhappy and decrement, look at 'f' and
break out to return the length of "abc%de#f"?
Perhaps that behaviour _is_ what you want, but then the comment is
lying, no?
--
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 v14 01/11] trailer: add data structures and basic functions
We will use a doubly linked list to store all information
about trailers and their configuration.
This way we can easily remove or add trailers to or from
trailer lists while traversing the lists in either direction.
Signed-off-by: Christian Couder
Signed-off-by: Junio C Hamano
---
Makefile | 1 +
trailer.c | 64 +++
2 files changed, 65 insertions(+)
create mode 100644 trailer.c
diff --git a/Makefile b/Makefile
index 63a210d..ef82972 100644
--- a/Makefile
+++ b/Makefile
@@ -888,6 +888,7 @@ LIB_OBJS += submodule-config.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
LIB_OBJS += trace.o
+LIB_OBJS += trailer.o
LIB_OBJS += transport.o
LIB_OBJS += transport-helper.o
LIB_OBJS += tree-diff.o
diff --git a/trailer.c b/trailer.c
new file mode 100644
index 000..2adc1b7
--- /dev/null
+++ b/trailer.c
@@ -0,0 +1,64 @@
+#include "cache.h"
+/*
+ * Copyright (c) 2013, 2014 Christian Couder
+ */
+
+enum action_where { WHERE_END, WHERE_AFTER, WHERE_BEFORE, WHERE_START };
+enum action_if_exists { EXISTS_ADD_IF_DIFFERENT_NEIGHBOR,
EXISTS_ADD_IF_DIFFERENT,
+ EXISTS_ADD, EXISTS_REPLACE, EXISTS_DO_NOTHING };
+enum action_if_missing { MISSING_ADD, MISSING_DO_NOTHING };
+
+struct conf_info {
+ char *name;
+ char *key;
+ char *command;
+ enum action_where where;
+ enum action_if_exists if_exists;
+ enum action_if_missing if_missing;
+};
+
+static struct conf_info default_conf_info;
+
+struct trailer_item {
+ struct trailer_item *previous;
+ struct trailer_item *next;
+ const char *token;
+ const char *value;
+ struct conf_info conf;
+};
+
+static struct trailer_item *first_conf_item;
+
+static char *separators = ":";
+
+static int after_or_end(enum action_where where)
+{
+ return (where == WHERE_AFTER) || (where == WHERE_END);
+}
+
+/* Get the length of buf from its beginning until its last alphanumeric
character */
+static size_t alnum_len(const char *buf, size_t len)
+{
+ while (len > 0 && !isalnum(buf[len - 1]))
+ len--;
+ return len;
+}
+
+static int same_token(struct trailer_item *a, struct trailer_item *b)
+{
+ size_t a_alnum_len = alnum_len(a->token, strlen(a->token));
+ size_t b_alnum_len = alnum_len(b->token, strlen(b->token));
+ size_t min_alnum_len = (a_alnum_len > b_alnum_len) ? b_alnum_len :
a_alnum_len;
+
+ return !strncasecmp(a->token, b->token, min_alnum_len);
+}
+
+static int same_value(struct trailer_item *a, struct trailer_item *b)
+{
+ return !strcasecmp(a->value, b->value);
+}
+
+static int same_trailer(struct trailer_item *a, struct trailer_item *b)
+{
+ return same_token(a, b) && same_value(a, b);
+}
--
2.0.3.960.g41c6e4c
--
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
