bug in edit.m

Ben Abbott bpabbott at mac.com
Sun Feb 17 21:39:53 CST 2008


On Jan 17, 2008, at 9:17 PM, Ben Abbott wrote:

> I made the changes, and attempted to include the functionality  
> regarding classnames ... although all in the m-file.
>
> The "classname" functionality of the attached patch may be broader  
> than intended. I'm searching the path for a sub-path/filename  
> combination ... so it is not restricted to classnames, but permits  
> all sub-paths.
>
> Essentially, I concatenate each of the following to each directory  
> in the path and check for existence.
>
> 	{"subpath/filename", "subpath/filename.m", "@subpath/filename",  
> "@subpath/filename.m"}
>
> The "path/subpath/filename" of the first positive occurrence is  
> returned.
>
> To facilitate the proposed change to the built-in  
> "file_in_loadpath", I created a place-hold function in edit.m called  
> "file_in_loadpath_local" which mocks what I understand to be the  
> desired behavior.
>
> Ben
>

I spent some time looking over this again ... and discovered some ...  
problems :-(

I won't call them "bugs", but ... let's say my contribution was  
"feature" rich ;-)

In any event, I've made another go of it.

Consistency with Matlab is no longer forced, but is configurable by  
adding the following lines to one of the startup scripts.

	edit editinplace true
	edit home .

The first forces all files to be open and edited in place, even when  
the user does not have write access to the file. The second results in  
all non-existing files being created in the pwd.

When the command "edit bar" is given a list of files is created which  
Octave's path will be searched for. In this instance that list is  
{"bar", "bar.m", "bar.cc"}.

When the command "edit foo/bar" is given a list of files is created  
which will Octave's path will be searched for. In this instance that  
list is {"foo/bar", "foo/bar.m", "foo/bar.cc". "@foo/bar", "@foo/ 
bar.m", "@foo/bar.cc"}.

When the file explicitly includes an extension, no other extensions  
are included in the search. For example, the command "edit bar.m" will  
search the path for the list {"foo/bar.m"} and "edit foo/bar.m" will  
search for the list {"foo/bar.m", "@foo/bar.m}.

When searching for the files, the entire path is searched for the fist  
file in the list, prior to proceeding to the next in the list (this  
appears to be different than how file_in_loadpath operates).

The built-in function file_in_loadpath(filestring) does not work if  
file string being searched for includes a partial path; ex: "foo/ 
bar.m". To overcome that I used file_in_path(path,filestring), which  
does respect the partial path information.

To be honest, I don't know the intended difference between  
file_in_loadpath(filestring), and file_in_path(path,filestring).  
Should these work differently?

In any event, I've attached an updated ChangeLog, and diff against the  
current mercurial sources ... for those who would like to shortcut the  
patching, I've also attached the complete script.

The final change is the ability to access the entire control structure  
by,

	s = edit get all
or
	s = edit ("get", "all");

Ben

ChangeLog:

	2008-01-16  Ben Abbott <bpabbott at mac.com>

	* miscellaneous/edit.m: Added control field "editinplace" to
	  optionally force all files to be edited in place (consistent
	  with Matalb). Explicitly gave preference to file list rather
	  than path list. Allow control structure to be returned when
	  "edit get all". Added support for file specifications including
	  partial path information.

Diff:

--- /Users/bpabbott/Development/mercurial/octave/scripts/miscellaneous/ 
edit.m	2008-02-16 16:47:33.000000000 -0500
+++ edit.m	2008-02-17 22:09:52.000000000 -0500
@@ -31,6 +31,10 @@
  ## that file is modifiable, then it will be edited in place.  If it
  ## is a system function, then it will first be copied to the directory
  ## @code{HOME} (see further down) and then edited.
+## If no file is found, then the m-file
+## variant, ending with ".m", will be considered. If still no file
+## is found, then variants with a leading "@@" and then with both a
+## leading "@@" and trailing ".m" will be considered.
  ##
  ## @item
  ## If @var{name} is the name of a function defined in the  
interpreter but
@@ -59,6 +63,9 @@
  ## the value of the control field @var{field} will be @var{value}.
  ## If an output argument is requested and the first argument is  
@code{get}
  ## then @code{edit} will return the value of the control field  
@var{field}.
+## If the control field does not exist, edit will return a structure
+## containing all fields and values. Thus, @code{edit get all} returns
+## a complete control structure.
  ## The following control fields are used:
  ##
  ## @table @samp
@@ -108,13 +115,17 @@
  ## Your own default copyright and license.
  ## @end table
  ##
+## Unless you specify @samp{pd}, edit will prepend the copyright  
statement
+## with "Copyright (C) yyyy Function Author".
+##
  ## @item mode
  ## This value determines whether the editor should be started in  
async mode
  ## or sync mode. Set it to "async" to start the editor in async  
mode. The
  ## default is "sync" (see also "system").
-##
-## Unless you specify @samp{pd}, edit will prepend the copyright  
statement
-## with "Copyright (C) yyyy Function Author".
+##
+## @item editinplace
+## Determines whether files should be edited in place, without regard  
to
+## whether they are modifiable or not. The default is @code{false}.
  ## @end table
  ## @end deftypefn

@@ -134,7 +145,8 @@
    				"AUTHOR", default_user(1),
    				"EMAIL",  [],
    				"LICENSE",  "GPL",
-				"MODE", "sync");
+  				"MODE", "sync",
+  				"EDITINPLACE", false);

    mlock; # make sure the state variables survive "clear functions"

@@ -159,8 +171,23 @@
        else
      	error('expected "edit MODE sync|async"');
        endif
+    case "EDITINPLACE"
+      if (ischar (state))
+        if (strcmpi (state, "true"))
+          state = true;
+        elseif (strcmpi (state, "false"))
+          state = false;
+        else
+          state = eval (state);
+        endif
+      endif
+      FUNCTION.EDITINPLACE = state;
      case "GET"
-      ret = FUNCTION.(toupper (state));
+      if (isfield (FUNCTION, toupper(state)))
+        ret = FUNCTION.(toupper (state));
+      else
+        ret = FUNCTION;
+      endif
      otherwise
        error ("expected \"edit EDITOR|HOME|AUTHOR|EMAIL|LICENSE|MODE  
val\"");
      endswitch
@@ -171,7 +198,7 @@
    if (nargin < 1)
      if (exist (FUNCTION.HOME, "dir") == 7 && (isunix () || ! ispc ()))
        system (strcat ("cd \"", FUNCTION.HOME, "\" ; ",
-		      sprintf (FUNCTION.EDITOR, "")),
+	      sprintf (FUNCTION.EDITOR, "")),
  	      [], FUNCTION.MODE);
      else
        system (sprintf (FUNCTION.EDITOR,""), [], FUNCTION.MODE);
@@ -185,48 +212,91 @@
        error ("unable to edit a built-in or compiled function");
    endswitch

-  ## Find file in path.
+  ## JWE has suggested that all the checks for whether the file is
+  ## absolute or relative should be handled inside file_in_loadpath.
+  ## That way, it will be possible to look up files correctly given
+  ## partial path information.  For example, you should be able to
+  ## edit a particular overloaded function by doing any one of
+  ##
+  ##   edit classname/foo
+  ##   edit classname/foo.m
+  ##   edit @classname/foo
+  ##   edit @classname/foo.m
+  ##
+  ## This functionality is needed for other functions as well (at least
+  ## help and type; there may be more).  So the place to fix that is in
+  ## file_in_loadpath, possibly with some help from the load_path  
class.
+
+  ## The code below includes a portion that serves as a place-holder  
for
+  ## JWE's suggested changes to file_in_loadpath the load_path class.
+
+  ## Create list of explicit and implicit file names.
+  filelist = {file};
+  ## If file has no extension, add file.m and file.cc to the list.
    idx = rindex (file, ".");
-  if (idx != 0)
-    ## If file has an extension, use it.
-    path = file_in_loadpath (file);
-  else
-    ## Otherwise try file.cc, and if that fails, default to file.m.
-    path = file_in_loadpath (strcat (file, ".cc"));
-    if (isempty (path))
-      file = strcat (file, ".m");
-      path = file_in_loadpath (file);
+  if (idx == 0)
+    ## Create the list of files to look for
+    filelist = {file};
+    if (isempty (regexp (file, "\\.m$")))
+      ## No ".m" at the end of the file, add to the list.
+      filelist{end+1} = cat (2, file, ".m");
+    endif
+    if (isempty (regexp (file, "\\.cc$")))
+      ## No ".cc" at the end of the file, add to the list.
+      filelist{end+1} = cat (2, file, ".cc");
      endif
    endif

-  ## If the file exists and is modifiable in place then edit it,
-  ## otherwise copy it and then edit it.
-  if (! isempty (path))
-    fid = fopen (path, "r+t");
-    if (fid < 0)
-      from = path;
-      path = strcat (FUNCTION.HOME, from (rindex (from, filesep):end))
-      [status, msg] = copyfile (from, path, 1)
-      if (status == 0)
-        error (msg);
-      endif
+  ## If the file includes a path, it may respect an overloaded  
function.
+  if (!(file=="@") && !isempty(find(file==filesep)))
+    ## No "@" at the beginning of the file, add to the list.
+    numfiles = numel(filelist);
+    for n = 1:numfiles
+      filelist{n+numfiles} = cat (2, "@", filelist{n});
+    endfor
+  endif
+
+  ## Search the entire path for the 1st instance of a file in the list.
+  fileandpath = "";
+  for n = 1:numel(filelist)
+    filetoedit = file_in_path (path, filelist{n});
+    if (! isempty(filetoedit))
+      ## The path is explicitly included.
+      fileandpath = filetoedit;
+      break;
+    endif
+  endfor
+
+  if (! isempty (fileandpath))
+    ## If the file exists, then edit it.
+    if (FUNCTION.EDITINPLACE)
+      ## Edit in place even if it is protected.
+      system (sprintf (FUNCTION.EDITOR, strcat ("\"", fileandpath,  
"\"")),
+              [], FUNCTION.MODE);
+      return;
      else
-      fclose(fid);
+      ## If the file is modifiable in place then edit it, otherwise  
make
+      ## a copy in HOME and then edit it.
+      fid = fopen (fileandpath, "r+t");
+      if (fid < 0)
+        from = fileandpath;
+        fileandpath = strcat (FUNCTION.HOME, from (rindex (from,  
filesep):end));
+        [status, msg] = copyfile (from, fileandpath, 1);
+        if (status == 0)
+          error (msg);
+        endif
+      else
+        fclose(fid);
+      endif
+      system (sprintf (FUNCTION.EDITOR, strcat ("\"", fileandpath,  
"\"")),
+              [], FUNCTION.MODE);
+      return;
      endif
-    system (sprintf (FUNCTION.EDITOR, strcat ("\"", path, "\"")),
-	    [], FUNCTION.MODE);
-    return;
    endif

-  file
-  ## If editing something other than a m-file or an oct-file, just
-  ## edit it.
-  idx = rindex (file, filesep);
-  if (idx != 0)
-    path = file;
-  else
-    path = fullfile (FUNCTION.HOME, file);
-  endif
+  ## If editing a new file that is neither a m-file or an oct-file,
+  ## just edit it.
+  fileandpath = file;
    idx = rindex (file, ".");
    name = file(1:idx-1);
    ext = file(idx+1:end);
@@ -234,7 +304,7 @@
      case { "cc", "m" }
        0;
      otherwise
-      system (sprintf (FUNCTION.EDITOR, strcat ("\"", path, "\"")),
+      system (sprintf (FUNCTION.EDITOR, strcat ("\"", fileandpath,  
"\"")),
  	      [], FUNCTION.MODE);
        return;
    endswitch
@@ -373,15 +443,15 @@
    endswitch

    ## Write the initial file (if there is anything to write)
-  fid = fopen (path, "wt");
+  fid = fopen (fileandpath, "wt");
    if (fid < 0)
-    error ("edit: could not create %s", path);
+    error ("edit: could not create %s", fileandpath);
    endif
    fputs (fid, text);
    fclose (fid);

    ## Finally we are ready to edit it!
-  system (sprintf (FUNCTION.EDITOR, strcat ("\"", path, "\"")),
+  system (sprintf (FUNCTION.EDITOR, strcat ("\"", fileandpath, "\"")),
  	  [], FUNCTION.MODE);

  endfunction
@@ -425,3 +495,38 @@
    endif

  endfunction
+
+%!test
+%! s.editor = edit get editor;
+%! s.home = edit get home;
+%! s.author = edit get author;
+%! s.email = edit get email;
+%! s.license = edit get license;
+%! s.editinplace = edit get editinplace;
+%! s.mode = edit get mode;
+%! edit editor none
+%! edit home none
+%! edit author none
+%! edit email none
+%! edit license none
+%! edit ("editinplace", !s.editinplace)
+%! if (s.mode(1) == "a")
+%!   edit mode sync
+%! else
+%!   edit mode async
+%! endif
+%! edit ("editor", s.editor);
+%! edit ("home", s.home);
+%! edit ("author", s.author);
+%! edit ("email", s.email);
+%! edit ("license", s.license);
+%! edit ("editinplace", s.editinplace);
+%! edit ("mode", s.mode);
+%! assert (edit ("get", "editor"), s.editor);
+%! assert (edit ("get", "home"), s.home);
+%! assert (edit ("get", "author"), s.author);
+%! assert (edit ("get", "email"), s.email);
+%! assert (edit ("get", "license"), s.license);
+%! assert (edit ("get", "editinplace"), s.editinplace);
+%! assert (edit ("get", "mode"), s.mode);
+









-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://www.cae.wisc.edu/pipermail/octave-maintainers/attachments/20080217/471d5a52/attachment-0003.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: edit.diff
Type: application/octet-stream
Size: 9459 bytes
Desc: not available
Url : https://www.cae.wisc.edu/pipermail/octave-maintainers/attachments/20080217/471d5a52/attachment-0002.obj 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://www.cae.wisc.edu/pipermail/octave-maintainers/attachments/20080217/471d5a52/attachment-0004.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: edit.m
Type: application/octet-stream
Size: 17953 bytes
Desc: not available
Url : https://www.cae.wisc.edu/pipermail/octave-maintainers/attachments/20080217/471d5a52/attachment-0003.obj 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://www.cae.wisc.edu/pipermail/octave-maintainers/attachments/20080217/471d5a52/attachment-0005.html 


More information about the Octave-maintainers mailing list