Re: [PATCH v3 01/13] graph: automatically track display width of graph lines
From: Junio C Hamano <hidden>
Date: 2019-10-16 03:35:23
"James Coglan via GitGitGadget" [off-list ref] writes:
+struct graph_line {
+ struct strbuf *buf;
+ size_t width;
+};
+
+static inline void graph_line_addch(struct graph_line *line, int c)
+{
+ strbuf_addch(line->buf, c);
+ line->width++;
+}
+
+static inline void graph_line_addchars(struct graph_line *line, int c, size_t n)
+{
+ strbuf_addchars(line->buf, c, n);
+ line->width += n;
+}
+
+static inline void graph_line_addstr(struct graph_line *line, const char *s)
+{
+ strbuf_addstr(line->buf, s);
+ line->width += strlen(s);
+}
+
+static inline void graph_line_addcolor(struct graph_line *line, unsigned short color)
+{
+ strbuf_addstr(line->buf, column_get_color_code(color));
+}All makes sense, and as long as nobody uses strbuf_add*() on line->buf directly, it shouldn't be too hard to extend these to support graph drawn characters outside ASCII in the future after this series settles.
static void graph_output_pre_commit_line(struct git_graph *graph,
...
for (i = 0; i < graph->num_columns; i++) {
struct column *col = &graph->columns[i];
if (col->commit == graph->commit) {
seen_this = 1;
- strbuf_write_column(sb, col, '|');
- strbuf_addchars(sb, ' ', graph->expansion_row);
- chars_written += 1 + graph->expansion_row;
+ graph_line_write_column(line, col, '|');
+ graph_line_addchars(line, ' ', graph->expansion_row);Nice reduction of noise, as the proposed log message promises.
int graph_next_line(struct git_graph *graph, struct strbuf *sb)
I just noticed it but does this have to be extern? Nobody outside graph.[ch] seems to have any reference to it. That might mean that it may be easier than we thought earlier to change the second parameter of this to "struct graph_line *", instead of us wrapping an incoming strbuf (which external callers are aware of, as opposed to graph_line which we prefer not to expose to outsiders). Whether we change the second parameter or not, the first clean-up may be to turn this function into a file-scope static, perhaps? All the changes are very pleasing to see. Thanks.