Re: [PATCH 1/1] git-gui: remove unused global declarations

[Pratyush Yadav <me@yadavpratyush.com>]

On 25/10/19 12:54PM, Junio C Hamano wrote:
> Pratyush Yadav <me@yadavpratyush.com> 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