{"thread":{"id":"33055","subject":"[PATCH] Revert \"graph.c: mark private file-scope symbols as static\"","startedAt":"2013-03-02T12:46:05Z","lastAt":"2013-03-04T04:25:19Z","messageCount":14,"participants":["John Keeping","Johan Herland","Thomas Rast","Junio C Hamano","Jason A. Donenfeld"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"210508","messageId":"50e7b3316fadbb550bea098ae92a0942a4429647.1362228122.git.john@keeping.me.uk","threadId":"33055","inReplyTo":null,"subject":"[PATCH] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-03-02T12:46:05Z","receivedAt":"2013-03-02T12:46:05Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.\n\nCGit uses these symbols to output the correct HTML around graph\nelements.  Making these symbols private means that CGit cannot be\nupdated to use Git 1.8.0 or newer, so let's not do that.\n\nSigned-off-by: John Keeping <john@keeping.me.uk>\n---\n\nI realise that Git isn't a library so making the API useful for outside\nprojects isn't a priority, but making these two methods public makes\nlife a lot easier for CGit.\n\nAdditionally, it seems that Johan added graph_set_column_colors\nspecifically so that CGit should use it - there's no value to having\nthat as a method just for its use in graph.c and he was the author of\nCGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15).\n\n graph.c | 32 ++------------------------------\n graph.h | 27 +++++++++++++++++++++++++++\n 2 files changed, 29 insertions(+), 30 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex 2a3fc5c..b24d04c 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -8,34 +8,6 @@\n /* Internal API */\n \n /*\n- * Output the next line for a graph.\n- * This formats the next graph line into the specified strbuf.  It is not\n- * terminated with a newline.\n- *\n- * Returns 1 if the line includes the current commit, and 0 otherwise.\n- * graph_next_line() will return 1 exactly once for each time\n- * graph_update() is called.\n- */\n-static int graph_next_line(struct git_graph *graph, struct strbuf *sb);\n-\n-/*\n- * Set up a custom scheme for column colors.\n- *\n- * The default column color scheme inserts ANSI color escapes to colorize\n- * the graph. The various color escapes are stored in an array of strings\n- * where each entry corresponds to a color, except for the last entry,\n- * which denotes the escape for resetting the color back to the default.\n- * When generating the graph, strings from this array are inserted before\n- * and after the various column characters.\n- *\n- * This function allows you to enable a custom array of color escapes.\n- * The 'colors_max' argument is the index of the last \"reset\" entry.\n- *\n- * This functions must be called BEFORE graph_init() is called.\n- */\n-static void graph_set_column_colors(const char **colors, unsigned short colors_max);\n-\n-/*\n  * Output a padding line in the graph.\n  * This is similar to graph_next_line().  However, it is guaranteed to\n  * never print the current commit line.  Instead, if the commit line is\n@@ -90,7 +62,7 @@ enum graph_state {\n static const char **column_colors;\n static unsigned short column_colors_max;\n \n-static void graph_set_column_colors(const char **colors, unsigned short colors_max)\n+void graph_set_column_colors(const char **colors, unsigned short colors_max)\n {\n \tcolumn_colors = colors;\n \tcolumn_colors_max = colors_max;\n@@ -1144,7 +1116,7 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf\n \t\tgraph_update_state(graph, GRAPH_PADDING);\n }\n \n-static int graph_next_line(struct git_graph *graph, struct strbuf *sb)\n+int graph_next_line(struct git_graph *graph, struct strbuf *sb)\n {\n \tswitch (graph->state) {\n \tcase GRAPH_PADDING:\ndiff --git a/graph.h b/graph.h\nindex 19b0f66..aff960c 100644\n--- a/graph.h\n+++ b/graph.h\n@@ -4,6 +4,22 @@\n /* A graph is a pointer to this opaque structure */\n struct git_graph;\n \n+/*\n+ * Set up a custom scheme for column colors.\n+ *\n+ * The default column color scheme inserts ANSI color escapes to colorize\n+ * the graph. The various color escapes are stored in an array of strings\n+ * where each entry corresponds to a color, except for the last entry,\n+ * which denotes the escape for resetting the color back to the default.\n+ * When generating the graph, strings from this array are inserted before\n+ * and after the various column characters.\n+ *\n+ * This function allows you to enable a custom array of color escapes.\n+ * The 'colors_max' argument is the index of the last \"reset\" entry.\n+ *\n+ * This functions must be called BEFORE graph_init() is called.\n+ */\n+void graph_set_column_colors(const char **colors, unsigned short colors_max);\n \n /*\n  * Create a new struct git_graph.\n@@ -33,6 +49,17 @@ void graph_update(struct git_graph *graph, struct commit *commit);\n  */\n int graph_is_commit_finished(struct git_graph const *graph);\n \n+/*\n+ * Output the next line for a graph.\n+ * This formats the next graph line into the specified strbuf.  It is not\n+ * terminated with a newline.\n+ *\n+ * Returns 1 if the line includes the current commit, and 0 otherwise.\n+ * graph_next_line() will return 1 exactly once for each time\n+ * graph_update() is called.\n+ */\n+int graph_next_line(struct git_graph *graph, struct strbuf *sb);\n+\n \n /*\n  * graph_show_*: helper functions for printing to stdout\n-- \n1.8.2.rc1.339.g93ec2c9\n"},{"id":"210511","messageId":"CALKQrgf9h_9_zOd9bDq9LZsg2Xo9zBaCS0GKaeJX2Og5LMiAyQ@mail.gmail.com","threadId":"33055","inReplyTo":"50e7b3316fadbb550bea098ae92a0942a4429647.1362228122.git.john@keeping.me.uk","subject":"Re: [PATCH] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-03-02T14:54:58Z","receivedAt":"2013-03-02T14:54:58Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Sat, Mar 2, 2013 at 1:46 PM, John Keeping <john@keeping.me.uk> wrote:\n> This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.\n>\n> CGit uses these symbols to output the correct HTML around graph\n> elements.  Making these symbols private means that CGit cannot be\n> updated to use Git 1.8.0 or newer, so let's not do that.\n>\n> Signed-off-by: John Keeping <john@keeping.me.uk>\n> ---\n>\n> I realise that Git isn't a library so making the API useful for outside\n> projects isn't a priority, but making these two methods public makes\n> life a lot easier for CGit.\n>\n> Additionally, it seems that Johan added graph_set_column_colors\n> specifically so that CGit should use it - there's no value to having\n> that as a method just for its use in graph.c and he was the author of\n> CGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15).\n\nCorrect,\n\nAcked-by: Johan Herland <johan@herland.net>\n\n\nHave fun! :)\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"210514","messageId":"87haktwr2a.fsf@pctrast.inf.ethz.ch","threadId":"33055","inReplyTo":"50e7b3316fadbb550bea098ae92a0942a4429647.1362228122.git.john@keeping.me.uk","subject":"Re: [PATCH] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2013-03-02T19:16:13Z","receivedAt":"2013-03-02T19:16:13Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"John Keeping <john@keeping.me.uk> writes:\n\n> This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.\n>\n> CGit uses these symbols to output the correct HTML around graph\n> elements.  Making these symbols private means that CGit cannot be\n> updated to use Git 1.8.0 or newer, so let's not do that.\n>\n> Signed-off-by: John Keeping <john@keeping.me.uk>\n> ---\n>\n> I realise that Git isn't a library so making the API useful for outside\n> projects isn't a priority, but making these two methods public makes\n> life a lot easier for CGit.\n>\n> Additionally, it seems that Johan added graph_set_column_colors\n> specifically so that CGit should use it - there's no value to having\n> that as a method just for its use in graph.c and he was the author of\n> CGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15).\n\nPerhaps you could add a comment in the source to prevent this from\nhappening again?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"210531","messageId":"20130303102946.GH7738@serenity.lan","threadId":"33055","inReplyTo":"87haktwr2a.fsf@pctrast.inf.ethz.ch","subject":"Re: [PATCH] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-03-03T10:29:46Z","receivedAt":"2013-03-03T10:29:46Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sat, Mar 02, 2013 at 08:16:13PM +0100, Thomas Rast wrote:\n> John Keeping <john@keeping.me.uk> writes:\n> \n> > This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.\n> >\n> > CGit uses these symbols to output the correct HTML around graph\n> > elements.  Making these symbols private means that CGit cannot be\n> > updated to use Git 1.8.0 or newer, so let's not do that.\n> >\n> > Signed-off-by: John Keeping <john@keeping.me.uk>\n> > ---\n> >\n> > I realise that Git isn't a library so making the API useful for outside\n> > projects isn't a priority, but making these two methods public makes\n> > life a lot easier for CGit.\n> >\n> > Additionally, it seems that Johan added graph_set_column_colors\n> > specifically so that CGit should use it - there's no value to having\n> > that as a method just for its use in graph.c and he was the author of\n> > CGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15).\n> \n> Perhaps you could add a comment in the source to prevent this from\n> happening again?\n\nThat feels wrong to me; would we really want a list of \"$OUTSIDE_PROJECT\nuses this\" against all methods?  CGit is using Git's internal API and so\nshould be prepared for breakage and to do what is necessary to work\naround it - it's just this one case where adding a 2 line function to\nGit makes CGit's life a lot easier.\n\nI would hope that having this message in the history should be enough to\nprevent this changing in the future; and it means that the comment is\nassociated with a date so that someone can decide to check whether CGit\nis still using this function in the distant future.\n\n\nJohn\n"},{"id":"210539","messageId":"7vk3pob38d.fsf@alter.siamese.dyndns.org","threadId":"33055","inReplyTo":"20130303102946.GH7738@serenity.lan","subject":"Re: [PATCH] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-03T21:08:50Z","receivedAt":"2013-03-03T21:08:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping <john@keeping.me.uk> writes:\n\n> On Sat, Mar 02, 2013 at 08:16:13PM +0100, Thomas Rast wrote:\n>> John Keeping <john@keeping.me.uk> writes:\n>> \n>> > This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.\n>> >\n>> > CGit uses these symbols to output the correct HTML around graph\n>> > elements.  Making these symbols private means that CGit cannot be\n>> > updated to use Git 1.8.0 or newer, so let's not do that.\n>> >\n>> > Signed-off-by: John Keeping <john@keeping.me.uk>\n>> > ---\n>> >\n>> > I realise that Git isn't a library so making the API useful for outside\n>> > projects isn't a priority, but making these two methods public makes\n>> > life a lot easier for CGit.\n>> >\n>> > Additionally, it seems that Johan added graph_set_column_colors\n>> > specifically so that CGit should use it - there's no value to having\n>> > that as a method just for its use in graph.c and he was the author of\n>> > CGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15).\n>> \n>> Perhaps you could add a comment in the source to prevent this from\n>> happening again?\n> ...\n> I would hope that having this message in the history should be enough to\n> prevent this changing in the future....\n\nGiven how it happened in the first place, I do not think anything\nshort of in-code comment would have helped.  There wouldn't be any\nhint to look into the history without one.\n"},{"id":"210541","messageId":"20130303214206.GL7738@serenity.lan","threadId":"33055","inReplyTo":"7vk3pob38d.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-03-03T21:42:06Z","receivedAt":"2013-03-03T21:42:06Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sun, Mar 03, 2013 at 01:08:50PM -0800, Junio C Hamano wrote:\n> John Keeping <john@keeping.me.uk> writes:\n> \n> > On Sat, Mar 02, 2013 at 08:16:13PM +0100, Thomas Rast wrote:\n> >> John Keeping <john@keeping.me.uk> writes:\n> >> \n> >> > This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.\n> >> >\n> >> > CGit uses these symbols to output the correct HTML around graph\n> >> > elements.  Making these symbols private means that CGit cannot be\n> >> > updated to use Git 1.8.0 or newer, so let's not do that.\n> >> >\n> >> > Signed-off-by: John Keeping <john@keeping.me.uk>\n> >> > ---\n> >> >\n> >> > I realise that Git isn't a library so making the API useful for outside\n> >> > projects isn't a priority, but making these two methods public makes\n> >> > life a lot easier for CGit.\n> >> >\n> >> > Additionally, it seems that Johan added graph_set_column_colors\n> >> > specifically so that CGit should use it - there's no value to having\n> >> > that as a method just for its use in graph.c and he was the author of\n> >> > CGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15).\n> >> \n> >> Perhaps you could add a comment in the source to prevent this from\n> >> happening again?\n> > ...\n> > I would hope that having this message in the history should be enough to\n> > prevent this changing in the future....\n> \n> Given how it happened in the first place, I do not think anything\n> short of in-code comment would have helped.  There wouldn't be any\n> hint to look into the history without one.\n\nSo you'd accept a patch doing that?  Something like this perhaps:\n\n    NOTE: Although these functions aren't used in Git outside graph.c,\n    they are used by CGit.\n"},{"id":"210545","messageId":"7vppzg9k0n.fsf@alter.siamese.dyndns.org","threadId":"33055","inReplyTo":"20130303214206.GL7738@serenity.lan","subject":"Re: [PATCH] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-03T22:49:12Z","receivedAt":"2013-03-03T22:49:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping <john@keeping.me.uk> writes:\n\n> On Sun, Mar 03, 2013 at 01:08:50PM -0800, Junio C Hamano wrote:\n>> >> > Additionally, it seems that Johan added graph_set_column_colors\n>> >> > specifically so that CGit should use it - there's no value to having\n>> >> > that as a method just for its use in graph.c and he was the author of\n>> >> > CGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15).\n>> >> \n>> >> Perhaps you could add a comment in the source to prevent this from\n>> >> happening again?\n>> > ...\n>> > I would hope that having this message in the history should be enough to\n>> > prevent this changing in the future....\n>> \n>> Given how it happened in the first place, I do not think anything\n>> short of in-code comment would have helped.  There wouldn't be any\n>> hint to look into the history without one.\n>\n> So you'd accept a patch doing that?\n\nThe answer obviously depends on the specifics of \"that\" ;-) I was\nmerely agreeing with what Thomas said.  A straight-revert would be\ninsufficient to prevent this from recurring again.\n\n> Something like this perhaps:\n>\n>     NOTE: Although these functions aren't used in Git outside graph.c,\n>     they are used by CGit.\n\nIt would be a good place to start, although I prefer to see it\ncompleted with s/used by CGit/& in order to do such and such/ by\nsomebody working on CGit.\n\nAlso it probably is worth adding contact information for folks who\nwork on CGit (http://hjemli.net/git/cgit/ might be sufficient), as\nchanging these functions (e.g. changing the function signature) will\naffect them; making them \"static\" is not the only way to hurt them.\n\nThanks.\n"},{"id":"210546","messageId":"20130303232413.GN7738@serenity.lan","threadId":"33055","inReplyTo":"7vppzg9k0n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-03-03T23:24:14Z","receivedAt":"2013-03-03T23:24:14Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sun, Mar 03, 2013 at 02:49:12PM -0800, Junio C Hamano wrote:\n> John Keeping <john@keeping.me.uk> writes:\n> \n> > On Sun, Mar 03, 2013 at 01:08:50PM -0800, Junio C Hamano wrote:\n> >> >> > Additionally, it seems that Johan added graph_set_column_colors\n> >> >> > specifically so that CGit should use it - there's no value to having\n> >> >> > that as a method just for its use in graph.c and he was the author of\n> >> >> > CGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15).\n> >> >> \n> >> >> Perhaps you could add a comment in the source to prevent this from\n> >> >> happening again?\n> >> > ...\n> >> > I would hope that having this message in the history should be enough to\n> >> > prevent this changing in the future....\n> >> \n> >> Given how it happened in the first place, I do not think anything\n> >> short of in-code comment would have helped.  There wouldn't be any\n> >> hint to look into the history without one.\n> >\n> > So you'd accept a patch doing that?\n> \n> The answer obviously depends on the specifics of \"that\" ;-) I was\n> merely agreeing with what Thomas said.  A straight-revert would be\n> insufficient to prevent this from recurring again.\n> \n> > Something like this perhaps:\n> >\n> >     NOTE: Although these functions aren't used in Git outside graph.c,\n> >     they are used by CGit.\n> \n> It would be a good place to start, although I prefer to see it\n> completed with s/used by CGit/& in order to do such and such/ by\n> somebody working on CGit.\n\nCGit uses graph_set_column_colors() to set the column colors to:\n\n    \"<span class='column1'>\",\n    ...\n    \"<span class='column6'>\",\n    \"</span>\"\n\n(the last value is RESET), thus avoiding the need to filter the output\nto convert ANSI colours to HTML.\n\nSimilarly, it accesses graph_next_line() directly so that it can output\nthe necessary HTML around each line.\n\n> Also it probably is worth adding contact information for folks who\n> work on CGit (http://hjemli.net/git/cgit/ might be sufficient),\n\nThe current CGit homepage is http://git.zx2c4.com/cgit/\n\n>                                                                 as\n> changing these functions (e.g. changing the function signature) will\n> affect them; making them \"static\" is not the only way to hurt them.\n\nBut since CGit uses a specific version of Git (as a submodule), in\ngeneral it doesn't need to worry about keeping consistency across\ndifferent versions.  CGit has been using Git 1.7.6 for quite a while -\nI posted a series of patches to take it to 1.7.12 yesterday [1] but hit\nthis issue when I got to 1.8.x.\n\nI think CGit expects to have to respond to changes in Git, so I don't\nthink it's worth restricting changes in Git for that reason - it's just\na case of exposing useful functionality somehow.\n\n[1] http://hjemli.net/pipermail/cgit/2013-March/000933.html\n"},{"id":"210547","messageId":"7vzjyk83gn.fsf@alter.siamese.dyndns.org","threadId":"33055","inReplyTo":"20130303232413.GN7738@serenity.lan","subject":"Re: [PATCH] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-03T23:32:08Z","receivedAt":"2013-03-03T23:32:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping <john@keeping.me.uk> writes:\n\n>> Also it probably is worth adding contact information for folks who\n>> work on CGit (http://hjemli.net/git/cgit/ might be sufficient),\n>\n> The current CGit homepage is http://git.zx2c4.com/cgit/\n\nAs the hjemli.net address is what I got as the first hit by asking\n[CGit] to websearch, it makes it clearer that we do need such\ncontact information there.\n\n> I think CGit expects to have to respond to changes in Git, so I don't\n> think it's worth restricting changes in Git for that reason - it's just\n> a case of exposing useful functionality somehow.\n\nI think you misread me.\n\nIt is not about restricting. It is to use their expertise to come up\nwith generally more useful API than responding only to the immediate\nneed we see in in-tree users. We want a discussion/patch to update\nthe graph.c infrastructure to be Cc'ed to them.\n\nFor example, with the current codeflow, the callers of these\nfunctions we have in-tree may be limited to those in graph.c but if\nthere are legitimate reason CGit wants to call them from sideways,\nperhaps there may be use cases in our codebase in the future to call\nthem from outside the normal graph.c codeflow.\n"},{"id":"210561","messageId":"20130304000337.GP7738@serenity.lan","threadId":"33055","inReplyTo":"7vzjyk83gn.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-03-04T00:03:37Z","receivedAt":"2013-03-04T00:03:37Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.\n\nCGit uses these symbols to output the correct HTML around graph\nelements.  Making these symbols private means that CGit cannot be\nupdated to use Git 1.8.0 or newer, so let's not do that.\n\nOn top of the revert, also add comments so that we avoid reintroducing\nthis problem in the future and suggest to those modifying this API\nthat they might want to discuss it with the CGit developers.\n\nSigned-off-by: John Keeping <john@keeping.me.uk>\n---\nOn Sun, Mar 03, 2013 at 03:32:08PM -0800, Junio C Hamano wrote:\n> John Keeping <john@keeping.me.uk> writes:\n> > I think CGit expects to have to respond to changes in Git, so I don't\n> > think it's worth restricting changes in Git for that reason - it's just\n> > a case of exposing useful functionality somehow.\n> \n> I think you misread me.\n\nYou're right - this makes more sense that what I originally thought you\nmeant :-)\n\n> It is not about restricting. It is to use their expertise to come up\n> with generally more useful API than responding only to the immediate\n> need we see in in-tree users. We want a discussion/patch to update\n> the graph.c infrastructure to be Cc'ed to them.\n> \n> For example, with the current codeflow, the callers of these\n> functions we have in-tree may be limited to those in graph.c but if\n> there are legitimate reason CGit wants to call them from sideways,\n> perhaps there may be use cases in our codebase in the future to call\n> them from outside the normal graph.c codeflow.\n\n graph.c | 32 ++------------------------------\n graph.h | 33 +++++++++++++++++++++++++++++++++\n 2 files changed, 35 insertions(+), 30 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex 2a3fc5c..b24d04c 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -8,34 +8,6 @@\n /* Internal API */\n \n /*\n- * Output the next line for a graph.\n- * This formats the next graph line into the specified strbuf.  It is not\n- * terminated with a newline.\n- *\n- * Returns 1 if the line includes the current commit, and 0 otherwise.\n- * graph_next_line() will return 1 exactly once for each time\n- * graph_update() is called.\n- */\n-static int graph_next_line(struct git_graph *graph, struct strbuf *sb);\n-\n-/*\n- * Set up a custom scheme for column colors.\n- *\n- * The default column color scheme inserts ANSI color escapes to colorize\n- * the graph. The various color escapes are stored in an array of strings\n- * where each entry corresponds to a color, except for the last entry,\n- * which denotes the escape for resetting the color back to the default.\n- * When generating the graph, strings from this array are inserted before\n- * and after the various column characters.\n- *\n- * This function allows you to enable a custom array of color escapes.\n- * The 'colors_max' argument is the index of the last \"reset\" entry.\n- *\n- * This functions must be called BEFORE graph_init() is called.\n- */\n-static void graph_set_column_colors(const char **colors, unsigned short colors_max);\n-\n-/*\n  * Output a padding line in the graph.\n  * This is similar to graph_next_line().  However, it is guaranteed to\n  * never print the current commit line.  Instead, if the commit line is\n@@ -90,7 +62,7 @@ enum graph_state {\n static const char **column_colors;\n static unsigned short column_colors_max;\n \n-static void graph_set_column_colors(const char **colors, unsigned short colors_max)\n+void graph_set_column_colors(const char **colors, unsigned short colors_max)\n {\n \tcolumn_colors = colors;\n \tcolumn_colors_max = colors_max;\n@@ -1144,7 +1116,7 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf\n \t\tgraph_update_state(graph, GRAPH_PADDING);\n }\n \n-static int graph_next_line(struct git_graph *graph, struct strbuf *sb)\n+int graph_next_line(struct git_graph *graph, struct strbuf *sb)\n {\n \tswitch (graph->state) {\n \tcase GRAPH_PADDING:\ndiff --git a/graph.h b/graph.h\nindex 19b0f66..0be62bd 100644\n--- a/graph.h\n+++ b/graph.h\n@@ -4,6 +4,25 @@\n /* A graph is a pointer to this opaque structure */\n struct git_graph;\n \n+/*\n+ * Set up a custom scheme for column colors.\n+ *\n+ * The default column color scheme inserts ANSI color escapes to colorize\n+ * the graph. The various color escapes are stored in an array of strings\n+ * where each entry corresponds to a color, except for the last entry,\n+ * which denotes the escape for resetting the color back to the default.\n+ * When generating the graph, strings from this array are inserted before\n+ * and after the various column characters.\n+ *\n+ * This function allows you to enable a custom array of color escapes.\n+ * The 'colors_max' argument is the index of the last \"reset\" entry.\n+ *\n+ * This functions must be called BEFORE graph_init() is called.\n+ *\n+ * NOTE: This function isn't used in Git outside graph.c but it is used\n+ * by CGit (http://git.zx2c4.com/cgit/) to use HTML for colors.\n+ */\n+void graph_set_column_colors(const char **colors, unsigned short colors_max);\n \n /*\n  * Create a new struct git_graph.\n@@ -33,6 +52,20 @@ void graph_update(struct git_graph *graph, struct commit *commit);\n  */\n int graph_is_commit_finished(struct git_graph const *graph);\n \n+/*\n+ * Output the next line for a graph.\n+ * This formats the next graph line into the specified strbuf.  It is not\n+ * terminated with a newline.\n+ *\n+ * Returns 1 if the line includes the current commit, and 0 otherwise.\n+ * graph_next_line() will return 1 exactly once for each time\n+ * graph_update() is called.\n+ *\n+ * NOTE: This function isn't used in Git outside graph.c but it is used\n+ * by CGit (http://git.zx2c4.com/cgit/) to wrap HTML around graph lines.\n+ */\n+int graph_next_line(struct git_graph *graph, struct strbuf *sb);\n+\n \n /*\n  * graph_show_*: helper functions for printing to stdout\n-- \n1.8.2.rc1.339.g93ec2c9\n"},{"id":"210565","messageId":"CAHmME9oAiZDcAeMCE=haUmC9yeC0crZCKB-WrxQ3CVd1YrBdHQ@mail.gmail.com","threadId":"33055","inReplyTo":"20130304000337.GP7738@serenity.lan","subject":"Re: [PATCH v2] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"Jason A. Donenfeld","fromEmail":"jason@zx2c4.com","sentAt":"2013-03-04T00:12:44Z","receivedAt":"2013-03-04T00:12:44Z","isPatch":true,"sender":{"key":"jason@zx2c4.com","avatar":"https://gravatar.com/avatar/8a3d6c2049b8353b1e0fb78142042d142c9dc72975698d944933997974d9fac5?d=mp&s=160"},"body":"Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>\n\nOn Sun, Mar 3, 2013 at 7:03 PM, John Keeping <john@keeping.me.uk> wrote:\n> This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.\n>\n> CGit uses these symbols to output the correct HTML around graph\n> elements.  Making these symbols private means that CGit cannot be\n> updated to use Git 1.8.0 or newer, so let's not do that.\n>\n> On top of the revert, also add comments so that we avoid reintroducing\n> this problem in the future and suggest to those modifying this API\n> that they might want to discuss it with the CGit developers.\n>\n> Signed-off-by: John Keeping <john@keeping.me.uk>\n> ---\n> On Sun, Mar 03, 2013 at 03:32:08PM -0800, Junio C Hamano wrote:\n>> John Keeping <john@keeping.me.uk> writes:\n>> > I think CGit expects to have to respond to changes in Git, so I don't\n>> > think it's worth restricting changes in Git for that reason - it's just\n>> > a case of exposing useful functionality somehow.\n>>\n>> I think you misread me.\n>\n> You're right - this makes more sense that what I originally thought you\n> meant :-)\n>\n>> It is not about restricting. It is to use their expertise to come up\n>> with generally more useful API than responding only to the immediate\n>> need we see in in-tree users. We want a discussion/patch to update\n>> the graph.c infrastructure to be Cc'ed to them.\n>>\n>> For example, with the current codeflow, the callers of these\n>> functions we have in-tree may be limited to those in graph.c but if\n>> there are legitimate reason CGit wants to call them from sideways,\n>> perhaps there may be use cases in our codebase in the future to call\n>> them from outside the normal graph.c codeflow.\n>\n>  graph.c | 32 ++------------------------------\n>  graph.h | 33 +++++++++++++++++++++++++++++++++\n>  2 files changed, 35 insertions(+), 30 deletions(-)\n>\n> diff --git a/graph.c b/graph.c\n> index 2a3fc5c..b24d04c 100644\n> --- a/graph.c\n> +++ b/graph.c\n> @@ -8,34 +8,6 @@\n>  /* Internal API */\n>\n>  /*\n> - * Output the next line for a graph.\n> - * This formats the next graph line into the specified strbuf.  It is not\n> - * terminated with a newline.\n> - *\n> - * Returns 1 if the line includes the current commit, and 0 otherwise.\n> - * graph_next_line() will return 1 exactly once for each time\n> - * graph_update() is called.\n> - */\n> -static int graph_next_line(struct git_graph *graph, struct strbuf *sb);\n> -\n> -/*\n> - * Set up a custom scheme for column colors.\n> - *\n> - * The default column color scheme inserts ANSI color escapes to colorize\n> - * the graph. The various color escapes are stored in an array of strings\n> - * where each entry corresponds to a color, except for the last entry,\n> - * which denotes the escape for resetting the color back to the default.\n> - * When generating the graph, strings from this array are inserted before\n> - * and after the various column characters.\n> - *\n> - * This function allows you to enable a custom array of color escapes.\n> - * The 'colors_max' argument is the index of the last \"reset\" entry.\n> - *\n> - * This functions must be called BEFORE graph_init() is called.\n> - */\n> -static void graph_set_column_colors(const char **colors, unsigned short colors_max);\n> -\n> -/*\n>   * Output a padding line in the graph.\n>   * This is similar to graph_next_line().  However, it is guaranteed to\n>   * never print the current commit line.  Instead, if the commit line is\n> @@ -90,7 +62,7 @@ enum graph_state {\n>  static const char **column_colors;\n>  static unsigned short column_colors_max;\n>\n> -static void graph_set_column_colors(const char **colors, unsigned short colors_max)\n> +void graph_set_column_colors(const char **colors, unsigned short colors_max)\n>  {\n>         column_colors = colors;\n>         column_colors_max = colors_max;\n> @@ -1144,7 +1116,7 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf\n>                 graph_update_state(graph, GRAPH_PADDING);\n>  }\n>\n> -static int graph_next_line(struct git_graph *graph, struct strbuf *sb)\n> +int graph_next_line(struct git_graph *graph, struct strbuf *sb)\n>  {\n>         switch (graph->state) {\n>         case GRAPH_PADDING:\n> diff --git a/graph.h b/graph.h\n> index 19b0f66..0be62bd 100644\n> --- a/graph.h\n> +++ b/graph.h\n> @@ -4,6 +4,25 @@\n>  /* A graph is a pointer to this opaque structure */\n>  struct git_graph;\n>\n> +/*\n> + * Set up a custom scheme for column colors.\n> + *\n> + * The default column color scheme inserts ANSI color escapes to colorize\n> + * the graph. The various color escapes are stored in an array of strings\n> + * where each entry corresponds to a color, except for the last entry,\n> + * which denotes the escape for resetting the color back to the default.\n> + * When generating the graph, strings from this array are inserted before\n> + * and after the various column characters.\n> + *\n> + * This function allows you to enable a custom array of color escapes.\n> + * The 'colors_max' argument is the index of the last \"reset\" entry.\n> + *\n> + * This functions must be called BEFORE graph_init() is called.\n> + *\n> + * NOTE: This function isn't used in Git outside graph.c but it is used\n> + * by CGit (http://git.zx2c4.com/cgit/) to use HTML for colors.\n> + */\n> +void graph_set_column_colors(const char **colors, unsigned short colors_max);\n>\n>  /*\n>   * Create a new struct git_graph.\n> @@ -33,6 +52,20 @@ void graph_update(struct git_graph *graph, struct commit *commit);\n>   */\n>  int graph_is_commit_finished(struct git_graph const *graph);\n>\n> +/*\n> + * Output the next line for a graph.\n> + * This formats the next graph line into the specified strbuf.  It is not\n> + * terminated with a newline.\n> + *\n> + * Returns 1 if the line includes the current commit, and 0 otherwise.\n> + * graph_next_line() will return 1 exactly once for each time\n> + * graph_update() is called.\n> + *\n> + * NOTE: This function isn't used in Git outside graph.c but it is used\n> + * by CGit (http://git.zx2c4.com/cgit/) to wrap HTML around graph lines.\n> + */\n> +int graph_next_line(struct git_graph *graph, struct strbuf *sb);\n> +\n>\n>  /*\n>   * graph_show_*: helper functions for printing to stdout\n> --\n> 1.8.2.rc1.339.g93ec2c9\n>\n"},{"id":"210567","messageId":"CALKQrgds0-Fj-cZuvBHKiRHDx9vyY=dYpfOP0wE0WTHfAJ62oA@mail.gmail.com","threadId":"33055","inReplyTo":"20130304000337.GP7738@serenity.lan","subject":"Re: [PATCH v2] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-03-04T00:52:42Z","receivedAt":"2013-03-04T00:52:42Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Mon, Mar 4, 2013 at 1:03 AM, John Keeping <john@keeping.me.uk> wrote:\n> This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.\n>\n> CGit uses these symbols to output the correct HTML around graph\n> elements.  Making these symbols private means that CGit cannot be\n> updated to use Git 1.8.0 or newer, so let's not do that.\n>\n> On top of the revert, also add comments so that we avoid reintroducing\n> this problem in the future and suggest to those modifying this API\n> that they might want to discuss it with the CGit developers.\n>\n> Signed-off-by: John Keeping <john@keeping.me.uk>\n\nStill-Acked-by: Johan Herland <johan@herland.net>\n\n\n...Johan\n\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"210569","messageId":"7vzjyjygnd.fsf@alter.siamese.dyndns.org","threadId":"33055","inReplyTo":"CAHmME9oAiZDcAeMCE=haUmC9yeC0crZCKB-WrxQ3CVd1YrBdHQ@mail.gmail.com","subject":"Re: [PATCH v2] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-04T03:42:46Z","receivedAt":"2013-03-04T03:42:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jason A. Donenfeld\" <Jason@zx2c4.com> writes:\n\n> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>\n\nI suspect that many people outside CGit circle may not know who this\nJason (who never appeared in git@vger.kernel.org) is, and are\nwondering in what capacity he is sending a S-o-b for somebody else's\npatch without explanation.  An inroduction may help others (as can\nbe seen on Cc: line, I added Lars there because I didn't even know\nhe was still the man behind CGit).\n\nLater last year, the project was taken over by an active contributor\nwith the other active contributor (Ferry Huberts)'s support and then\nlater with Lars's blessing:\n\n  http://hjemli.net/pipermail/cgit/2013-January/000884.html\n\nThat active contributor is Jason. The repository has also been moved\nto Jason's http://git.zx2c4.com/cgit/ even though mailing list and\nits archive seem to be still suported by Lars's hjemli.net.  And the\nlatest CGit 0.9.1 was release mid November 2012 by him.\n\nJohn Keeping has been actively working on making sure that CGit\nworks with newer codebase for the past couple of days, until he\nfound out that after our v1.7.12.4 some symbols were unavailable to\nhim from us X-< which triggered this topic.  Jason's S-o-b as the\nCGit maintainer is certainly appropriate for this patch.\n\nSo, thanks, Jason and John for your efforts.\n"},{"id":"210570","messageId":"CAHmME9p_bDLxa8dG=b=5+FL4CXgdeKJVv4hFoFJwG-vxW-TiYQ@mail.gmail.com","threadId":"33055","inReplyTo":"7vzjyjygnd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] Revert \"graph.c: mark private file-scope symbols as static\"","fromName":"Jason A. Donenfeld","fromEmail":"jason@zx2c4.com","sentAt":"2013-03-04T04:25:19Z","receivedAt":"2013-03-04T04:25:19Z","isPatch":true,"sender":{"key":"jason@zx2c4.com","avatar":"https://gravatar.com/avatar/8a3d6c2049b8353b1e0fb78142042d142c9dc72975698d944933997974d9fac5?d=mp&s=160"},"body":"On Sun, Mar 3, 2013 at 10:42 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> I suspect that many people outside CGit circle may not know who this\n> Jason\n> That active contributor is Jason. The repository has also been moved\n> to Jason's http://git.zx2c4.com/cgit/\n> So, thanks, Jason and John for your efforts.\n\nHey folks,\n\nSorry for not providing the explanation myself, but Junio -- thanks\nfor introducing me!\n\nJason\n"}]}