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

From: Shourya Shukla <[email-masked]>
Date: Sat, 4 Jul 2020 20:58:02 +0530

On 03/07 05:07, Johannes Schindelin wrote:
> Hi Shourya,
> 
> On Fri, 3 Jul 2020, Shourya Shukla wrote:
> 
> > From: Prathamesh Chavan <[email-masked]>
> >
> > 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).

These are some excellent commit messages! I will take inspiration from
these. Thanks Dscho! :)

> 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()`.

I do not see it clashing with any other functions. Do we need to change
the name still (maybe to make the functions purpose even more clear)?