Re: standard output/error/input streams

[Eli Zaretskii]

> From: address@hidden (Phillip Lord)
> Date: Sat, 14 Jan 2017 12:22:50 +0000
> 
> I've worked up the patch that I made to add input streams for writing to
> the system standard out. There was some discussion about this in Jul.
> 
> https://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00910.html
> 
> The new version is on feature/stdout-stderr-stream.

Thanks.  Some of the concerns I expressed here:

  https://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00938.html

are still relevant, and at the very least should be reflected in the
docs.

> Can I add this to master?

I think this still "needs work", see my comments below:

  address@hidden @code{external-standard-input}
  address@hidden external-standard-input &optional char

No need to explicitly index a function that is introduced in a @defun:
the latter automatically adds an index entry.

  +This function reads a single character from the system standard input
  +(as opposed to @var{standard-input}) and functions as a stream. Note,

Two spaces between sentences, please (here and elsewhere).

  +however, that if Emacs is running in a terminal its use can be
  +unpredictable.

I think we should explain more about the issue here, so that the
reader understands what she is up against.  (I think this function
will simply not work on a TTY frame.)

  +These functions are predominately useful for debugging, as they are a
  +mechanism for producing output that does not change any buffer. Note,
  +however, that if Emacs is running in a terminal their use can affect
  +the display unpredictably.

Likewise here.  This should also mention the problems with
stdout/stderr disposition in GUI sessions.

  +(defvar external-standard-input-pushback nil
  +  "Pushback character for `external-standard-input'.")

Isn't this an internal variable?  If so, it should use double hyphen
in its name.  And what is the purpose of this pushback?  I didn't see
any code that sets it in patch.

  +DEFUN 
("external-standard-input-read-char",Fexternal_standard_input_read_char, 
Sexternal_standard_input_read_char, 0, 0, 0,
  +       doc: /* Read a single character from the system standard input.
  +
  +Returns -1 if standard input is at the end.*/)
  +     (void)
  +{
  +  int c;
  +  Lisp_Object val;
  +
  +  c = getchar();
  +  XSETINT(val,c);
  +
  +  return val;
  +}

This implementation has a number of issues:

  . getchar reads a _byte_, not a character, so unless input is
    plain-ASCII, what you return here is a raw byte, not a character
    in the Emacs sense of that word.  That is inconsistent with every
    other input facility we have, and could very well get the user in
    trouble.

  . getchar doesn't return until you type RET, at least on my system,
    which might come as a surprise to users.

  . (nitpicking) our coding style keeps one space between the
    function/macro name and the opening parenthesis.  (Same issue
    exists elsewhere in the patch.)

  +DEFUN ("external-standard-input-read-line", 
Fexternal_standard_input_read_line, Sexternal_standard_input_read_line, 0, 0, 0,
  +      doc: /* Read a line from the system standard input.*/)

This function doesn't seem to be documented in the ELisp manual.

  +  if (len || c == '\n' || c == '\r')
  +    {
  +      val = make_string (line, len);
  +      xfree (line);

What about EOL decoding, as in all the other Emacs input functions?

Also, make_string has its own ideas about when to produce unibyte
strings and when multibyte strings.  You should instead decode the
input text using coding-system-for-read (if non-nil) or
locale-coding-system (or maybe what terminal-coding-system returns).

  +DEFUN ("external-standard-output", Fexternal_standard_output, 
Sexternal_standard_output, 1, 1, 0,
  +       doc: /* Output character CHARACTER to system standard output. */)
  +     (Lisp_Object character)
  +{
  +  CHECK_NUMBER (character);
  +  printchar_to_stream (XINT(character), stdout);

printchar_to_stream converts the output text via
standard-display-table, if that is non-nil; do we really want that for
functionality that is supposed to be a debugging aid?  It also sends
its argument to the debugger on MS-Windows -- is that desirable?