All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Ramsay Jones <ramsay@ramsayjones.plus.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	GIT Mailing-list <git@vger.kernel.org>,
	Adam Dinwoodie <git@dinwoodie.org>
Subject: Re: [PATCH 03/12] meson.build: only set build variables for non-default values
Date: Wed, 19 Mar 2025 14:36:02 +0100	[thread overview]
Message-ID: <Z9rIQlUtutWPiPgD@pks.im> (raw)
In-Reply-To: <5d0112ae-98b5-46f2-91ad-35ed11358c3e@ramsayjones.plus.com>

On Sat, Mar 15, 2025 at 02:46:59AM +0000, Ramsay Jones wrote:
> 
> Some preprocessor -Defines have defaults sets in the source code when
> they have not been provided to the C compiler. In this case, there is
> no need to pass them on the command-line, unless the build requires a
> non-standard value.
> 
> The build variables for DEFAULT_EDITOR, DEFAULT_HELP_FORMAT along with
> DEFAULT_PAGER have appropriate defaults ('vi', 'man' and 'less') set in
> the code. Add the preprocessor -Defines to the 'libgit_c_args' only if
> the values set with the corresponding 'options' are different to these
> standard values.

Hm. Does this really change anything though? The behaviour before and
after this patch are exactly the same as far as I understand, and by
explicitly handling the defaults we basically have to hard-code more
assumptions. So in the current form I don't see that this patch adds
much.

What we _could_ be doing is to completely drop the default values in
"meson_options.txt". In that case we could instead compare whether we
saw the empty string, which allows us to stop encoding the default vaule
both in "meson_options.txt" and in "editor.c".

Patrick

  reply	other threads:[~2025-03-19 13:36 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-15  2:46 [PATCH 03/12] meson.build: only set build variables for non-default values Ramsay Jones
2025-03-19 13:36 ` Patrick Steinhardt [this message]
2025-03-20  2:22   ` Ramsay Jones
2025-03-20  9:26     ` Patrick Steinhardt

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=Z9rIQlUtutWPiPgD@pks.im \
    --to=ps@pks.im \
    --cc=git@dinwoodie.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ramsay@ramsayjones.plus.com \
    /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.