Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-1-17-g77332b2

Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-1-17-g77332b2

[Ludovic Courtès]

Hello!

"Michael Gran" <spk121@yahoo.com> writes:

> The branch, master has been updated
>        via  77332b21a01fac906ae4707426e00f01e62c0415 (commit)
>       from  e5dc27b86d0eaa470f92cdaa9f4ed2a961338c49 (commit)

Oops, I hadn't realized this was in `master'.  Was it intended?  (I
don't remember seeing a discussion, but I may have skipped it.)

>     Replace global charnames variables with accessors
>     
>     The global variables scm_charnames and scm_charnums are replaced with
>     the accessor functions scm_i_charname and scm_i_charname_to_num.
>     Also, the incomplete and broken EBCDIC support is removed.

Does it have a user-visible effect?

(If so, please update `NEWS' for 1.9.1->1.9.2.)

>            * libguile/print.c (iprin1): use new func scm_i_charname
>     
>             * libguile/read.c (scm_read_character): use new func
>             scm_i_charname_to_num
>     
>             * libguile/chars.c (scm_i_charname): new function
>             (scm_i_charname_to_char): new function
>             (scm_charnames, scm_charnums): removed

These removals are incompatible in theory, but probably they don't
warrant a `NEWS' entry.  Thoughts?

> +const char *const scm_r5rs_charnames[] = 

Please follow the GCS when it comes to spacing, indentation, etc.  If in
doubt, run GNU Indent.

> +int scm_n_C0_control_charnames = sizeof (scm_C0_control_charnames) / sizeof (char *);

Scary name!  ;-)

Shouldn't it be private?  And shouldn't it be a macro instead?

Thanks,
Ludo'.

Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-1-17-g77332b2

[Ludovic Courtès]

Hi Mike,

Mike Gran <spk121@yahoo.com> writes:

> On Fri, 2009-07-31 at 00:57 +0200, Ludovic Courtès wrote:
> Hello!
>> 
>> "Michael Gran" <spk121@yahoo.com> writes:
>> 
>> > The branch, master has been updated
>> >        via  77332b21a01fac906ae4707426e00f01e62c0415 (commit)
>> >       from  e5dc27b86d0eaa470f92cdaa9f4ed2a961338c49 (commit)
>> 
>> Oops, I hadn't realized this was in `master'.  Was it intended?  (I
>> don't remember seeing a discussion, but I may have skipped it.)
>> 
>
> It was discussed after a fashion.  This is a precursor to the tree
> that Andy reviewed, and he seemed to be okay with me committing some of
> it.[1]  With all the exciting stuff going on, I was having a little
> trouble getting some mindshare. [2] [3] So I pushed this one since
> it was kind of your idea anyway. [4]

Oh right, thanks for reminding me!  And sorry for having been
unresponsive lately (too much good work going on and too little spare
time!).

>> Does it have a user-visible effect?
>
> No change at all in the character names it will accept.  A minor
> change on the output: writing U+0012 now gives #\ff over #\np.
> Certainly the removal of EBCDIC would be a user-visible effect if
> there were an EBCDIC user. 

Right, it probably won't hurt many people, so that's OK.  Nevertheless,
it's probably better to mention it in `NEWS', what do you think?

>> >            * libguile/print.c (iprin1): use new func scm_i_charname
>> >     
>> >             * libguile/read.c (scm_read_character): use new func
>> >             scm_i_charname_to_num
>> >     
>> >             * libguile/chars.c (scm_i_charname): new function
>> >             (scm_i_charname_to_char): new function
>> >             (scm_charnames, scm_charnums): removed
>> 
>> These removals are incompatible in theory, but probably they don't
>> warrant a `NEWS' entry.  Thoughts?
>
> If they were API, they weren't well documented as such.  They look like
> internal information to me.

Definitely.  It's just that they lacked the `_i_' prefix, which in
theory means that it's public and that we won't change it without
notice.  Let's assume removing these two is harmless.

Thanks,
Ludo'.