widelands --help does not document all possible parameters

Bug #536163 reported by Sigra
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
widelands
Won't Fix
Medium
Unassigned

Bug Description

widelands --help does not document all possible parameters

Revision history for this message
Timowi (timo-wingender) wrote :

The current code is hard to use. I would suggest doing this with something like libpopt.

Changed in widelands:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
Jens Beyer (qcumber-some) wrote :

I could do this using GLIB2 Command line option parser* which I found indeed easier to use than libpopt. Of course this would add a GLIB2 dependency to the build. With libpopt it would add a dependency to this library, so it would be a new dependency anyway.

Would a GLIB2 dependency be troublesome to anyone, especially looking at Mac/Windows?

* http://developer.gnome.org/glib/2.30/glib-Commandline-option-parser.html

Revision history for this message
SirVer (sirver) wrote : Re: [Bug 536163] Re: widelands --help does not document all possible parameters

I am reluctant to use another library just for parsing the cmdline. If
you want to use a third party library, I would strongly suggest boost
program options because we already depend on a number of boost stuff.

GLIB especially is a bit far from us..

Just my 2c of course :)

Revision history for this message
Shevonar (shevonar) wrote :

A few weeks ago I also had some thought about this bug and tried to use Boost Program Options. Several problems came up:
1. The format of the command line parameters would change slightly or we need two write an own parser. The default parser has no '=' between option and value and you cannot create options with an optional value (like --editor).
2. All values are then stored in Boost PO's value storage and somehow need to be transferred to g_options (our current value store).
3. Boost PO can also parse config files, again with a different format (no "" around values). It would be nice to manage all options in a single place.
4. When I changed the default value for OpenGL I faced another problem: The default value for an options is currently defined in every get statement. There was a stupid bug because I forgot to change one of them from false to true. So default values should also be defined in a central place (like Boost PO does it)
5. Boost PO's value storage is read only, there are no set methods and you cannot save the options to a config file. These are two required extensions if we decide against using the current g_options variable.

Conclusion: Boost PO does many things very well, but does not exactly fit our requirements. I don't know if it easier to write a similar class from scratch or to extend Boost PO. Maybe I find some time to try it out.

Revision history for this message
SirVer (sirver) wrote :

Matthias, as always your arguments are well thought out and convincing. So not boost.

If given the choice between GLIB or lipopt I'd prefer GLIB because it might contain more useful stuff (though I do not know it very well, so maybe not). However, how much work would it to improve the code we currently have?

Revision history for this message
Jens Beyer (qcumber-some) wrote :

Well, the original intention of #1 was to improve the parsing code, and apparently also the automatic creation of the help page. Both are supplied by libpopt and the GLIB Command line parser.

It needs decision if we want to do that, or only parse a command line more conveniently, and have the help page set up manually. This means as it is now, if you implement a command line option, you need to update the help page yourself. If you don't, it doesn't appear there.

To only parse the command line, there are a few more (mostly header only) libraries like tclap.

But additionally to command line parsing, we have the additional requirement to maintain (read and write) a config file. This is something which no library seem to provide. The boost program options have readonly file support, but can not write. And I think it won't be a good move to have a library do half the work and your own code to do the rest.

I personally would vote for using the library as intended to parse the options more conveniently, and to build the help page automatically. Then only reading and writing the config file would be done (as it is now) in own code.

I would suggest to try and use GLIB command line parser. There are many desktop programs which depend on glib on Linux, at least GNOME and all direct derivates; I would also think it is already installed on Unity/Cinnamon desktops, KDE uses it for a few programs like systemsettings, firefox uses it, libreoffice/openoffice, gimp, gstreamer... so I would think on Linux it will be present on all distributions which can also handle widelands.

The first thing to do would be find out if all options used by widelands can be supported by GLIB command line parser. I could do that, and it would take me a while (as in response to #5), but it would suit my knowledge level quite well :-D

Revision history for this message
Shevonar (shevonar) wrote :

I appreciate your effort, but I also see some problems in using a library just for command line parsing. This library (whichever we choose) will solve two problems: command line parsing and providing the help message. But there are two more problems that should be solved by the same code IMHO. These are:
- Reading and writing the config file to have a single options array
- Providing default values for options that are nether specified in the config file nor in the command line parameters

If we use our own code to solve these and a library for command line parsing we need to merge the options from the config file and the command line somehow. In my tries with Boost PO and the current config file parser I had to do this manually in the code for every single options! Maybe there is a better way but I could not figure it out. Another nice little feature that will be possible if command line and config file are handled by the same code are comments inside the config file describing the options (the same as for the command line help).

Having said that I also see a problem in writing a new options parser for command line and config file together. We already have a config file parser! But this one is not easy to extend to suit the needs for the new options config file parser. On the other hand writing a new config file parser for all conf files is a major undertaking since it touches loads of files (every file where a conf value is accessed). However having two config file parsers in parallel is also not a nice solution.

I did not start to work on this bug because I have not come up with a good solution yet. I am not sure if it is good to start using a library for command line parsing now and ignore the other problems (no offence!). I think these problems are belong together and should be solved as a whole.

Revision history for this message
Jens Beyer (qcumber-some) wrote :

That's why we are discussing this before taking too much effort into an unloved solution. :-)
So I don't see a place for being offended.

I will leave this as it is now, until we find a proper solution to all problems/requirements. But I think the result then will be writing a parser on our own.

Revision history for this message
Nasenbaer (nasenbaer) wrote :

Strange, the bug is listed in the blueprint it is assigned to, but the blueprint is not listed in the bug....

here we go: https://blueprints.launchpad.net/widelands/+spec/refactor-options

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

>Strange, the bug is listed in the blueprint it is assigned to, but the blueprint is not listed in the bug....

I can see it on the right side (below subscribers)?

Revision history for this message
Nasenbaer (nasenbaer) wrote :

oh indeed. Seems I should have searched longer. I thought it would show up in the summary bar like "target to milestone" does.
How ever I this is offtopic ;)...

Revision history for this message
SirVer (sirver) wrote :

Setting to incomplete for bug sweeping.

Changed in widelands:
status: Confirmed → Incomplete
Revision history for this message
SirVer (sirver) wrote :

This was wrongly set to incomplete.

Changed in widelands:
status: Incomplete → Confirmed
Revision history for this message
Ferdinand T. (f-thiessen) wrote :

I just want to add that GLib also has an config file parser: GKeyFile Parser
So it can be used for command-line parsing and for config file parsing.

Revision history for this message
SirVer (sirver) wrote :

GLib is quite the heavy dependency though and I'd rather not pull it in just for that.

Revision history for this message
GunChleoc (gunchleoc) wrote :
Changed in widelands:
status: Confirmed → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Related blueprints

Remote bug watches

Bug watches keep track of this bug in other bug trackers.