Re: bug in edit.m

[Ben Abbott]

On Thursday, January 17, 2008, at 02:31PM, "John W. Eaton" <address@hidden> 
wrote:
>On 16-Jan-2008, Ben Abbott wrote:
>
>| Please review and comment.
>
>OK.  Why did this thread suddenly move here from the bug list?

my bad. I made an improper inference from a comment you made in an email last 
week

>
>|      2008-01-16  Ben Abbott <address@hidden>
>| 
>|      * 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 "\").

ok

>  
>| +    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.

I'll be of little help with c/c++ ... if someone can take care of that end, I 
can work on the scripts.
 
>
>| +    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))

ok, I've added these to my list of preferred conventions for octave code.