bugGNU Octave - Bugs: bug #55855, GUI configuration files are not...

 
 

bug #55855: GUI configuration files are not located following each platform's convention

Submitter:  Torsten Lilge <ttl>
Submitted:  Thu 07 Mar 2019 04:51:20 PM UTC
   
 
Category:  GUI Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Feature Request
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 02 Apr 2019 09:24:55 PM UTC, comment #20: 

Thanks, finally got around to testing again and it looks like this works perfectly now.

Mike Miller <mtmiller>
Group Member
Sun 31 Mar 2019 06:25:48 PM UTC, comment #19: 

Closing this report. It can be reopened if issues are showing up later.

Torsten Lilge <ttl>
Group Member
Fri 22 Mar 2019 05:43:06 PM UTC, comment #18: 
Torsten Lilge <ttl>
Group Member
Fri 22 Mar 2019 06:07:26 AM UTC, comment #17: 

Oops, this was not intended. The XDG_CONFIG_HOME check was commented out for various tests because my patch was initially not working on windows and I just forgot this before pushing.

I will prepare a patch.

Torsten Lilge <ttl>
Group Member
Thu 21 Mar 2019 11:27:53 PM UTC, comment #16: 

This works well for me.

Is there a reason you changed the check so that it does copy old settings if XDG_CONFIG_HOME is set? I thought your explanation in comment #12 made sense at the time.

Now if I run with XDG_CONFIG_HOME set to an empty directory, the welcome wizard does not show and the settings from my ~/.config/qt-settings file are copied over.

Other than that, I think this is working.

Mike Miller <mtmiller>
Group Member
Wed 20 Mar 2019 07:54:09 PM UTC, comment #15: 

I have tested the updated patch on Debian/Gnome and Windows 7 and pushed it with cset http://hg.savannah.gnu.org/hgweb/octave/rev/9e2f69c1a05d


Torsten Lilge <ttl>
Group Member
Sat 16 Mar 2019 10:32:13 PM UTC, comment #14: 

Now I see your point. Since the settings file does not yet exist at the new location, I see no (obvious) reason against copying a valid ini file to that location. However, I agree that we should avoid any risk and will implement the proposed loop for rewriting of all keys from the old location.

Torsten Lilge <ttl>
Group Member
Sat 16 Mar 2019 09:06:45 PM UTC, comment #13: 

Ok, I see I was reading the XDG_CONFIG_HOME thing backwards. I think it would help to comment that, something like "Look for settings in the legacy qt-settings file, but only if XDG_CONFIG_HOME is not set". That would help explain why settings may not migrate for some particular user.

For the copying keys, I was really looking for something like "m_default_settings->merge (other_qsettings_object)", but there doesn't seem to be any method for copying or merging one QSettings into another. I thought that would be safer than directly editing the file on disk. How safe is QSettings when the .ini file is changed under it?

Mike Miller <mtmiller>
Group Member
Sat 16 Mar 2019 08:28:41 AM UTC, comment #12: 

Mike, thanks for reviewing.

  • The check for XDG_CONFIG_HOME is not done for finding old settings. Instead, the old settings are only copied to the new location if XDG_CONFIG_HOME is not set. If XDG_CONFIG_HOME is set, a non existing new settings might be intended and is not necessarily caused by the first run of octave with the new settings location.


  • Regarding your pseudo code: Why writing the settings key by key instead of just copying the file?
Torsten Lilge <ttl>
Group Member
Thu 14 Mar 2019 01:21:02 AM UTC, comment #11: 

I haven't tested this patch yet, but I looked it over, here are a few suggestions.

  • When you check for the old_settings_file, I don't think you need to refer to XDG_CONFIG_HOME, since that was only done recently on default, just look in home_path + "/.config/octave".


  • I don't think we need a new prompt here when migrating old settings to new settings. Instead of extra code to try to copy the old file on top of the new one, what about (pseudocode):



m_default_settings = new QSettings (...).
m_settings_file = m_default_settings->fileName ();

if (! exists m_settings_file)
  old_settings_file = home + "/.config/octave/qt-settings";
  if (exists old_settings_file)
    QSettings old_settings (old_settings_file, IniFormat);
    for (key : old_settings.allKeys ())
      m_default_settings->setValue (key, old_settings.value (key));
    endfor
  endif
endif


With that kind of approach, I don't think it's necessary to prompt whether old settings should be read, or show a warning if old settings can't be read, just move on and start with defaults if the old file doesn't exist or can't be read for some reason.

Mike Miller <mtmiller>
Group Member
Mon 11 Mar 2019 08:47:44 PM UTC, comment #10: 

I have attached a first patch for the new location of the gui preferences file for testing.

Organization and application name are directly passed to the QSettings constructor. Therefore, no names set in the main window have to be changed.

(file #46497)

Torsten Lilge <ttl>
Group Member
Sun 10 Mar 2019 08:38:15 PM UTC, comment #9: 

When changing the configuration file location, there should be the choice for the user to copy the existing file from the old location to the new location. Should this always be asked when the new location does not exist (and the old one does) or only if the new file does not exists and XDG_CONFIG_HOME is not set?

Torsten Lilge <ttl>
Group Member
Fri 08 Mar 2019 08:12:42 PM UTC, comment #8: 

QStandardPaths::CacheLocation sounds good. The version number is required since the compiled keyword and function lists depend on octave's version.

Torsten Lilge <ttl>
Group Member
Fri 08 Mar 2019 07:34:33 PM UTC, comment #7: 

You might have to change the setApplicationName in libgui/src/main-window.cc to get what you want out of QStandardPaths. And I don't know if that will cause any regressions with the association of the application window with the launcher icon. It's unfortunate that Qt overloaded all of these things to depend on a single global application name.

What about using QStandardPaths::CacheLocation for the autocompletion files? Do you want to keep the Octave version number in that directory name?

octaverc can be the same, it will have to configure it in liboctave / liboctinterp, but I will make it correspond to where the GUI configuration is saved.

Mike Miller <mtmiller>
Group Member
Fri 08 Mar 2019 07:14:57 PM UTC, comment #6: 

Okay. Then we use QSettings for the gui preferences and AppConfigLocation/octave for the cached keyword lists used by autocompletion? What about octaverc?

Torsten Lilge <ttl>
Group Member
Fri 08 Mar 2019 06:05:47 PM UTC, comment #5: 

Yeah, so how about organization = "octave", application = "octave-gui"?

I prefer lowercase, but doesn't really matter to me. I see about 6:1 in favor of lowercase in my ~/.config directory. Empathy, GIMP, QtProject, Thunar, VirtualBox are uppercase, all others are lowercase.

Mike Miller <mtmiller>
Group Member
Fri 08 Mar 2019 06:06:08 AM UTC, comment #4: 

QSettings still should use the default location and name except for the ini extension.

As organization "GNU Octave" would be good, but I think we should prevent spaces. Application: "octave-gui"?

Torsten Lilge <ttl>
Group Member
Thu 07 Mar 2019 11:48:48 PM UTC, comment #3: 

True about QSettings::IniFormat, but that also has side effects that changes the default location and file name on other systems. Maybe that's ok.

If we want to keep a "Octave" or "octave" directory with a file under it like we have now, it looks like we need to set organization to "Octave" or "octave", and set the application name to "gui" or "qt-settings" or whatever we want to call it.

Mike Miller <mtmiller>
Group Member
Thu 07 Mar 2019 06:09:26 PM UTC, comment #2: 

Regarding QSettings: If we use the constructor of the form


QSettings::QSettings(QSettings::Format format,
                     QSettings::Scope scope,
                     const QString &organization,
                     const QString &application = QString(),
                     QObject *parent = nullptr)


we can avoid the windows registry and force the INI format without forcing file name or location.




Torsten Lilge <ttl>
Group Member
Thu 07 Mar 2019 05:27:25 PM UTC, comment #1: 

Another couple thoughts about choosing the directory:

  • Do we care whether GUI configuration files are in the same directory as octaverc files? On all platforms? I've learned that the ~/Library/Preferences directory is used on macOS by Qt for QSettings, but that directory is considered "hidden" by most macOS users, so shouldn't be used for user-editable configuration files. So ~/.config/octave for user configuration and ~/Library/Preferences/octave for GUI configuration on macOS?


  • I'll probably be adding a library call to liboctave or liboctinterp that will return the platform-dependent configuration directory for reading and writing config files in the interpreter. This will probably not use ~/Library/Preferences. Once that exists that would be another alternative to getting the configuration directory in libgui.


One downside to letting QSettings decide is that it uses the registry on Windows instead of a real file if using NativeFormat.

But an advantage to use QSettings is the "Fallback Mechanism", that it can read multiple system-wide and user-specific configuration sources and merge them.

Mike Miller <mtmiller>
Group Member
Thu 07 Mar 2019 04:51:20 PM UTC, original submission:  

This bug report covers the part of bug #40444 which is related to the GUI configuration files. As discussed in bug #55822, there are two options for storing the GUI configuration files at a location that follows the platform's conventions:

  • Let QSettings select the location and name of the settings file on each platform based on names for "application" and "organization"
  • Using QStandardPaths::GenericConfigLocation and the current directory structure (octave/qt-settings) relative to this


In the first case, which is to my opinion the approach with best alignment to the platform's conventions, we would have to chose suitable names for "organization" and "application".

Torsten Lilge <ttl>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #46497:  bug55855_v01.patch added by ttl (4KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ttl (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-04-02 mtmiller StatusReady For Test Fixed
    2019-03-31 ttl Open/ClosedOpen Closed
    2019-03-20 ttl StatusPatch Submitted Ready For Test
    2019-03-11 ttl StatusNone Patch Submitted
    2019-03-11 ttl Attached File- Added bug55855_v01.patch, #46497

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code