unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
From: ludo@gnu.org (Ludovic Courtès)
To: guile-devel@gnu.org
Subject: Re: `mike-port-encodings' branch (bug #29643)
Date: Wed, 16 Jun 2010 23:13:44 +0200	[thread overview]
Message-ID: <877hlyk7ef.fsf@gnu.org> (raw)
In-Reply-To: 604202.69520.qm@web37905.mail.mud.yahoo.com

Hi Mike,

Mike Gran <spk121@yahoo.com> writes:

>> Hmm, “simplification”?  :-)
>
>> Is this commit really needed?  What’s the operational effect?
>
> That commit has no net effect; however, the code
> uses the 'i' variable in a for loop as an iterator and then
> uses its value after the loop.  Using a for loop iterator
> variable after the loop is complete is bad style, IMHO.

Hmm right.  Looking again at this part, it looks like the new variable
names are indeed more descriptive than before, and that ‘i’ is not
reused throughout may help, so OK.

Please change the commit log from “cleanup” to something that briefly
describes the actual change, though.

>>>  +Returns, as a string, the character encoding that 
>>> @var{port} uses to interpret
>>>  +its input and output.  The value @code{#f} is equivalent to 
>>> @code{"ISO-8859-1"}.
>
>> Instead of having the #f special case, I’ve been 
>> thinking that it would be nicer and easier to have ‘port-encoding’ always 
>> return a string (similar for ‘set-port-encoding!’), which would be 
>> "ISO-8859-1" instead of #f.  Perhaps it’s a bit late, 
>> though.
>
>> What do you think?
>
> I have no problem with that.  But, there is some small optimization that
> comes from using NULL and SCM_BOOL_F as shorthand for ISO-8859-1,
> because is eliminates a strcmp operation before each block of port text
> is read or written.

Hmm, right.  Then forget about what I said.

However, it would be nice if the documentation would state the
equivalence of #f and ISO-8859-1 in a single place.

>> I dislike this whole notion of “binary-compatibility”.  What is meant
>> here is that for binary I/O, 
>> you’d better choose ISO-8859-1 as theport’s encoding because non-ASCII byte 
>> values will go through as is,right?  (I suppose that’s true of any 
>> 8-bit encoding.)
>
> In the documentation, (rnrs io ports) could be recommended for binary
> ports.  No problem.

OK, with an xref to that part of the doc right after the description of
the ‘b’ flag.  :-)

> But do you agree with the idea that Guile should force 8-bit encoding when
> a legacy binary port is opened?

Yes.

>   Or should it still inherit %default-port-encoding?  Which if those
> does not violate the 'principle of least surprise'?

ISO-8859-1 is probably best in that respect.

Thanks,
Ludo’.




      reply	other threads:[~2010-06-16 21:13 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <E1OL1A2-00071t-DR@vcs-noshell.in.savannah.gnu.org>
2010-06-08 22:29 ` `mike-port-encodings' branch (bug #29643) Ludovic Courtès
2010-06-10 18:16   ` Mike Gran
2010-06-16 21:13     ` Ludovic Courtès [this message]

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

  List information: https://www.gnu.org/software/guile/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877hlyk7ef.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=guile-devel@gnu.org \
    /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).