From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sat, 05 Jun 2010 19:33:45 GMT
Subject: Re: [PATCH/RFC v7 1/2] Add infrastructure for translating Git with gettext
Message-ID: <AANLkTilRqVaVV5q-7NrserSDr0t4L1D_j8rNIWxtMYe1@mail.gmail.com>
In-Reply-To: <20100605185926.GA5273@progeny.tock>

On Sat, Jun 5, 2010 at 18:59, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi Ævar,
>
> Thanks for trying it out.

And thanks for your comments helping to make the series better.

> Ævar Arnfjörð Bjarmason wrote:
>
>> --- a/gettext.c
>> +++ b/gettext.c
>> @@ -1,11 +1,8 @@
>> -#ifdef NO_GETTEXT
>> -void git_setup_gettext(void) {}
>> -#else
>>  #include "exec_cmd.h"
>>  #include <libintl.h>
>>  #include <stdlib.h>
>>
>> -void git_setup_gettext(void) {
>> +inline void git_setup_gettext(void) {
>
> This should not be inline when NO_GETTEXT is unset, since other
> translation units don’t get a chance to see the definition.

I'm not familiar with what you mean, since I'm not that familiar with
the semantics of inline in C/GCC.

As an aside, I'd appreciate a document pointer, what are the
implications here exactly?

>> diff --git a/gettext.h b/gettext.h
>> index a99da6a..8d44808 100644
>> --- a/gettext.h
>> +++ b/gettext.h
>> @@ -1,7 +1,11 @@
>>  #ifndef GETTEXT_H
>>  #define GETTEXT_H
>>
>> -void git_setup_gettext(void);
>> +#ifdef NO_GETTEXT
>> +static inline void git_setup_gettext(void) {}
>> +#else
>> +inline void git_setup_gettext(void);
>> +#endif
>
> With s/^inline \(.*;\)/extern &/, to make it:
>
>  -void git_setup_gettext(void);
>  +#ifdef NO_GETTEXT
>  +static inline void git_setup_gettext(void) {}
>  +#else
>  +extern void git_setup_gettext(void);
>  +#endif
>
> this part is exactly how I was imagining it (i.e., like might_fault()
> in linux-2.6/include/linux/kernel.h and many other examples).

Done:

    diff --git a/gettext.c b/gettext.c
    index 4825799..407fbc0 100644
    --- a/gettext.c
    +++ b/gettext.c
    @@ -2,7 +2,7 @@
     #include <libintl.h>
     #include <stdlib.h>

    -inline void git_setup_gettext(void) {
    +extern void git_setup_gettext(void) {
     	char *podir;
     	char *envdir = getenv("GIT_TEXTDOMAINDIR");

    diff --git a/gettext.h b/gettext.h
    index 8d44808..7c36c1e 100644
    --- a/gettext.h
    +++ b/gettext.h
    @@ -4,7 +4,7 @@
     #ifdef NO_GETTEXT
     static inline void git_setup_gettext(void) {}
     #else
    -inline void git_setup_gettext(void);
    +extern void git_setup_gettext(void);
     #endif

     #ifdef NO_GETTEXT