From: Junio C Hamano <gitster@pobox.com>
To: Philip <philip.c.peterson@gmail.com>
Cc: Philip Peterson via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH 1/2] apply: add unit tests for parse_range and rename to parse_fragment_range
Date: Thu, 04 Apr 2024 12:27:44 -0700 [thread overview]
Message-ID: <xmqqsf01yllb.fsf@gitster.g> (raw)
In-Reply-To: <CAJ6X7_Uc0OdzYToJSs15+vbydraKAB8x4DPj7UsL1PKLzyY0dQ@mail.gmail.com> (Philip's message of "Wed, 3 Apr 2024 23:53:27 -0400")
Philip <philip.c.peterson@gmail.com> writes:
> On Mon, Feb 19, 2024 at 4:35 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "Philip Peterson via GitGitGadget" <gitgitgadget@gmail.com> writes:
It's quite a blast from a long time ago that I no longer remember.
>> Alternatively we could do something like this to make the blast
>> radius of this patch smaller.
>>
>> -static int parse_range(const char *line, int len, int offset, const char *expect,
>> +#define apply_parse_fragment_range parse_range
>> +int parse_range(const char *line, int len, int offset, const char *expect,
>> unsigned long *p1, unsigned long *p2)
>
> From what I understand, this still creates a new extern symbol
> called parse_range.
Sorry, I misspoke. The direction of #define is the other way
around. That is, we may have to give the function a name that is
overly long because it needs to be externally linkable only to
support for your test, but we would want to locally rename that long
name down to the name currently used by the primary callers of that
function, so that the patch does not have to touch these existing
calling sites. After all, this function is a small implementation
detail and not a part of the official apply.c API, and the only
reason why we are making it extern is because some new tests want to
link with it from the side.
So, in the <apply.h> header file you'll do
/*
* exposed only for tests; do not call this as it not
* a part of the API
*/
extern int apply_parse_fragment_range(...);
and then in the original file that used to have "static int
parse_range(...)", you'd add
#define parse_range apply_parse_fragment_range
near the top, after the header files are included.
Thanks.
next prev parent reply other threads:[~2024-04-04 19:27 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-19 4:45 [PATCH 0/2] apply: add unit tests for parse_range Philip Peterson via GitGitGadget
2024-02-19 4:45 ` [PATCH 1/2] apply: add unit tests for parse_range and rename to parse_fragment_range Philip Peterson via GitGitGadget
2024-02-19 21:35 ` Junio C Hamano
2024-04-04 3:53 ` Philip
2024-04-04 19:27 ` Junio C Hamano [this message]
2024-02-19 4:45 ` [PATCH 2/2] apply: rewrite unit tests with structured cases Philip Peterson via GitGitGadget
2024-02-19 21:49 ` Junio C Hamano
2024-02-19 22:04 ` Kristoffer Haugsbakk
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=xmqqsf01yllb.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=philip.c.peterson@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).