[COMMIT] Import make-temp-name (the functionality of mkstemp(3)) from GNU

Stephen J. Turnbull stephen at xemacs.org
Wed Nov 14 17:15:51 EST 2007


QUERY

AFAICS your implementation is basically buggy.  Some suggestions for
alternative implementation below.

Aidan Kehoe writes:
 > 
 > Yes, the CODING-SYSTEM-OR-MUSTBENEW argument is ugly. No, I don't see a
 > better approach to this which would be compatible with GNU, given that our
 > CODING-SYSTEM argument was already in 21.4. 

Where is this argument used with MUSTBENEW semantics?  Wouldn't it be
better to keep the CODING-SYSTEM name and simply document the
additional semantics?  I don't think we should encourage use of this
misfeature in application code.

 > 	* code-files.el (write-region):
 > 	Provide a new arg, CODING-SYSTEM-OR-MUSTBENEW, for compatibility
 > 	both with GNU (where it has the MUSTBENEW meaning) and earlier
 > 	XEmacs code (where it has the CODING-SYSTEM meaning). 

For the sake of review, please be accurate.  This argument is not new,
it's just been given additional semantics.

 > 	* fileio.c (Fwrite_region_internal):
 > 	Take a new arg, MUSTBENEW, to error if the file to be written
 > 	already exists.

*gag*  It's just wrong to do this at that level.  Isn't there some way
to avoid this?  That function is already way overcomplicated.

As long as we're here ...

 >  Kludgy feature:

Wouldn't it be less work to mark the *clean* features of this monster?
:-(  How about something like

  Write current region into specified file.
  Called interactively, prompts for a file name.
  With a prefix arg, prompts for a coding system as well.

  When called from a program, takes three required arguments:
  START, END and FILENAME.  START and END are buffer positions.
  APPEND, if non-nil, means append to existing file contents (if any), else
    the file's existing contents are replaced by the specified region.
  VISIT, if non-nil, should be a string naming a file.  The buffer is marked
                   as  visiting VISIT.  VISIT is also the file name to lock
                   and unlock for clash detection.
  LOCKNAME, if non-nil, specifies the name to use for locking and unlocking,
    overriding FILENAME and VISIT.
  CODING-SYSTEM-OR-MUSTBENEW specifies the coding system used to encode the
    text written.  It defaults to the value of `buffer-file-coding-system'
    in the current buffer.

  For compatibility with GNU Emacs, several arguments are overloaded:
  START may be a string, which is written to the file.  END is ignored.
  VISIT may take the value t, meaning to set last-save-file-modtime of buffer
    to this file's modtime and mark buffer not modified.  With any other
    non-nil value of VISIT, suppress printing of the "Wrote file" message.
  CODING-SYSTEM-OR-MUSTBENEW may be a non-nil, non-coding-system value.
    If it is `excl' and FILENAME already exists, signal `file-already-exists'.
    Otherwise, if FILENAME already exists, ask for confirmation before
    writing, and signal `file-already-exists' if not confirmed.

  See also `write-region-pre-hook' and `write-region-post-hook'.

BTW, do you insist on 'excl (eg, for gagmacs compatibility)?  Really
I'd like to see t instead of 'excl, and restrict to 'ask for the query
confirmation.

 > Index: src/fileio.c
 > ===================================================================

 > +    if (!NILP (mustbenew) && !EQ (mustbenew, Qexcl))
 > +      barf_or_query_if_file_exists (filename, "overwrite", 1, NULL);

Isn't it cleaner to do

    if (!NILP (mustbenew))
      barf_or_query_if_file_exists (filename, "overwrite",
                                    EQ (mustbenew, Qexcl) ? 0 : 1, NULL);

 > @@ -3433,12 +3448,14 @@
 >    desc = -1;
 >    if (!NILP (append))
 >      {
 > -      desc = qxe_open (XSTRING_DATA (fn), O_WRONLY | OPEN_BINARY, 0);
 > +      desc = qxe_open (XSTRING_DATA (fn), O_WRONLY | OPEN_BINARY
 > +                       | (EQ (mustbenew, Qexcl) ? O_EXCL : 0), 0);

Note that on Mac OS X, at least, O_EXCL is documented to have no
effect unless O_CREAT is also specified.  Even if this does work on
some systems, the user will get a less specific `file-error' rather
than `file-already-exists'.

 >      }
 >    if (desc < 0)
 >      {
 >        desc = qxe_open (XSTRING_DATA (fn),
 > -		       O_WRONLY | O_TRUNC | O_CREAT | OPEN_BINARY,
 > +		       O_WRONLY | (EQ (mustbenew, Qexcl) ? O_EXCL : O_TRUNC)
 > +                       | O_CREAT | OPEN_BINARY,
 >  		       auto_saving ? auto_save_mode_bits : CREAT_MODE);
 >      }
 >  
 > @@ -4007,11 +4024,11 @@
 >      Fwrite_region_internal (Qnil, Qnil, a, Qnil, Qlambda, Qnil,
 >  #if 1 /* #### Kyle wants it changed to not use escape-quoted.  Think
 >  	 carefully about how this works. */
 > -	        	    Qescape_quoted
 > +	        	    Qescape_quoted,
 >  #else
 > -			    current_buffer->buffer_file_coding_system
 > +			    current_buffer->buffer_file_coding_system,
 >  #endif
 > -			    );
 > +			    Qnil);
 >  }
 >  
 >  static Lisp_Object
 > @@ -4367,6 +4384,7 @@
 >    DEFSYMBOL (Qverify_visited_file_modtime);
 >    DEFSYMBOL (Qset_visited_file_modtime);
 >    DEFSYMBOL (Qcar_less_than_car); /* Vomitous! */
 > +  DEFSYMBOL (Qexcl);
 >  
 >    DEFSYMBOL (Qauto_save_hook);
 >    DEFSYMBOL (Qauto_save_error);
 > 
 > -- 
 > On the quay of the little Black Sea port, where the rescued pair came once
 > more into contact with civilization, Dobrinton was bitten by a dog which was
 > assumed to be mad, though it may only have been indiscriminating. (Saki)
 > 
 > _______________________________________________
 > XEmacs-Patches mailing list
 > XEmacs-Patches at xemacs.org
 > http://calypso.tux.org/cgi-bin/mailman/listinfo/xemacs-patches



More information about the XEmacs-Patches mailing list