On Sat, 19 Aug 2017 at 7:14:03 -0400, Doug Torrance wrote: > On 08/19/2017 06:13 AM, Carlos R. Mafra wrote: > >Thanks Doug. > > > >I have one comment: > > > >On Fri, 18 Aug 2017 at 20:37:46 -0400, Doug Torrance wrote: > >>Previously, WPrefs could only be used to edit the menu specified in > >>WMRootMenu. > >> > >>In a recent commit, the ability to specify a menu in proplist format defined > >>in another file which is referenced by WMRootMenu was added. However, if a > >>user attempted to edit such a menu in WPrefs, an error dialog appeared. > >> > >>We add the ability for WPrefs to read such a menu. > > > >That's nice! > > > >>After the user makes any changes, the result is stored in WMRootMenu, > >>and *not* the original file. > > > >This is unexpected and will probably defeat the purpose of > >having a separate file for the menu in the first place. > > > >What is the technical challenge to save the changes back > >into the original file? That would be much better. > > There is a chance that the original file is read-only.
Then why a user would try to modify it using WPrefs? > For example, the default menu in the Debian package is stored in > /usr/share/WindowMaker/menu.hook (which is actually a symbolic link to > /etc/GNUstep/Defaults/plmenu.Debian), and WMRootMenu consists of the single > line: > "menu.hook" > > As of the lastest version of the Debian package (0.95.8-2), this file is now > in proplist format, and so WPrefs can read it (this was my original impetus > for writing this patch), but it can't write back to it. WPrefs should not do anything behind our back to bypass this oddity in how the Debian menu is set up. It is more desirable to fix Debian than to make WPrefs behave in ways users don't expect. Because a legitimate user with proplist menus in his _home_ directory will have a good reason to have them as separate files (otherwise he would not be using this feature). This user will not be happy to see this file merged into WMRootMenu without his permission. > I suppose a solution would be to write back to the original file if > possible. Otherwise, copy the modified version of the file to the user > directory. No, this is not a solution. If the user has no permission on the file then he has no right to expect to write to it and he should be given a "no permission" error. Can't you just make Debian behave better here instead? Just copy the menu file to the user home and use that instead. No need to play games with WPrefs. > So for example, in the Debian case, after modifying the menu, we create a > new file, ~/GNUstep/Library/WindowMaker/menu.hook. Why doesn't Debian write a copy of the menu.hook in the user dir in the first place and start from there? > Since we look in the user directory before the system directory for > menu files, WMRootMenu will now point us to the new, modified version. > > One issue with this would be in the case that WMRootMenu references the full > path, e.g., > "/usr/share/WindowMaker/menu.hook" > > Then we would need to also modify WMRootMenu to point to the new version. > > If this seems reasonable, I'll work on a patch. I'd say modify your patch to write the changes back into the original file and let the user be given an error if he has no permission. And then make Debian use a menu file from the user's home. How does that sound? Btw, I have uploaded your patches already to #next, but feel free to come up with a new series and I'll rebase. -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
