On 25/10/19 12:54PM, Junio C Hamano wrote:

Pratyush Yadav [off-list ref] writes:

quoted

 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