bug in edit.m

[John W. Eaton]

On 16-Jan-2008, Ben Abbott wrote:

| Please review and comment.

OK.  Why did this thread suddenly move here from the bug list?

| 	2008-01-16  Ben Abbott <bpabbott at mac.com>
| 
| 	* miscellaneous/edit.m: Changed behavior to be consistent with Matlab.
| 	Open all existing files in place. New files are opened in the pwd.

| -  ## Find file in path.
| -  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 no path info, and no ".m" look for both the file and file+".m"
| +  if !(any (strfind (file, filesep)) || \
| +      (any (strfind (file, ".")) && isempty (regexp (file, "\\.m$"))))

Please write multi-line conditions like this:

  if (! (any (strfind (file, filesep))
	 || (any (strfind (file, "."))
	     && isempty (regexp (file, "\\.m$")))))

The Emacs Octave mode will help with the indentation in cases like
this.

Note that by including parens around the whole condition, you don't
need a continuation character.

If you do need a continuation character, then please prefer "..." over
"\", since "\" has another meaning (I admit that it was a bad choice
to overload "\").
  
| +    files = {file};
| +    if (isempty (regexp (file, "\\.m$")))
| +      ## If the name is not explicity an m-file, create a list with  
| each.
| +      files{2} = strcat (file, ".m");
| +    endif
| +    ## File without the ".m" exists, it has precidence over the ".m"  
| version.
| +    file_to_edit = file_in_loadpath (files);

I think the rules need to be slightly different here, and 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

We need this functionality 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.

| +    if (numel (file_to_edit))

I think it is marginally clearer to write

  if (! isempty (file_to_edit))

for things like this.

| +  if exist (file)

To match the convention used in the rest of Octave, please write

  if (exist (file))


Thanks,

jwe