All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Eygene Ryabinkin <rea-git@codelabs.ru>,
	Philip Oakley <philipoakley@iee.org>
Subject: Re: [PATCH] Makefile: extend NO_TCLTK=NoThanks to cover docs
Date: Wed, 15 Aug 2018 19:57:19 +0200	[thread overview]
Message-ID: <87lg97qzr4.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <xmqq1sazk12m.fsf@gitster-ct.c.googlers.com>


On Wed, Aug 15 2018, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> 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 ;-)

Yeah maybe it should be *_TMP. It's because you can't assign to a make
variable twice (or rather, define a variable in terms of its previous
value via filter). Otherwise I would just munge it in-place.

>>> +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?

The only explicitly listed files are those that don't match the wildcard
git-*.txt. Therefore if we want gitk.txt we need to explicitly list only
it, but if we don't want the TCL programs we also need to list the ones
that match git-*.txt.

  reply	other threads:[~2018-08-15 17:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-15 15:15 [PATCH] Makefile: extend NO_TCLTK=NoThanks to cover docs Ævar Arnfjörð Bjarmason
2018-08-15 16:56 ` Junio C Hamano
2018-08-15 17:10   ` Junio C Hamano
2018-08-15 17:57     ` Ævar Arnfjörð Bjarmason [this message]
2018-08-15 18:07       ` Junio C Hamano
2018-08-17  6:42 ` Jonathan Nieder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lg97qzr4.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=philipoakley@iee.org \
    --cc=rea-git@codelabs.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.