Re: [PATCH 2/8] Export three functions from diff.c
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 14, 2010, 23:08 UTC
- Message-ID
- <7v39q0rlz8.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <4973127c3f9251b92c4836b19d818a340741102a.1292291624.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
Show 26 quoted lines
> From: Bo Yang <struggleyb.nku@gmail.com>
>
> Use fill_metainfo to fill the line level diff meta data,
> emit_line to print out a line and quote_two to quote
> paths.
>
> Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
> diff.c | 6 +++---
> diff.h | 17 +++++++++++++++++
> 2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 6991ed4..a0ea9e5 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -151,7 +151,7 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
> return git_color_default_config(var, value, cb);
> }
>
> -static char *quote_two(const char *one, const char *two)
> +char *quote_two(const char *one, const char *two)
> {
> int need_one = quote_c_style(one, NULL, NULL, 1);
> int need_two = quote_c_style(two, NULL, NULL, 1);This is not specific to "diff" anymore (it is a utility to cquote a path that happens to be stored as two separate components); wouldn't quote.c be a better home for it?
Show 5 quoted lines
> -static void emit_line(struct diff_options *o, const char *set, const char *reset,
> +void emit_line(struct diff_options *o, const char *set, const char *reset,
> const char *line, int len)
> {
> emit_line_0(o, set, reset, line[0], line+1, len-1);Within the context of "diff", it is clear that a function called emit_line() will be used to emit a single line of patch output, and within that context, it is understandable that it takes coloring related arguments as we do emit diff output in color.
But does it still make sense to give it such a generic sounding name when exported outside of its original context? Call it either emit-diff-line (if "diff-ness" is more important, and the new callers will use the function to emit diff output) or emit-colored-line (if the new caller produces output not necessarily related to diff, but is merely borrowing the coloring infrastructure from this implementation), perhaps?
Show 9 quoted lines
> @@ -2583,7 +2583,7 @@ static int similarity_index(struct diff_filepair *p) > return p->score * 100 / MAX_SCORE; > } > > -static void fill_metainfo(struct strbuf *msg, > +void fill_metainfo(struct strbuf *msg, > const char *name, > const char *other, > struct diff_filespec *one,
Likewise. Within the context of "diff", it is clear what kind of metainfo we mean (that's the one between "diff --git" and the first "@@ ... @@"). Other APIs with different kind of "metainfo" may later want to introduce a totally different function that fills their metainfo, and calling this "fill_metainfo" forces them to use some different name, and cause confusion to readers. Perhaps fill-diff-metainfo?