octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

patch: extension to save


From: John W. Eaton
Subject: patch: extension to save
Date: Mon, 11 Feb 2008 14:17:33 -0500

On 11-Feb-2008, Jaroslav Hajek wrote:

| hello,
| 
| attached is a patch (against the "development" Octave source tarball
| <ftp://ftp.octave.org/pub/octave/bleeding-edge/octave-3.0.0.tar.gz>)
| to implement -struct and -regexp modifiers in the builtin save,
| according to Matlab.
| 
| save [options] file -regex [pat1 pat2 ...]
| uses regexing instead of globbing for the patterns
| 
| save [options] file -struct struct_name [fld1 fld2 ...]
| saves the field of a scalar structure sruct_name as top-level variables
| (this is the inverse operation to load with an output argument).
| 
| I've added inline documentation for the new features and included a
| simple test of
| the features into test/test_io.m.
| The patch is made with diff -cp as advised by Octave's manual, for
| each pair of files and
| simply concatenated together (I'm not sure if that's the right thing
| to do, so please correct me if not).
| 
| Patched files:
| src/load-save.cc
| src/symtab.cc
| src/symtab.h
| test/test_io.m
| 
| any further comments or suggestions are welcome.

Your patch seems to be reversed.  You should make diffs 

Also, the symbol table code was completely rewritten after 3.0.0, so
before we can include your changes, they will need to be adapted for
the new symbol table code.  To get the current sources, please check
out a copy from the new Mercurial archive here:

  http://www.gnu.org/software/octave/download.html

The src/DLD-FUNCTIONS/regexp.cc file uses

  #if defined (HAVE_PCRE)
  #include <pcre.h>
  #elif defined (HAVE_REGEX)
  #if defined (__MINGW32__)
  #define __restrict
  #endif
  #if defined (HAVE_SYS_TYPES_H)
  #include <sys/types.h>
  #endif
  #include <regex.h>
  #endif

to get regexp headers, but your code uses only <regex.h>.  I think we
should probably always include and use the regex headers in the same
way wherever they are used in Octave.  Perhaps we should even wrap the
functionality in a class.  But at the very least, I think we should
introduce a new header file in Octave that hides the details of which
regex header files to include. 

Also, please try to follow the coding style of the existing code in
Octave when submitting changes.

Thanks,

jwe


reply via email to

[Prev in Thread] Current Thread [Next in Thread]