You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Hudson <gh...@MIT.EDU> on 2003/02/07 17:02:01 UTC

Re: svn commit: rev 4739 - in trunk/subversion: svnadmin svnlook clients clients/cmdline tests/libsvn_subr svnserve svnversion

On Tue, 2003-02-04 at 21:51, Branko Čibej wrote:
> Correctness first, as usual. I'll fix these issues presently, but even
> deciding where to put such a function (it doesn't fit in any of our
> libraries) takes some thought.

I realize there is a difficult problem to solve here, but (speaking as
an individual developer) please do not commit code like this to the
trunk as an interim solution.  Putting function definitions in header
files is a gross violation of our coding methodology.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 4739 - in trunk/subversion: svnadmin svnlook clients clients/cmdline tests/libsvn_subr svnserve svnversion

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> well, the fact that this code makes svn fail to function on mac os x
> is a little more concerning to me...  would anyone object to me
> removing the error checking around the setlocale call like it was
> before?  as far as i can tell there is no way to get 'setlocale
> (LC_ALL, "")' to work there, and as a number of our developers (and
> users) use that operating system, i'd prefer to 'fix' it soon.

If the code has problems, by all means fix them.  But the problem
you're talking about is independent of the fact that the code happens
to be located in a .h file, right?  That's all I was talking about.

-K


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 4739 - in trunk/subversion: svnadmin svnlook clients clients/cmdline tests/libsvn_subr svnserve svnversion

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Friday, February 7, 2003, at 08:28 PM, Branko Čibej wrote:

> Garrett Rooney wrote:
>
>> On Friday, February 7, 2003, at 08:17 PM, Branko Čibej wrote:
>>
>>> Garrett Rooney wrote:
>>>
>>>> well, the fact that this code makes svn fail to function on mac os x
>>>> is a little more concerning to me...  would anyone object to me
>>>> removing the error checking around the setlocale call like it was
>>>> before?  as far as i can tell there is no way to get 'setlocale
>>>> (LC_ALL, "")' to work there, and as a number of our developers (and
>>>> users) use that operating system, i'd prefer to 'fix' it soon.
>>>
>>>
>>> How about changing LC_ALL to LC_CTYPE? IIRC, you said that is 
>>> supposed
>>> to work even on Mac OS X. IF you just remove the error checking, then
>>> the UTF-8 conversion has no chance of working on that OS.
>>
>>
>> that works fine.  will setting LC_CTYPE accomplish what the utf8 code
>> needs?
>
> It should; APR uses nl_langinfo to guess the native character encoding
> on Unix, see http://www.sensi.org/~alec/man/man3/nl_langinfo.html and
> http://www.sensi.org/~alec/man/man_h/langinfo.html.

great.  i'll commit the change.

thanks,

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 4739 - in trunk/subversion: svnadmin svnlook clients clients/cmdline tests/libsvn_subr svnserve svnversion

Posted by Branko Čibej <br...@xbc.nu>.
Garrett Rooney wrote:

> On Friday, February 7, 2003, at 08:17 PM, Branko Čibej wrote:
>
>> Garrett Rooney wrote:
>>
>>> well, the fact that this code makes svn fail to function on mac os x
>>> is a little more concerning to me...  would anyone object to me
>>> removing the error checking around the setlocale call like it was
>>> before?  as far as i can tell there is no way to get 'setlocale
>>> (LC_ALL, "")' to work there, and as a number of our developers (and
>>> users) use that operating system, i'd prefer to 'fix' it soon.
>>
>>
>> How about changing LC_ALL to LC_CTYPE? IIRC, you said that is supposed
>> to work even on Mac OS X. IF you just remove the error checking, then
>> the UTF-8 conversion has no chance of working on that OS.
>
>
> that works fine.  will setting LC_CTYPE accomplish what the utf8 code
> needs?

It should; APR uses nl_langinfo to guess the native character encoding
on Unix, see http://www.sensi.org/~alec/man/man3/nl_langinfo.html and
http://www.sensi.org/~alec/man/man_h/langinfo.html.


-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 4739 - in trunk/subversion: svnadmin svnlook clients clients/cmdline tests/libsvn_subr svnserve svnversion

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Friday, February 7, 2003, at 08:17 PM, Branko Čibej wrote:

> Garrett Rooney wrote:
>
>> well, the fact that this code makes svn fail to function on mac os x
>> is a little more concerning to me...  would anyone object to me
>> removing the error checking around the setlocale call like it was
>> before?  as far as i can tell there is no way to get 'setlocale
>> (LC_ALL, "")' to work there, and as a number of our developers (and
>> users) use that operating system, i'd prefer to 'fix' it soon.
>
> How about changing LC_ALL to LC_CTYPE? IIRC, you said that is supposed
> to work even on Mac OS X. IF you just remove the error checking, then
> the UTF-8 conversion has no chance of working on that OS.

that works fine.  will setting LC_CTYPE accomplish what the utf8 code 
needs?

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 4739 - in trunk/subversion: svnadmin svnlook clients clients/cmdline tests/libsvn_subr svnserve svnversion

Posted by Branko Čibej <br...@xbc.nu>.
Garrett Rooney wrote:

> well, the fact that this code makes svn fail to function on mac os x
> is a little more concerning to me...  would anyone object to me
> removing the error checking around the setlocale call like it was
> before?  as far as i can tell there is no way to get 'setlocale
> (LC_ALL, "")' to work there, and as a number of our developers (and
> users) use that operating system, i'd prefer to 'fix' it soon.

How about changing LC_ALL to LC_CTYPE? IIRC, you said that is supposed
to work even on Mac OS X. IF you just remove the error checking, then
the UTF-8 conversion has no chance of working on that OS.

-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 4739 - in trunk/subversion: svnadmin svnlook clients clients/cmdline tests/libsvn_subr svnserve svnversion

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Friday, February 7, 2003, at 05:02 PM, Karl Fogel wrote:

> Branko Čibej <br...@xbc.nu> writes:
>> I wish people would stop getting the jitters every time they see 
>> actual
>> code in a file ending with .h. It's perfectly valid C, and accepted
>> practice for certain situations. But all right, I can rename the file 
>> to
>> end in .c, if ti makes you feel better.
>
> No one's arguing that it's not valid C, and whether it is "accepted
> practice" is, of course, not a question that can be resolved by
> asserting that it is :-).
>
> I'd say, don't bother to rename the file.  When I get back to Chicago,
> I'll move the function definition to some reasonable place and put the
> prototype in a header, unless you've beaten me to it.  (This isn't a
> crisis, after all, only an annoyance.)

well, the fact that this code makes svn fail to function on mac os x is 
a little more concerning to me...  would anyone object to me removing 
the error checking around the setlocale call like it was before?  as 
far as i can tell there is no way to get 'setlocale (LC_ALL, "")' to 
work there, and as a number of our developers (and users) use that 
operating system, i'd prefer to 'fix' it soon.

-garrett


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 4739 - in trunk/subversion: svnadmin svnlook clients clients/cmdline tests/libsvn_subr svnserve svnversion

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Branko Čibej <br...@xbc.nu> writes:
> I wish people would stop getting the jitters every time they see actual
> code in a file ending with .h. It's perfectly valid C, and accepted
> practice for certain situations. But all right, I can rename the file to
> end in .c, if ti makes you feel better.

No one's arguing that it's not valid C, and whether it is "accepted
practice" is, of course, not a question that can be resolved by
asserting that it is :-).

I'd say, don't bother to rename the file.  When I get back to Chicago,
I'll move the function definition to some reasonable place and put the
prototype in a header, unless you've beaten me to it.  (This isn't a
crisis, after all, only an annoyance.)

-K, from sunny California

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 4739 - in trunk/subversion: svnadmin svnlook clients clients/cmdline tests/libsvn_subr svnserve svnversion

Posted by Branko Čibej <br...@xbc.nu>.
Greg Hudson wrote:

>On Tue, 2003-02-04 at 21:51, Branko Čibej wrote:
>  
>
>>Correctness first, as usual. I'll fix these issues presently, but even
>>deciding where to put such a function (it doesn't fit in any of our
>>libraries) takes some thought.
>>    
>>
>
>I realize there is a difficult problem to solve here, but (speaking as
>an individual developer) please do not commit code like this to the
>trunk as an interim solution.  Putting function definitions in header
>files is a gross violation of our coding methodology.
>  
>

I wish people would stop getting the jitters every time they see actual
code in a file ending with .h. It's perfectly valid C, and accepted
practice for certain situations. But all right, I can rename the file to
end in .c, if ti makes you feel better.

-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org