Re: [PATCH 3/4] diff: change scope of the function count_lines()

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Shourya,

On Fri, 3 Jul 2020, Shourya Shukla wrote:

> From: Prathamesh Chavan <pc44800@gmail.com>
>
> Change the scope of function count_lines for allowing the function
> to be reused in other parts of the code as well.

It may be just me, but I'd rather see the word "visibility" instead of
"scope" here. I mistook the subject line to indicate that the function is
changed to serve an (at least slightly) different purpose than before,
which is not actually the case.

Another alternative to "visibility" might be to imitate existing commit
messages, such as e4cb659ebdd (diff: export diffstat interface,
2019-11-13), 22184497a36 (factor out refresh_and_write_cache function,
2019-09-11) or ef283b3699f (apply: make parse_git_diff_header public,
2019-07-11).

In addition, as with all such changes, we need to be careful to consider
whether unrelated function names coming in from system headers might
clash. In this case, I think `count_lines()` is a bit too generic, but
will probably not clash. Personally, I would probably have opted for
`count_lines_in_string()`.

Ciao,
Johannes

>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Stefan Beller <sbeller@google.com>
> Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
> Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
> ---
>  diff.c | 2 +-
>  diff.h | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/diff.c b/diff.c
> index 4a2c631c37..39ddbcf451 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -547,7 +547,7 @@ struct emit_callback {
>  	struct strbuf *header;
>  };
>
> -static int count_lines(const char *data, int size)
> +int count_lines(const char *data, int size)
>  {
>  	int count, ch, completely_empty = 1, nl_just_seen = 0;
>  	count = 0;
> diff --git a/diff.h b/diff.h
> index 9443dc1b00..bd96c8c434 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -495,6 +495,7 @@ void free_diffstat_info(struct diffstat_t *diffstat);
>  int parse_long_opt(const char *opt, const char **argv,
>  		   const char **optarg);
>
> +int count_lines(const char *data, int size);
>  int git_diff_basic_config(const char *var, const char *value, void *cb);
>  int git_diff_heuristic_config(const char *var, const char *value, void *cb);
>  void init_diff_ui_defaults(void);
> --
> 2.27.0
>
>