On 25/10/19 12:54PM, Junio C Hamano wrote:
> Pratyush Yadav <me@xxxxxxxxxxxxxxxxx> writes:
>
> > proc next_diff {{after {}}} {
> > - global next_diff_p next_diff_w next_diff_i
> > + global next_diff_p next_diff_w
> > show_diff $next_diff_p $next_diff_w {} {} $after
> > }
>
> Not in particular about next_diff_i, but seeing a hunk like this
> makes me wonder if you want to go the other way around. If a future
> fix needs to (re)introduce the use of next_diff_i global variable in
> this proc (it seems that there are two procs that declare the
> variable as global, one of which is this one, and the other one
> assigns to it), the code change must resurrect this declaration;
> otherwise the code would only confuse itself by potentially having
> two variables (one global, one local) with the same name, no?
I'm not sure what you mean by this. Do you mean we should keep
next_diff_i global, or do you mean we should instead convert it to a
local variable?
Or is it related to similar sounding variable names (next_diff_i,
next_diff_w, next_diff_p), which appear in multiple functions together?
> For next_diff_i in particular, I think the right solution would be
> to remove both global decl and the assignment, as the assignment is
> made to otherwise unused variable. But the primary point in such a
> change is not "remove unused global decl"; it is "remove unused
> variable".
Thanks for spotting it! Will fix.
--
Regards,
Pratyush Yadav
