Junio C Hamano <gitster@xxxxxxxxx> writes: >> # Guard against environment variables >> MAN1_TXT = >> +MAN1_TXT_WIP = >> +TCLTK_FILES = > > The latter name loses the fact that it is to hold candidates to be > on MAN1_TXT that happen to be conditionally included; calling it > MAN1_TXT_TCLTK or something, perhaps, may be an improvement. > > The former name makes it look it is work-in-progress, but in fact > they are definite and unconditional part of MAN1_TXT. Perhaps > MAN1_TXT_CORE or something? Sorry, I misread the patch. You collect all possible MAN1_TXT candidates on _WIP, so "this is unconditional core part" is wrong. Work-in-progress still sounds a bit funny, but now I know what is going on a bit better, it has become at last understandable ;-) >> +ifndef NO_TCLTK >> +MAN1_TXT_WIP += gitk.txt >> +MAN1_TXT = $(MAN1_TXT_WIP) >> +else >> +TCLTK_FILES += git-gui.txt >> +TCLTK_FILES += gitk.txt >> +TCLTK_FILES += git-citool.txt >> +MAN1_TXT = $(filter-out \ >> + $(TCLTK_FILES), \ >> + $(MAN1_TXT_WIP)) >> +endif I didn't notice it when I read it for the first time, but asymmetry between these two looks somewhat strange. If we are adding gitk.txt when we are not declining TCLTK based programs, why can we do without adding git-gui and git-citool at the same time? If we know we must add gitk.txt when we are not declining TCLTK based programs to MAN1_TXT_WIP in this section, it must mean that when we do not want TCLTK based programs, MAN1_TXT_WIP would not have gitk.txt on it, so why do we even need it on TCLTK_FILES list to filter it out?
