{"thread":{"id":"25745","subject":"[PATCH] commit: Append commit_list prefix in two function names.","startedAt":"2010-11-14T21:03:03Z","lastAt":"2010-11-29T00:05:09Z","messageCount":5,"participants":["Thiago Farina","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"155875","messageId":"0b9cde68dcdfddc13a91a63f9deb3384576164de.1289768501.git.tfransosi@gmail.com","threadId":"25745","inReplyTo":null,"subject":"[PATCH] commit: Append commit_list prefix in two function names.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-11-14T21:03:03Z","receivedAt":"2010-11-14T21:03:03Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"Append commit_lis prefix to insert_by_date function and to sort_by_date,\nso it's clear that these functions refer to commit_list structure.\n\nSigned-off-by: Thiago Farina <tfransosi@gmail.com>\n---\n builtin/describe.c    |    6 +++---\n builtin/fetch-pack.c  |    4 ++--\n builtin/show-branch.c |    6 +++---\n commit.c              |   24 ++++++++++++------------\n commit.h              |   10 +++++-----\n revision.c            |   12 ++++++------\n sha1_name.c           |    2 +-\n upload-pack.c         |    4 ++--\n walker.c              |    2 +-\n 9 files changed, 35 insertions(+), 35 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 43caff2..9304dd0 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -189,7 +189,7 @@ static unsigned long finish_depth_computation(\n \t\t\tstruct commit *p = parents->item;\n \t\t\tparse_commit(p);\n \t\t\tif (!(p->object.flags & SEEN))\n-\t\t\t\tinsert_by_date(p, list);\n+\t\t\t\tcommit_list_insert_by_date(p, list);\n \t\t\tp->object.flags |= c->object.flags;\n \t\t\tparents = parents->next;\n \t\t}\n@@ -300,7 +300,7 @@ static void describe(const char *arg, int last_one)\n \t\t\tstruct commit *p = parents->item;\n \t\t\tparse_commit(p);\n \t\t\tif (!(p->object.flags & SEEN))\n-\t\t\t\tinsert_by_date(p, &list);\n+\t\t\t\tcommit_list_insert_by_date(p, &list);\n \t\t\tp->object.flags |= c->object.flags;\n \t\t\tparents = parents->next;\n \t\t}\n@@ -328,7 +328,7 @@ static void describe(const char *arg, int last_one)\n \tqsort(all_matches, match_cnt, sizeof(all_matches[0]), compare_pt);\n \n \tif (gave_up_on) {\n-\t\tinsert_by_date(gave_up_on, &list);\n+\t\tcommit_list_insert_by_date(gave_up_on, &list);\n \t\tseen_commits--;\n \t}\n \tseen_commits += finish_depth_computation(&list, &all_matches[0]);\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex dbd8b7b..b999413 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -47,7 +47,7 @@ static void rev_list_push(struct commit *commit, int mark)\n \t\t\tif (parse_commit(commit))\n \t\t\t\treturn;\n \n-\t\tinsert_by_date(commit, &rev_list);\n+\t\tcommit_list_insert_by_date(commit, &rev_list);\n \n \t\tif (!(commit->object.flags & COMMON))\n \t\t\tnon_common_revs++;\n@@ -436,7 +436,7 @@ static int mark_complete(const char *path, const unsigned char *sha1, int flag,\n \tif (o && o->type == OBJ_COMMIT) {\n \t\tstruct commit *commit = (struct commit *)o;\n \t\tcommit->object.flags |= COMPLETE;\n-\t\tinsert_by_date(commit, &complete);\n+\t\tcommit_list_insert_by_date(commit, &complete);\n \t}\n \treturn 0;\n }\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex 8663cca..da69581 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -243,7 +243,7 @@ static void join_revs(struct commit_list **list_p,\n \t\t\tif (mark_seen(p, seen_p) && !still_interesting)\n \t\t\t\textra--;\n \t\t\tp->object.flags |= flags;\n-\t\t\tinsert_by_date(p, list_p);\n+\t\t\tcommit_list_insert_by_date(p, list_p);\n \t\t}\n \t}\n \n@@ -859,7 +859,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t */\n \t\tcommit->object.flags |= flag;\n \t\tif (commit->object.flags == flag)\n-\t\t\tinsert_by_date(commit, &list);\n+\t\t\tcommit_list_insert_by_date(commit, &list);\n \t\trev[num_rev] = commit;\n \t}\n \tfor (i = 0; i < num_rev; i++)\n@@ -868,7 +868,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \tif (0 <= extra)\n \t\tjoin_revs(&list, &seen, num_rev, extra);\n \n-\tsort_by_date(&seen);\n+\tcommit_list_sort_by_date(&seen);\n \n \tif (merge_base)\n \t\treturn show_merge_base(seen, num_rev);\ndiff --git a/commit.c b/commit.c\nindex 0094ec1..554dcc3 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -360,7 +360,7 @@ void free_commit_list(struct commit_list *list)\n \t}\n }\n \n-struct commit_list * insert_by_date(struct commit *item, struct commit_list **list)\n+struct commit_list * commit_list_insert_by_date(struct commit *item, struct commit_list **list)\n {\n \tstruct commit_list **pp = list;\n \tstruct commit_list *p;\n@@ -374,11 +374,11 @@ struct commit_list * insert_by_date(struct commit *item, struct commit_list **li\n }\n \n \n-void sort_by_date(struct commit_list **list)\n+void commit_list_sort_by_date(struct commit_list **list)\n {\n \tstruct commit_list *ret = NULL;\n \twhile (*list) {\n-\t\tinsert_by_date((*list)->item, &ret);\n+\t\tcommit_list_insert_by_date((*list)->item, &ret);\n \t\t*list = (*list)->next;\n \t}\n \t*list = ret;\n@@ -398,7 +398,7 @@ struct commit *pop_most_recent_commit(struct commit_list **list,\n \t\tstruct commit *commit = parents->item;\n \t\tif (!parse_commit(commit) && !(commit->object.flags & mark)) {\n \t\t\tcommit->object.flags |= mark;\n-\t\t\tinsert_by_date(commit, list);\n+\t\t\tcommit_list_insert_by_date(commit, list);\n \t\t}\n \t\tparents = parents->next;\n \t}\n@@ -487,7 +487,7 @@ void sort_in_topological_order(struct commit_list ** list, int lifo)\n \n \t/* process the list in topological order */\n \tif (!lifo)\n-\t\tsort_by_date(&work);\n+\t\tcommit_list_sort_by_date(&work);\n \n \tpptr = list;\n \t*list = NULL;\n@@ -513,7 +513,7 @@ void sort_in_topological_order(struct commit_list ** list, int lifo)\n \t\t\t */\n \t\t\tif (--parent->indegree == 1) {\n \t\t\t\tif (!lifo)\n-\t\t\t\t\tinsert_by_date(parent, &work);\n+\t\t\t\t\tcommit_list_insert_by_date(parent, &work);\n \t\t\t\telse\n \t\t\t\t\tcommit_list_insert(parent, &work);\n \t\t\t}\n@@ -573,10 +573,10 @@ static struct commit_list *merge_bases_many(struct commit *one, int n, struct co\n \t}\n \n \tone->object.flags |= PARENT1;\n-\tinsert_by_date(one, &list);\n+\tcommit_list_insert_by_date(one, &list);\n \tfor (i = 0; i < n; i++) {\n \t\ttwos[i]->object.flags |= PARENT2;\n-\t\tinsert_by_date(twos[i], &list);\n+\t\tcommit_list_insert_by_date(twos[i], &list);\n \t}\n \n \twhile (interesting(list)) {\n@@ -594,7 +594,7 @@ static struct commit_list *merge_bases_many(struct commit *one, int n, struct co\n \t\tif (flags == (PARENT1 | PARENT2)) {\n \t\t\tif (!(commit->object.flags & RESULT)) {\n \t\t\t\tcommit->object.flags |= RESULT;\n-\t\t\t\tinsert_by_date(commit, &result);\n+\t\t\t\tcommit_list_insert_by_date(commit, &result);\n \t\t\t}\n \t\t\t/* Mark parents of a found merge stale */\n \t\t\tflags |= STALE;\n@@ -608,7 +608,7 @@ static struct commit_list *merge_bases_many(struct commit *one, int n, struct co\n \t\t\tif (parse_commit(p))\n \t\t\t\treturn NULL;\n \t\t\tp->object.flags |= flags;\n-\t\t\tinsert_by_date(p, &list);\n+\t\t\tcommit_list_insert_by_date(p, &list);\n \t\t}\n \t}\n \n@@ -618,7 +618,7 @@ static struct commit_list *merge_bases_many(struct commit *one, int n, struct co\n \twhile (list) {\n \t\tstruct commit_list *next = list->next;\n \t\tif (!(list->item->object.flags & STALE))\n-\t\t\tinsert_by_date(list->item, &result);\n+\t\t\tcommit_list_insert_by_date(list->item, &result);\n \t\tfree(list);\n \t\tlist = next;\n \t}\n@@ -711,7 +711,7 @@ struct commit_list *get_merge_bases_many(struct commit *one,\n \tresult = NULL;\n \tfor (i = 0; i < cnt; i++) {\n \t\tif (rslt[i])\n-\t\t\tinsert_by_date(rslt[i], &result);\n+\t\t\tcommit_list_insert_by_date(rslt[i], &result);\n \t}\n \tfree(rslt);\n \treturn result;\ndiff --git a/commit.h b/commit.h\nindex 9113bbe..7fbebb5 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -38,20 +38,20 @@ struct commit *lookup_commit_reference_gently(const unsigned char *sha1,\n \t\t\t\t\t      int quiet);\n \n int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size);\n-\n int parse_commit(struct commit *item);\n \n /* Find beginning and length of commit subject. */\n int find_commit_subject(const char *commit_buffer, const char **subject);\n \n-struct commit_list * commit_list_insert(struct commit *item, struct commit_list **list_p);\n+struct commit_list *commit_list_insert(struct commit *item,\n+\t\t\t\t\tstruct commit_list **list);\n unsigned commit_list_count(const struct commit_list *l);\n-struct commit_list * insert_by_date(struct commit *item, struct commit_list **list);\n+struct commit_list *commit_list_insert_by_date(struct commit *item,\n+\t\t\t            struct commit_list **list);\n+void commit_list_sort_by_date(struct commit_list **list);\n \n void free_commit_list(struct commit_list *list);\n \n-void sort_by_date(struct commit_list **list);\n-\n /* Commit formats */\n enum cmit_fmt {\n \tCMIT_FMT_RAW,\ndiff --git a/revision.c b/revision.c\nindex b1c1890..f8d4f87 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -444,15 +444,15 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n \tcommit->object.flags |= TREESAME;\n }\n \n-static void insert_by_date_cached(struct commit *p, struct commit_list **head,\n+static void commit_list_insert_by_date_cached(struct commit *p, struct commit_list **head,\n \t\t    struct commit_list *cached_base, struct commit_list **cache)\n {\n \tstruct commit_list *new_entry;\n \n \tif (cached_base && p->date < cached_base->item->date)\n-\t\tnew_entry = insert_by_date(p, &cached_base->next);\n+\t\tnew_entry = commit_list_insert_by_date(p, &cached_base->next);\n \telse\n-\t\tnew_entry = insert_by_date(p, head);\n+\t\tnew_entry = commit_list_insert_by_date(p, head);\n \n \tif (cache && (!*cache || p->date < (*cache)->item->date))\n \t\t*cache = new_entry;\n@@ -494,7 +494,7 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,\n \t\t\tif (p->object.flags & SEEN)\n \t\t\t\tcontinue;\n \t\t\tp->object.flags |= SEEN;\n-\t\t\tinsert_by_date_cached(p, list, cached_base, cache_ptr);\n+\t\t\tcommit_list_insert_by_date_cached(p, list, cached_base, cache_ptr);\n \t\t}\n \t\treturn 0;\n \t}\n@@ -521,7 +521,7 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,\n \t\tp->object.flags |= left_flag;\n \t\tif (!(p->object.flags & SEEN)) {\n \t\t\tp->object.flags |= SEEN;\n-\t\t\tinsert_by_date_cached(p, list, cached_base, cache_ptr);\n+\t\t\tcommit_list_insert_by_date_cached(p, list, cached_base, cache_ptr);\n \t\t}\n \t\tif (revs->first_parent_only)\n \t\t\tbreak;\n@@ -1891,7 +1891,7 @@ int prepare_revision_walk(struct rev_info *revs)\n \t\tif (commit) {\n \t\t\tif (!(commit->object.flags & SEEN)) {\n \t\t\t\tcommit->object.flags |= SEEN;\n-\t\t\t\tinsert_by_date(commit, &revs->commits);\n+\t\t\t\tcommit_list_insert_by_date(commit, &revs->commits);\n \t\t\t}\n \t\t}\n \t\te++;\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 484081d..e55ea05 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -683,7 +683,7 @@ static int handle_one_ref(const char *path,\n \t}\n \tif (object->type != OBJ_COMMIT)\n \t\treturn 0;\n-\tinsert_by_date((struct commit *)object, list);\n+\tcommit_list_insert_by_date((struct commit *)object, list);\n \tobject->flags |= ONELINE_SEEN;\n \treturn 0;\n }\ndiff --git a/upload-pack.c b/upload-pack.c\nindex f05e422..b40a43f 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -366,7 +366,7 @@ static int reachable(struct commit *want)\n {\n \tstruct commit_list *work = NULL;\n \n-\tinsert_by_date(want, &work);\n+\tcommit_list_insert_by_date(want, &work);\n \twhile (work) {\n \t\tstruct commit_list *list = work->next;\n \t\tstruct commit *commit = work->item;\n@@ -387,7 +387,7 @@ static int reachable(struct commit *want)\n \t\tfor (list = commit->parents; list; list = list->next) {\n \t\t\tstruct commit *parent = list->item;\n \t\t\tif (!(parent->object.flags & REACHABLE))\n-\t\t\t\tinsert_by_date(parent, &work);\n+\t\t\t\tcommit_list_insert_by_date(parent, &work);\n \t\t}\n \t}\n \twant->object.flags |= REACHABLE;\ndiff --git a/walker.c b/walker.c\nindex 11d9052..dce7128 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -207,7 +207,7 @@ static int mark_complete(const char *path, const unsigned char *sha1, int flag,\n \tstruct commit *commit = lookup_commit_reference_gently(sha1, 1);\n \tif (commit) {\n \t\tcommit->object.flags |= COMPLETE;\n-\t\tinsert_by_date(commit, &complete);\n+\t\tcommit_list_insert_by_date(commit, &complete);\n \t}\n \treturn 0;\n }\n-- \n1.7.3.2.161.g3089c\n"},{"id":"155876","messageId":"20101114211905.GB10150@burratino","threadId":"25745","inReplyTo":"0b9cde68dcdfddc13a91a63f9deb3384576164de.1289768501.git.tfransosi@gmail.com","subject":"Re: [PATCH] commit: Append commit_list prefix in two function names.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-14T21:19:05Z","receivedAt":"2010-11-14T21:19:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Thiago Farina wrote:\n\n> Append commit_lis prefix to insert_by_date function and to sort_by_date,\n> so it's clear that these functions refer to commit_list structure.\n\nNits: by \"Append\" I think you mean \"Add\"; missing \"t\" after commit_lis.\n\nThis gives the oft-used insert_by_name() function a fairly long name:\n\n\tcommit_list_insert_by_name\n\nThe proposed name is long enough to be unwieldly.  It might have\nthe virtue of fitting better with some of the commit_list lib:\n\n\tcommit_list_count\n\tcommit_list_insert\n\tfree_commit_list\n\nCompare:\n\n\tsort_by_date\n\tpop_most_recent_commit\n\tsort_in_topological_order\n\tpop_commit\n\nPerhaps this change would work better if some of the others were\nrenamed at the same time?\n\nSome notes on commit_lists in api-revision-walking.txt would do even\nmore to make it intuitive. :)\n\nHope that helps,\nJonathan\n"},{"id":"156707","messageId":"AANLkTi=V7e-KFhKVDLjH4TvoT6U3xmFieo5uqigPhqKF@mail.gmail.com","threadId":"25745","inReplyTo":"20101114211905.GB10150@burratino","subject":"Re: [PATCH] commit: Append commit_list prefix in two function names.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-11-27T01:54:05Z","receivedAt":"2010-11-27T01:54:05Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Sun, Nov 14, 2010 at 7:19 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Thiago Farina wrote:\n>\n>> Append commit_lis prefix to insert_by_date function and to sort_by_date,\n>> so it's clear that these functions refer to commit_list structure.\n>\n> Nits: by \"Append\" I think you mean \"Add\"; missing \"t\" after commit_lis.\n>\nThanks, I will fix this in a follow up.\n\n> This gives the oft-used insert_by_name() function a fairly long name:\n>\n>        commit_list_insert_by_name\n>\n> The proposed name is long enough to be unwieldly.  It might have\n> the virtue of fitting better with some of the commit_list lib:\n>\n>        commit_list_count\n>        commit_list_insert\n>        free_commit_list\n>\n> Compare:\n>\n>        sort_by_date\n>        pop_most_recent_commit\n>        sort_in_topological_order\n>        pop_commit\n>\n\nI don't understand what you are arguing here. Is about the size of\n\"commit_list_insert_by_name\"? I don't care about it's size, I just\nwant to make it consistent by adding commit_list in the functions that\nare part of the commit_list API.\n\n> Perhaps this change would work better if some of the others were\n> renamed at the same time?\n>\n\nI don't think so, this would increasing the size of the change and\nmake it less readable.\n"},{"id":"156719","messageId":"20101127082933.GA24840@burratino","threadId":"25745","inReplyTo":"AANLkTi=V7e-KFhKVDLjH4TvoT6U3xmFieo5uqigPhqKF@mail.gmail.com","subject":"Re: [PATCH] commit: Append commit_list prefix in two function names.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-27T08:29:33Z","receivedAt":"2010-11-27T08:29:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Thiago Farina wrote:\n> On Sun, Nov 14, 2010 at 7:19 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n\n>> This gives the oft-used insert_by_name() function a fairly long name:\n>>\n>>        commit_list_insert_by_name\n>>\n>> The proposed name is long enough to be unwieldly.  It might have\n>> the virtue of fitting better with some of the commit_list lib:\n>>\n>>        commit_list_count\n>>        commit_list_insert\n>>        free_commit_list\n>>\n>> Compare:\n>>\n>>        sort_by_date\n>>        pop_most_recent_commit\n>>        sort_in_topological_order\n>>        pop_commit\n>>\n>\n> I don't understand what you are arguing here. Is about the size of\n> \"commit_list_insert_by_name\"? I don't care about it's size,\n\nFor code clarity, length of function names can matter...\n\n> I just\n> want to make it consistent by adding commit_list in the functions that\n> are part of the commit_list API.\n\n... though this consideration is probably more important.\n\n>> Perhaps this change would work better if some of the others were\n>> renamed at the same time?\n>\n> I don't think so, this would increasing the size of the change and\n> make it less readable.\n\nEven if split up into multiple patches?  I don't think it makes much\nsense to say \"functions in the commit_list API all start with\ncommit_list_\" while at the same time leaving half of the functions in\nthe commit_list API without that suffix.\n\nBy the way, how did this come up?  Presumably some particular code\nwas confusing?  If so, that information could be useful as an example\nin the log message.\n\nRegards,\nJonathan\n"},{"id":"156784","messageId":"AANLkTi=hBzyufVoPrWJ1ngjzc0CZ0e202MaBtke9s_eR@mail.gmail.com","threadId":"25745","inReplyTo":"20101127082933.GA24840@burratino","subject":"Re: [PATCH] commit: Append commit_list prefix in two function names.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-11-29T00:05:09Z","receivedAt":"2010-11-29T00:05:09Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Sat, Nov 27, 2010 at 6:29 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Even if split up into multiple patches?  I don't think it makes much\n> sense to say \"functions in the commit_list API all start with\n> commit_list_\"\nI didn't say that in the patch.\n\n> while at the same time leaving half of the functions in\n> the commit_list API without that suffix.\n>\nWhat's is the problem with this? I don't see a reason to do this to\nall the functions in the same time. I prefer to do this in small\npatches. But it seems Junio is not going to pick this up?\n\n> By the way, how did this come up?  Presumably some particular code\n> was confusing?\nIt became with the idea of API, we see this with strbuf and\nstring_list. And this would be good to make the code more easy to\nread.\n"}]}