Page MenuHomeWildfire Games

Remove Gamesetup.cpp readme.txt duplication
AbandonedPublic

Authored by elexis on Apr 4 2018, 8:52 PM.

Details

Reviewers
None
Summary

It is really annoying and errorprone to have to update two files with every change and different format. An RTFM comment should be sufficient.

Test Plan

Become convinced.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 5762
Build 9667: Vulcan BuildJenkins
Build 9666: arc lint + arc unit

Event Timeline

elexis created this revision.Apr 4 2018, 8:52 PM
mimo added a subscriber: mimo.Apr 4 2018, 9:07 PM

I agree with such a duplication removal, but i don't find the "RTFM" quite handy. I'd rather have an option "autostart -help" which would display it on the console.

Vulcan added a subscriber: Vulcan.Apr 4 2018, 11:24 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/differential/342/display/redirect

Itms added a comment.Apr 5 2018, 10:02 AM

I second mimo's proposal. readme.txt should be generated somehow, either from the code itself (probably a lot of work but a very cool feature) or extracted from code comments. And in the meantime I think it would be preferable to keep the comments and the readme, rather than removing helpful documentation.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/344/display/redirect

I agree that having a command line option to show all command line options would be great.
But that use case for the user, not for the developer.
Which use case does the Gamesetup.cpp contents serve for the reader of this file that can just as well read the same thing in readme.txt?

An alternative would be removing the commandline options part from readme.txt, moving them to a string in GameSetup.cpp and showing them when --help is passed and stating in readme.txt that these things can be seen when passing --help.

mimo added a comment.Apr 5 2018, 6:54 PM
In D1433#58491, @elexis wrote:

I agree that having a command line option to show all command line options would be great.
But that use case for the user, not for the developer.
Which use case does the Gamesetup.cpp contents serve for the reader of this file that can just as well read the same thing in readme.txt?

also developper :)  usually i use that doc when i try to modify the  Gamesetup.cpp file, it is much simpler to have it there than having to go through the directory hierarchy to find the readme.txt

An alternative would be removing the commandline options part from readme.txt, moving them to a string in GameSetup.cpp and showing them when --help is passed and stating in readme.txt that these things can be seen when passing --help.

I'd be fine with that (except that --help should be kept for a future possible 0ad help, here it is an autostart help, so better call it -autostart-help or something analoguous).

In D1433#58512, @mimo wrote:

also developper :) usually i use that doc when i try to modify the Gamesetup.cpp file, it is much simpler to have it there than having to go through the directory hierarchy to find the readme.txt

Since the exact filepath is printed there, one doesn't have to type anything besides the fileviewer program, for instance gedit ctrl+v at worst (when using an IDE like eclipse it may only be ctrl+R + ctrl+V).
If the effort argument is true, then we could copy many things to many places leading to the duplication pattern problems.
For instance the other commandline options explained in readme.txt could be copied to the other files where the commandline options are implemented, resulting in even more files to keep in sync.

When exactly is the less-effort use case met?
When wanting to use the binary without reading or changing Gamesetup.cpp, the effort to open the readme and Gamesetup.cpp file are identical.
When wanting to extend Gamesetup.cpp, one has to change readme.txt too.
When wanting to modify but not extend Gamesetup.cpp, one has the name of the broken property already given.
I can abandon this without being convinced by the presented arguments however.

readme.txt should be generated somehow, either from the code itself (probably a lot of work but a very cool feature) or extracted from code comments

It sounds like a cure worse than the disease.

--help should be kept for a future possible 0ad help, here it is an autostart help, so better call it -autostart-help or something analoguous

All commandline helps besides the one of git that I know of list some generic help and then all command line options.

If we really want a commandline help tool, then the autostart Gamesetup.cpp arguments should be split as described in #3438 and then we would need a structure similar to the one used in rP19475 (though I doubt the latter is sanely possible in cpp).
But again this is a feature discussion which should be placed at a ticket that proposes the desired feature.
The intention of the patch is to remove something whose harm exceeds the benefit, not to add a commandline help.

Stan added a subscriber: Stan.Apr 11 2018, 5:24 PM

Commands like netsh on windows have multiple levels of help

For instance

Netsh /?
Netsh start /?

So if someone uses incorrect autostart parameters it could display it

mimo added a comment.Apr 11 2018, 7:44 PM
In D1433#58958, @elexis wrote:
In D1433#58512, @mimo wrote:

also developper :) usually i use that doc when i try to modify the Gamesetup.cpp file, it is much simpler to have it there than having to go through the directory hierarchy to find the readme.txt

Since the exact filepath is printed there, one doesn't have to type anything besides the fileviewer program, for instance gedit ctrl+v at worst (when using an IDE like eclipse it may only be ctrl+R + ctrl+V).
If the effort argument is true, then we could copy many things to many places leading to the duplication pattern problems.
For instance the other commandline options explained in readme.txt could be copied to the other files where the commandline options are implemented, resulting in even more files to keep in sync.
When exactly is the less-effort use case met?
When wanting to use the binary without reading or changing Gamesetup.cpp, the effort to open the readme and Gamesetup.cpp file are identical.
When wanting to extend Gamesetup.cpp, one has to change readme.txt too.
When wanting to modify but not extend Gamesetup.cpp, one has the name of the broken property already given.
I can abandon this without being convinced by the presented arguments however.

Why are discussions with you always so complicated! It was never question of adding duplication, and everybody agrees with removing that one. The question is simply how to do it best. And for me (if i'm still allowed to work my own way without you imposing what people should do or think), i don't find the readme.txt user friendly and i'd rather wait for a proper fix (with some help command) rather than that removal.

readme.txt should be generated somehow, either from the code itself (probably a lot of work but a very cool feature) or extracted from code comments

It sounds like a cure worse than the disease.

--help should be kept for a future possible 0ad help, here it is an autostart help, so better call it -autostart-help or something analoguous

All commandline helps besides the one of git that I know of list some generic help and then all command line options.

What don't you understand in the "or something analoguous"? in it, i include of course "--help autostart" which is quite generic in linux. My only point was to not have it only "--help" as you proposed as this should be kept for a generic 0ad help and not an autostart one.

If we really want a commandline help tool, then the autostart Gamesetup.cpp arguments should be split as described in #3438 and then we would need a structure similar to the one used in rP19475 (though I doubt the latter is sanely possible in cpp).

I don't see any connection with a help support and #3438. And why doing complicated thing when a simple approach would work? (parsing the argument and printing a string when --help is found, and btw that's the only reason why i spoke about "-autostart-help" above as all arguments starting by autostart are already treated. So if we wanted to start with a simple autostart help, that would make it trivial. But a more generic approach is also fine for me of course.

But again this is a feature discussion which should be placed at a ticket that proposes the desired feature.
The intention of the patch is to remove something whose harm exceeds the benefit, not to add a commandline help.

elexis abandoned this revision.Apr 11 2018, 7:59 PM

To me the objective was to remove the duplication, which is not easily achievable when we want to implement a CLI help and keep a readme.txt.

mimo added a comment.Apr 11 2018, 8:06 PM
In D1433#58973, @elexis wrote:

To me the objective was to remove the duplication, which is not easily achievable when we want to implement a CLI help and keep a readme.txt.

I think the (long term) solution is to have a script (possibly run automatically once or two per week as for translations) which would generate the readme.txt from the different CLI helps.