bugGNU Octave - Bugs: bug #54463, Creating a new .m file with full...

 
 

bug #54463: Creating a new .m file with full path shows popup with full file path appended to current directory

Submitted by:  Philip Nienhuis <philipnienhuis>
Submitted on:  Wed 08 Aug 2018 01:54:24 PM UTC  
 
Category:  GUI Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  philipnienhuis Open/Closed:  Closed
Release:  4.4.1-rc2 Operating System:  Any

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)

Wed 15 Aug 2018 05:03:13 AM UTC, comment #25:

Philip, thanks for testing. Closing report.

Torsten <ttl>
Project Member
Sun 12 Aug 2018 12:27:22 AM UTC, comment #24:

works fine on Windows

Philip Nienhuis <philipnienhuis>
Project Member
Sat 11 Aug 2018 08:12:44 PM UTC, comment #23:

Changeset
http://hg.savannah.gnu.org/hgweb/octave/rev/e222da9beaac
should fix this issue.

octave::sys::env::make_absolute turned out to be quite useful here.

Torsten <ttl>
Project Member
Sat 11 Aug 2018 05:47:36 AM UTC, comment #22:

I will update the patch using octave::sys::env::absolute_pathname.

Torsten <ttl>
Project Member
Fri 10 Aug 2018 07:37:52 PM UTC, comment #21:

Thanks for the explanation, Mike. I did catch your point about exposure.

Your reasoning makes sense from an Octave POV, as much as my question does from a Qt POV. But then yes, we're Octave developers, not Qt ....

Philip Nienhuis <philipnienhuis>
Project Member
Fri 10 Aug 2018 07:27:41 PM UTC, comment #20:

Philip - my point wasn't that this would fix an equivalent bug in the edit function in the console version of Octave at all.

My point was only that other parts of Octave use the octave::sys::env::absolute_pathname function for other limited purposes. If we use it in the GUI as well, it exposes the function to more users and may help identify latent bugs or other problems with the function.

Same thing with functions having to do with system time, or temporary files, or network access. If we use the Qt functions for all of these, then the liboctave functions don't get exercised as much.

Mike Miller <mtmiller>
Project Administrator
Fri 10 Aug 2018 07:17:54 PM UTC, comment #19:

In any event Torsten's path from comment #15 works fine in Linux and in Windows 10.

I can see Mike's reasoning - in principle this bug isn't confined to the GUI (although it was noted in a GUI popup).
But I just tried in a CLI on Windows and I see no bug there. Notepad++ opens the file in the right place (after a few hickups with some LoadLibrary plugin) and I see no mention anywhere of a wrong path.
(note: I didn't try in Linux)

So do we still need to use an internal function to fix a nonexistent, or maybe still concealed, bug in the CLI while a working Qt-based fix for the GUI is available?

Philip Nienhuis <philipnienhuis>
Project Member
Fri 10 Aug 2018 05:57:41 PM UTC, comment #18:

I don't think we have a policy or a definitive answer, but I would want to use our own liboctave portability functions as much as possible.

Since the interpreter doesn't link with Qt, and needs to rely on our own system portability functions, I would think we might as well exercise them and use them consistently from the libgui directory as well. It provides more test cases to ensure liboctave is working correctly.

Mike Miller <mtmiller>
Project Administrator
Fri 10 Aug 2018 05:32:19 PM UTC, comment #17:

@Mike: I have used the corresponding method of Qt for the patch (QFileInfo::absoluteFilePath ()). Do you think we should prefer internal octave functions?

Torsten <ttl>
Project Member
Fri 10 Aug 2018 04:21:04 PM UTC, comment #16:

You might find the liboctave function octave::sys::env::absolute_pathname useful here.

This is the library function behind the interpreter function is_absolute_filename.

Mike Miller <mtmiller>
Project Administrator
Fri 10 Aug 2018 03:48:36 PM UTC, comment #15:

QFileInfo provides methods for testing on absolute paths. The attached patch should work on windows (although I have only got the possibility to test on Linux).

(file #44761)

Torsten <ttl>
Project Member
Fri 10 Aug 2018 12:22:38 PM UTC, comment #14:

@Marco;
yes "regular" relative paths work fine. I was thinking more of relative paths on another drive, but that doesn't work properly anyway in Octave (MSYS). That MS-DOS legacy (which still works in Win 10) was complicated enough and many people simply don't know about it. Maybe better so :-)

A full path on Windows starts with:
- one letter (case-insensitive) followed by a semicolon
- \\<Server name>\ (maybe //<server name>/ also works)
- file:///
- maybe even http:// or ftp://
so there is some choice....

I wouldn't be surprised if network paths and UNC paths don't work anyway.
If only the first, maybe the first two, in te list above would work the goal is achieved.

Philip Nienhuis <philipnienhuis>
Project Member
Fri 10 Aug 2018 09:21:25 AM UTC, comment #13:

Windows full paths do not start with a separator. The patch cannot work on Windows systems. What is a full path in Windows? Something whose second character is a colon? And on MacOS?

Marco Caliari <caliari>
Project Member
Fri 10 Aug 2018 08:10:01 AM UTC, comment #12:

The attachment (a simple diff) seems to solve the problem, please check and try.

(file #44759)

Marco Caliari <caliari>
Project Member
Fri 10 Aug 2018 07:53:51 AM UTC, comment #11:

@Philip: when I use a relative path (i.e., no separator as first character), the popup window shows the right full path (current directory + separator + relative path).

Marco Caliari <caliari>
Project Member
Thu 09 Aug 2018 02:15:52 PM UTC, comment #10:

We should also try with relative paths. I've only tried with full paths.

Philip Nienhuis <philipnienhuis>
Project Member
Thu 09 Aug 2018 09:06:13 AM UTC, comment #9:

@Torsten: In my test on Linux, the file was created in the directory given in the edit command.

The problem is in libgui/src/octave-qt-link.cc, around line 112: QDir::currentPath () is always prepended. It should not when QString::fromStdString (file) starts with a separator.

Marco Caliari <caliari>
Project Member
Wed 08 Aug 2018 06:58:58 PM UTC, comment #8:

@Philip: In my test on Linux, the file was created in the current directory, not the one given in the edit command.

Torsten <ttl>
Project Member
Wed 08 Aug 2018 06:47:39 PM UTC, comment #7:

@Torsten:
To clarify: I often have .m files / functions for a project in a subdir m-files/ while the current working directory could be model/ or data/ or whatever.
So if I want to create a new function I want it to live in m-files/, and it is quite clumsy to first have to navigate there to be able to just create it. That's why I enter relative or full paths to functions with the edit command.

FYI Matlab allows this so the Item Group could also be "Matlab Compatibility", be it that Octave already does the right thing, it's just the popup text that is wrong.

Philip Nienhuis <philipnienhuis>
Project Member
Wed 08 Aug 2018 05:31:37 PM UTC, comment #6:

Torsten - yes, the edit function is intended to be called with either a relative or an absolute file name.

Mike Miller <mtmiller>
Project Administrator
Wed 08 Aug 2018 05:19:23 PM UTC, comment #5:

When looking at the documentation of 'edit', I am wondering if 'edit' is really intended to be called αΊƒith (absolute) paths and not only the file name. If it is, I can provide a patch to fix the issue.

Torsten <ttl>
Project Member
Wed 08 Aug 2018 04:20:45 PM UTC, comment #4:

@Philip: I see an analogous problem, a concatenation of current and destination folder. I change to Any Operating System.

Marco Caliari <caliari>
Project Member
Wed 08 Aug 2018 03:54:32 PM UTC, comment #3:

@Marco:
What did the popup text on your Linux system look like?

Philip Nienhuis <philipnienhuis>
Project Member
Wed 08 Aug 2018 03:15:00 PM UTC, comment #2:

Sorry, I used 4.2.2. 4.4.1-rc2 is fine.

Marco Caliari <caliari>
Project Member
Wed 08 Aug 2018 03:06:55 PM UTC, comment #1:

@Philip: and what about the proposed function name? If I do it in linux, edit opens emacs with a file

The proposed function name is not valid.

Marco Caliari <caliari>
Project Member
Wed 08 Aug 2018 01:54:24 PM UTC, original submission:

(Minor cosmetic bug.)
Steps to reproduce:

1. Go to some directory, say 'C:\temp'

2. In the terminal: edit d:/tmp/newmfile.m
(assuming d:/tmp exists)

Result: a pop-up saying:

File
c:/temp\d:/temp/newmfile.m
does not exist. Do you want to create it?

Nevertheless, clicking the "Create" button creates the file in the right place (d:/tmp).
So it's just that the pop-up filename processing needs a little attention.

Philip Nienhuis <philipnienhuis>
Project Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #44761:  bug54463_ttl_v01.patch added by ttl (2KiB - text/x-patch)
file #44759:  octave-qt-link.cc.diff added by caliari (2KiB - application/x-tex)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by ttl (Posted a comment)
  • -email is unavailable- added by caliari (Posted a comment)
  • -email is unavailable- added by philipnienhuis (Submitted the item)
  •  

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

    Only project members can vote.

     

     

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-08-15 ttl StatusReady For Test => Fixed
        Open/ClosedOpen => Closed
    2018-08-11 ttl StatusIn Progress => Ready For Test
    2018-08-10 ttl Attached File- => Added bug54463_ttl_v01.patch, #44761
    2018-08-10 caliari Attached File- => Added octave-qt-link.cc.diff, #44759
        StatusNone => In Progress
    2018-08-08 caliari Operating SystemMicrosoft Windows => Any

    Back to the top


    Powered by Savane 3.3