-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CSSTUDIO-2231 Add flag -select_settings
for selecting a Phoebus configuration file on startup
#2995
Conversation
…ttings' is also present.
…ove error handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strings shown to user should be externalized to messages.properties (e.g. PhoebusApplication).
Failure to start (e.g. incorrect configuration file, unsupported combination of program arguments) should trigger a dialog with suitable error message. Otherwise user will not know why Phoebus did not start, and console/stdout/stderr might not be available.
Good points. I will fix these. |
So far it's fine to list multiple Similarly, it should be OK to run like this if we support
So you have several base settings which add up, and finally some selectable option. So |
I found it potentially confusing, since settings can shadow each other, and therefore I disallowed it. But if this is used, then it makes sense to support it! In that case I would think that not only a combination of
would then launch two selection dialogs in succession. Do you think this is a good idea? |
Multiple dialogs is not ideal. |
…elect_settings' is also present." This reverts commit e3d33cf.
Suppose the commandline options |
That would not be different than supporting multiple -settings. Supporting multiple -select_settings to support multiple configuration directories is maybe a degree of non-essential flexibility. |
…ttings' -> '-select_settings'.
I have implemented these changes now. |
I disagree with this, since I think the natural intended meaning of two specifications of
This may be the case. |
Mind you, I am questioning the need for |
We have a usecase for this functionality, and while the functionality in principle could be implemented in a separate script, I do not think that the functionality is out of place in Phoebus either. |
@kasemir: Do you object to this PR, or is it, in your view, OK that I go ahead and merge it? |
I'd consider it superfluous, but it's not breaking anything, so go ahead. |
This PR adds the flag
-select_settings
which allows for selecting a Phoebus configuration file on startup of Phoebus.When Phoebus is started with the option
-select_settings <directory>
, a dialog will open that allows one to select a configuration file indirectory
when starting Phoebus. Any file in<directory>
that ends with either.ini
or.xml
can be selected.-select_settings
is not allowed to be used in conjunction with the option-settings
in order to avoid confusion.Testing request: Phoebus seems to support settings in the form of XML-files. Unfortunately, I do not have a configuration file in XML-format to test with. Therefore the loading of configuration files in the XML format in this PR has not been tested. Could someone test this functionality with a configuration file in XML format and confirm that it works?