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

 
 

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

Submitted by:  None
Submitted on:  Mon 27 Jan 2014 04:46:19 PM UTC  
 
Category: Octave FunctionSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Matlab Compatibility
Status: FixedAssigned to: None
Originator Name: joergBOriginator Email: -unavailable-
Open/Closed: ClosedRelease: 3.8.0
Operating System: Microsoft Windows

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission 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>
Project Administrator
Thu 30 Jan 2014 12:27:54 AM UTC, comment #17:

Pushed for CLI: http://hg.savannah.gnu.org/hgweb/octave/rev/073fbe7e6a3a

John Donoghue <lostbard>
Project Member
Wed 29 Jan 2014 06:41:04 PM UTC, comment #16:

Patch for CLI uigetfile

(file #30417)

John Donoghue <lostbard>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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

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:

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>
Project Administrator
Tue 28 Jan 2014 04:50:30 PM UTC, comment #7:

And the patch for the GUI ...

(file #30395)

John Donoghue <lostbard>
Project 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.

John Donoghue <lostbard>
Project 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>
Project 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>
Project 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>
Project 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 File(s):
   
   
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
  • -unavailable- added by lostbard (Posted a comment)
  • -unavailable- added by fgnievinski (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by None (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 10 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 31 Jan 2014 04:52:29 AM UTCrik5StatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Thu 30 Jan 2014 12:27:54 AM UTClostbardStatusPatch Submitted=>Ready For Test
    Wed 29 Jan 2014 06:41:04 PM UTClostbardAttached File-=>Added fltkgetfile.patch, #30417
      StatusConfirmed=>Patch Submitted
    Wed 29 Jan 2014 03:54:16 PM UTCrik5StatusPatch 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
    Tue 28 Jan 2014 05:26:50 PM UTCrik5StatusNone=>Patch Submitted
    Tue 28 Jan 2014 04:50:30 PM UTClostbardAttached File-=>Added uigetfile.patch, #30395

    Back to the top


    Powered by Savane 3.1-cleanup1