To comment on the following update, log in, then open the issue:
http://www.openoffice.org/issues/show_bug.cgi?id=50817
------- Additional comments from [EMAIL PROTECTED] Fri Jun 17 01:28:54 -0700
2005 -------
This way of handling dependencies within one layer is not correct, as you don't
know whether the setting from this layer will prevail. Even if the proxy mode in
gconf is not 'manual' the user may override that in his settings or there may be
a different layer that locks it with higher priority. In both cases the
effective value may be 'manual'. And in that case the user probably wants to use
a default value set by gconf (if any). Your code assumes, that if 'manual' is
not set in gconf, then there is no valid value for the proxy settings in gconf
at all. In my experience that is not necessarily true and gconf certainly
doesn't enforce this.
Some more comments on the new patch:
- This is C++. Typedef'ing an anonymous enum
typedef enum { ...} EnumName;
is a C-ism. and should not be used. In C++ use
enum EnumName { ... };
After that declaration EnumName can be used as a type name.
(applies to LockdownSetting)
- Using the names 'Lockdown...' is misleading, as lockdown is not the primary
function of this backend. Instead this is about injecting or importing gconf
values into the OOo configuration. Primarily that makes the gconf values act as
default in OOo. It is a secondary function that lockdown status is also
injected. So I suggest you look for a better name. Maybe you can simply use
'GConf' instead of 'Lockdown'.
- You should add a comment that the LockdownValue list must by in sync to enum
LockdownSetting, so they enumerators can be used as index into the list.
- Of course this can't be commited while you have a long list of OOo settings
that are mapped to arbitrary/inappropriate gconf values.
- I only now discovered that you want to define new gconf settings explicitly
intended to control OOo settings via gconf. I don't think that is a good idea.
It certainly doesn't scale. The OOo schema has close to 3000 different
properties and there are even ~18000 actual keys. Mapping only a significant
proportion of then would be a maintenance (and probably also a performance)
nightmare. OOo itself has its own desktop-independent equvalents for most gconf
features, e.g. setting up site defaults or lockdown. I think the better approach
would be to create a configuration of this mechanism and tools to manage the
settings that aligns and integrates with the tools you have for managing gconf
applications.
But if you prefer to stay with your approach, please conditionalize this and
make it a (non-default) configure option, because I don't think we want that for
stock openoffice.org and certainly not for StarOffice (especially considering
the potential performace implications of reading ever increasing amount of gconf
settings during startup).
- Using a hardcoded directory name 'Documents' for the working directory setting
is not acceptable IMHO. Is this really the hard-coded, non-configurable,
non-localizable name of that directory in gnome? (If so that looks broken to me)
In the case of the working directory it also looks wrong to me that the user
gets a different one depending on whether he starts OOo under Gnome or a
different desktop (without any gconf setting being involved).
But it either case this is the wrong place to do that:
1. The path configuration has its own mechanism for handling a default for the
'current' settings, which dates back to a time when the built-in default
handling did work correctly yet (or the developer didn't know about it yet). You
change only the 'current' but not the default that the user gets when she resets
that setting in the UI.
2. The $(work) placeholder should always be identical to the default working
directory. It might be used elsewhere as well. The correct place to change this
is in the PathSubstitution component of the configuration.
So if you really want to do this, please do it in the PathSubstitution
configuration and don't do it in code in this backend. Instead you can create a
minmal PathSubstitution-Documentsdir.xcu and install that into
$(install)/share/registry/modules/org/openoffice/Office/PathSubstitution/. Of
course the corresponding scp item should also be conditionalized, and the
directory name should be configurable.
- If you still think you want to keep this working directory item as is, then it
should only be provided, if the component is org.openoffice.Office.Common and it
should probably become part of the timestamp.
- Your preloads should also be pruned to correspond to the module being loaded
(why preload proxy settings for the VCL component or lockdown settings for
Inet?).
- Instead of yourString.equals( OUString::createFromAscii("...")) you should use
the far more efficient (no need for allocations) myString.equalsAscii("...").
---------------------------------------------------------------------
Please do not reply to this automatically generated notification from
Issue Tracker. Please log onto the website and enter your comments.
http://qa.openoffice.org/issue_handling/project_issues.html#notification
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
