getoptX Help Messages for getopt #2072

burner · 2014-04-07T12:48:01Z

the getopt module is lacking help information. This PR add getoptX. getoptX allows to specify help information, in string from, after the option. It does not change anything in the getopt implementation. getoptX passes all option strings and option refs to getopt. Additionally, it returns a tuple containing a bool, that indicates if "--help|-h" where passed in the args, and an array of Option, that contains the available options. A very simple help page printer is present in form of defaultGetoptXPrinter.

as requested by @jcrapuchettes Required was added to mark options as required options.

https://d.puremagic.com/issues/show_bug.cgi?id=11876

previously AndrejMitrovic was working on this. In conjunction with him, I'm taking over (see bugzilla).
#1840 AndrejMitrovic version
#1030 jkm version
https://www.bountysource.com/issues/1327158-getopt-improvements-by-igor-lesik

burner · 2014-04-15T22:04:46Z

anyone?

monarchdodra · 2014-04-17T09:07:06Z

std/getopt.d

+
+/** The result of the $(D getoptXHelp) function.
+*/
+alias Tuple!(string, "optShort", string, "optLong", string, "help") Option;


I really don't get the point of wanting to pull out Tuples for things like this, when:

struct Option {string optShort; string optLong; string help}

Works just as well. And is POD. This is abusive use of Tuples, IMO.

I think you're right, I will change that.

DmitryOlshansky · 2014-04-22T17:51:50Z

Not looking in great detail but why can't getopt be extended as is?
Secondly there is a precedent of enforce/enforceEx so go with getoptEx.

burner · 2014-04-22T18:21:16Z

It could be extended, but it would break the current api.
I will change the name to getoptEx

ghost · 2014-04-22T19:57:37Z

It could be extended, but it would break the current api.

I don't understand why. The discussion we had in #1840 suggests we don't have to break anything.

burner · 2014-04-22T21:03:04Z

int help;
getopt("h|help", &heig)
...

"h|help" might have two meanings

DmitryOlshansky · 2014-04-23T10:27:12Z

"h|help" might have two meanings

Just check if "h|help" is already used. I think a sane default would be to issue a compile error in case there is both a user-defined "h|help" option AND documentation strings. It won't break a thing.

burner · 2014-04-23T11:21:07Z

I don't like this fix. It doesn't even fix my example. I really don't want to stick such code into getopt. getopt has an easy, forthcoming api and adding the help stuff to it would make it much more complicated IMO. I don't think that is a good trade-off. IMO getoptEx is the right way to go here.

DmitryOlshansky · 2014-04-23T11:25:53Z

I don't like this fix.

You don't have to.

It doesn't even fix my example.

It does because by definition there is no user code out there that does BOTH define an option 'h' and uses help strings (nobody can use them yet).

burner · 2014-04-23T11:31:18Z

I don't see why you don't have to fix it. My argument is that mixing getopt and getoptEx will break the api. Of course, currently no one can use this, but if an getopt mix gets merged everyone will use this.

jcrapuchettes · 2014-05-23T16:23:02Z

I'm really looking forward to getting this into phobos. It will allow me to remove a lot of custom code. Can we get this into the next release?

DmitryOlshansky · 2014-05-30T11:25:52Z

@burner take a look at the referenced pull request - it does extend getopt in backward compatible way, what was the exact reason that you say it couldn't be done?

burner · 2014-05-30T21:43:53Z

it requires that h|help are not in use. It breaks the semantic of the getopt api.

DmitryOlshansky · 2014-05-30T21:46:32Z

it requires that h|help are not in use. It breaks the semantic of the getopt api.

It however only needs that if and only if there were text messages describing each option. It in turn this means that nothing could break as long as you don't treat "h|help' specially when no descriptions are given (and all of the code there is doesn't have them - so it can't break)

jmdavis · 2014-06-02T19:25:13Z

If the concern is that somehow might already be using "h|help", and that changing getopt to support help info would then break them, then all we have to do is add another option to the config enum which turns the new functionality on. Existing code would then be unaffected.

burner · 2014-06-03T08:29:52Z

I will move the stuff to getopt

burner · 2014-06-05T23:15:49Z

it is now part of getopt in a none breaking way

DmitryOlshansky · 2014-06-07T11:01:41Z

std/getopt.d

+
+If an option string is followed by another string, this string serves as an
+description for this option. The function $(D getopt) returns a struct of type 
+$(D GetoptRslt). This return value contains information about all passed options


See no gain in abbr-itng GetotpResult :)

Well, getopt is already an abbr. But two chars more will not kill me.

I agree, Rslt is a bit too much. ;)

jcrapuchettes · 2014-06-13T23:27:41Z

Can we get this into 2.066? 👍

burner · 2014-06-14T10:49:13Z

That would be awesome. But nobody other than @DmitryOlshansky has taken a look at the source lately.

DmitryOlshansky · 2014-06-14T14:28:23Z

std/getopt.d

@@ -70,12 +70,12 @@ Color color;

 void main(string[] args)
 {
-  getopt(
+  auto helpInformation = getopt(


What's the progress? Adding unused variable to an example is silly. Either use it later or drop this variable.

I will fix that, thanks?

what do you mean by progress? getoptx is now part of getopt, without breaking getopt usage where h or help is already in use.

I mean that if the change was intentional (looks like not) then my question was if it does anything new, i.e. what is the progress compared to the previous version of unittest.

The change should show that getopt returns something. I just forgot to show what this something does and how to use it.

some tinkering alot more comments some more more doc getoptX cleanup tuple -> struct getoptX -> getoptEx Required getoptEx is now inline GetoptRslt -> GetoptResult more info some comment fixes

WalterBright · 2014-07-10T21:22:52Z

Auto-merge toggled on

burner · 2014-07-10T21:24:03Z

thank you

getoptX Help Messages for getopt

jcrapuchettes · 2014-07-10T22:49:21Z

@AndrewEdwards - Would it be possible to get this into the 2.066 release?

AndrewEdwards · 2014-07-11T00:27:54Z

@jcrapuchettes https://issues.dlang.org/show_bug.cgi?id=13072,
A link to the issue used for cherry-picking is included at the bottom of each beta announcement. A new one will be created for each beta/rc. Post there any bug/regression fix deemed critical/necessary for the current release.

mihails-strasuns · 2014-07-11T09:37:21Z

And considering this is neither bug nor regression it should wait for 2.067

jcrapuchettes · 2014-07-11T16:18:09Z

We have a need for this here at EMSI and having it in the 2.066 release would be a big benefit to us.

mihails-strasuns · 2014-07-11T17:26:00Z

Would it help preparing separate 2.066 distribution for internal EMSI usage with this changeset included?

jcrapuchettes · 2014-07-11T19:21:42Z

We prefer to use stock releases. If this change can't make it into 2.066, we can wait till 2.067.

mihails-strasuns · 2014-07-11T20:17:27Z

I believe that making any exceptions in release process by company requests is the wrong thing to do, it creates certain expectations and does not scale with increased user base. If we start adding features during beta right now, when do we stop?

Having custom distributions for such needs is quite a common practice to address this problem if some changes are really important. We actually do this in Sociomantic despite having 0 difference with D1 upstream (and despite the fact it is mostly frozen).

I don't want to sound like I don't think your needs are not important but we need to find a good compromise between addressing the needs of commercial users and keeping upstream community development process adamant.

jcrapuchettes · 2014-07-11T20:22:41Z

I understand. We currently use a workaround and will plan on continuing to use that till the next release.

MartinNowak · 2015-03-03T23:02:08Z

This is actually amazing. Just found out about it now, should make it to the changelog for 2.067.
http://dlang.org/changelog.html

CyberShadow · 2017-07-21T09:19:22Z

This pull request introduced a regression:
https://issues.dlang.org/show_bug.cgi?id=16684

burner changed the title ~~getoptX Help Messages for getOp~~ getoptX Help Messages for getopt Apr 7, 2014

monarchdodra reviewed Apr 17, 2014
View reviewed changes

ghost mentioned this pull request May 22, 2014

Issue 11876 - Implement automatic --help and --help=option printout for getopt. #1840

Closed

DmitryOlshansky reviewed Jun 7, 2014
View reviewed changes

DmitryOlshansky reviewed Jun 14, 2014
View reviewed changes

getoptx

d602a53

some tinkering alot more comments some more more doc getoptX cleanup tuple -> struct getoptX -> getoptEx Required getoptEx is now inline GetoptRslt -> GetoptResult more info some comment fixes

WalterBright added a commit that referenced this pull request Jul 10, 2014

Merge pull request #2072 from burner/getoptx

dc97d14

getoptX Help Messages for getopt

WalterBright merged commit dc97d14 into dlang:master Jul 10, 2014

MartinNowak added this to the 2.067 milestone Mar 3, 2015

burner deleted the getoptx branch October 23, 2015 09:28

getoptX Help Messages for getopt #2072

getoptX Help Messages for getopt #2072

Conversation

burner commented Apr 7, 2014

burner commented Apr 15, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DmitryOlshansky commented Apr 22, 2014

burner commented Apr 22, 2014

ghost commented Apr 22, 2014

burner commented Apr 22, 2014

DmitryOlshansky commented Apr 23, 2014

burner commented Apr 23, 2014

DmitryOlshansky commented Apr 23, 2014

burner commented Apr 23, 2014

jcrapuchettes commented May 23, 2014

DmitryOlshansky commented May 30, 2014

burner commented May 30, 2014

DmitryOlshansky commented May 30, 2014

jmdavis commented Jun 2, 2014

burner commented Jun 3, 2014

burner commented Jun 5, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcrapuchettes commented Jun 13, 2014

burner commented Jun 14, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WalterBright commented Jul 10, 2014

burner commented Jul 10, 2014

jcrapuchettes commented Jul 10, 2014

AndrewEdwards commented Jul 11, 2014

mihails-strasuns commented Jul 11, 2014

jcrapuchettes commented Jul 11, 2014

mihails-strasuns commented Jul 11, 2014

jcrapuchettes commented Jul 11, 2014

mihails-strasuns commented Jul 11, 2014

jcrapuchettes commented Jul 11, 2014

MartinNowak commented Mar 3, 2015

CyberShadow commented Jul 21, 2017