bugGNU Octave - Bugs: bug #41367, CLI version of uigetfile uses...

 
 

bug #41367: CLI version of uigetfile uses incorrect file separator on Windows

Submitter:  None
Submitted:  Mon 27 Jan 2014 04:46:19 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  joergB Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 3.8.0
Operating System:  * Microsoft Windows Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 31 Jan 2014 04:52:29 AM UTC, comment #18: 

Finally got this compiled on MXE and it works just fine.  Closing report.

Rik <rik5>
Group administrator
Thu 30 Jan 2014 12:27:54 AM UTC, comment #17: 
John Donoghue <lostbard>
Group Member
Wed 29 Jan 2014 06:41:04 PM UTC, comment #16: 

Patch for CLI uigetfile

(file #30417)

John Donoghue <lostbard>
Group Member
Wed 29 Jan 2014 03:57:21 PM UTC, comment #15: 

ok - I have a patch for it based on comment 8 - I will attach after checking it works.

John Donoghue <lostbard>
Group Member
Wed 29 Jan 2014 03:54:16 PM UTC, comment #14: 

I'm re-titling the bug report to reflect the remaining open issue: CLI version of uigetfile returns the wrong file separator on Windows.

Rik <rik5>
Group administrator
Wed 29 Jan 2014 03:13:55 PM UTC, comment #13: 

uigetfile in Matlab 2012a under linux appends the separator
uigetfile in Matlab 2012a under windows appends the separator

but, as mentioned by Rik before
uigetdir in Matlab 2012a under linux doesn't append the separator
uigetdir in Matlab 2012a under windows doesn't append the separator

Anonymous
Wed 29 Jan 2014 03:06:17 PM UTC, comment #12: 

OK, thanks for checking.

John W. Eaton <jwe>
Group administrator
Wed 29 Jan 2014 02:54:08 PM UTC, comment #11: 

Linux version of matlab:
R2010b: appends the separator
R2013a: appends the separator

John Donoghue <lostbard>
Group Member
Wed 29 Jan 2014 02:29:24 PM UTC, comment #10: 

We seem to have conflicting reports about what Matlab does.

I'd prefer to only append the directory separator if required for Matlab compatibility.  What does it do?  Does the behavior depend on the Matlab version?

John W. Eaton <jwe>
Group administrator
Wed 29 Jan 2014 11:19:16 AM UTC, comment #9: 

Pushed patch for appending the separator;  http://hg.savannah.gnu.org/hgweb/octave/rev/6ebd37faae5f

John Donoghue <lostbard>
Group Member
Tue 28 Jan 2014 05:26:50 PM UTC, comment #8: 

This is a minor compatibility issue.  I would place it on the gui-release branch rather than the stable branch.

From uigetfile.m I see


  if (__octave_link_enabled__ ())
    [retfile, retpath, retindex] = __octave_link_file_dialog__ (outargs{:});
  else
    [retfile, retpath, retindex] = feval (funcname, outargs{:});
  endif


In the CLI version, uigetfile eventually calls out to _fltk_uigetfile_.cc which is in libinterp/dldfcn.

In the C++ code for _fltk_uigetfile_ I see this:


//fltk uses forward slash even for windows
std::string sep = "/";
...
retval(1) = std::string (fc.directory ()) + sep;


So that would explain things for the CLI interface.

It seems like it would be simple enough to add either a regexprep call after the feval in the m-file (regexprep (retpath, '/', filesep), or to add the search and replace code in C++ to _fltk_uigetfile_ using file_ops::dir_sep_char (), string.find, and string.replace calls.

This seems like a useful enough function that the best option would be to add the equivalent of QDir::toNativeSeparators function to Octave's own liboctave library.  The class to change is file_ops which lives in liboctave/system/file_ops.[h|cc].  The function would probably need to go through the path character by character so that it would not replace instances of dir_sep_char that had been escaped.






Rik <rik5>
Group administrator
Tue 28 Jan 2014 04:50:30 PM UTC, comment #7: 

And the patch for the GUI ...

(file #30395)

John Donoghue <lostbard>
Group Member
Tue 28 Jan 2014 04:49:20 PM UTC, comment #6: 

Ive fixed the GUI in this patch.
Currrently it also then converts the slashes to their OS type, which is what pwd returns.

I will push it if ok ... stable or gui?

A related problem is that the non gui version (in windows) returns the / separator for the path names which then has issues  with creating file names with fullpath and using ls.


octave:1> [a,b,c] = uigetfile
a = testfile.m
b = C:/Users/jdonoghue/
c =  1
octave:2> d = fullfile(b,a)
d = C:/Users/jdonoghue/\testfile.m
octave:3> ls(d)
Invalid switch - "Users".
error: ls: command exited abnormally with status 127
octave:3>



John Donoghue <lostbard>
Group Member
Tue 28 Jan 2014 08:56:02 AM UTC, comment #5: 

@ Rik, I was indeed using the GUI version of Octave.

Anonymous
Mon 27 Jan 2014 08:29:20 PM UTC, comment #4: 

@Rik: Matlab's uigetdir does not append a dir separator.

Felipe G. Nievinski <fgnievinski>
Mon 27 Jan 2014 05:46:18 PM UTC, comment #3: 

Since you have access to Matlab, how does it handle uigetdir?  It seems like it should append the directory separator character in this case to indicate that this is a folder, not a file.

Rik <rik5>
Group administrator
Mon 27 Jan 2014 05:24:41 PM UTC, comment #2: 

Were you running the GUI or the CLI when you called uigetfile()?

In the CLI version the output separator is appended, in the GUI version it is not, according to my tests.

Rik <rik5>
Group administrator
Mon 27 Jan 2014 05:02:51 PM UTC, comment #1: 

You could also use fullfile (dir, file) to concatenate the files.

John W. Eaton <jwe>
Group administrator
Mon 27 Jan 2014 04:46:19 PM UTC, original submission:  

Dear all,

the matlab uigetfile appends the folder separator to the returned path, like for example
retfile = myfile.m
retpath = /home/myname/
The Octave uigetfile however omits the last slash and returns
retfile = myfile.m
retpath = /home/myname

I kinda like the way matlab does it, because I can, after selecting a file with uigetfile, simply open it by for example
dlmread([retpath retfile]) without having to insert a (OS-specific) folder separator

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #30417:  fltkgetfile.patch added by lostbard (2KiB - application/octet-stream)
file #30395:  uigetfile.patch added by lostbard (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by lostbard (Posted a comment)
  • -email is unavailable- added by fgnievinski (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-01-31 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2014-01-30 lostbard StatusPatch Submitted Ready For Test
    2014-01-29 lostbard Attached File- Added fltkgetfile.patch, #30417
        StatusConfirmed Patch Submitted
    2014-01-29 rik5 StatusPatch Submitted Confirmed
        Operating SystemGNU/Linux Microsoft Windows
        Summaryappend folder separator in second output of uigetfile CLI version of uigetfile uses incorrect file separator on Windows
    2014-01-28 rik5 StatusNone Patch Submitted
    2014-01-28 lostbard Attached File- Added uigetfile.patch, #30395

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code