bug in edit.m

[Ben Abbott]

On Jan 17, 2008, at 2:31 PM, John W. Eaton wrote:

> 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))
>

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: edit.patch
Type: application/octet-stream
Size: 5548 bytes
Desc: not available
Url : https://www.cae.wisc.edu/pipermail/octave-maintainers/attachments/20080117/7bf1c7b9/attachment-0001.obj 
-------------- next part --------------