--help me if you can #5680

emilengler · 2020-07-14T11:13:46Z

Introduction

This implements the --help me if you can by Daniel Fandrich presented at curl-up 2020. See here.
I know that the diff looks big (242 files changed). However, ~99% of these just set the category for each item.
I suggest you to have a look at this YouTube Video where I explain how this works in detail.
I hope this can get into the next release. Maybe set this high priority?

I split up curl into 22 categories:

 auth                Options related to authentication of any kind
 connection          Options related to low level network operations
 curl                Options related to the behaviour of curl itself
 dns                 Options related to DNS
 file                Options related to the FILE protocol
 ftp                 Options related to the FTP protocol
 http                Options related to the HTTP and HTTPS protocols
 imap                Options related to the IMAP protocol
 misc                Options that don't fit in any other category
 output              Options that handle the output of the downloaded content
 pop3                Options related to the POP3 protocol
 post                Optons related to HTTP Post
 proxy               Options related to proxy settings of any kind
 scp                 Options related to the SCP protocol
 sftp                Options related to the SFTP protocol
 smtp                Options related to the SMTP protocol
 ssh                 Options related to the SSH protocol
 telnet              Options related to the Telnet protocol
 tftp                Options related to the TFTP protocol
 tls                 Options related to any kind of TLS/SSL operation
 upload              Options related to uploads of any kind
 verbose             Options related to any kind of command line output of curl

It works through a bitmask. Because of this every item can contain multiple categories.
If the user does not request any special category, a list of the most important curl commands is being shown. I know this topic is highly controversial and I am open to any suggestions (maybe also start a user survey?).

Reviewing

Please see this PR as two parts. One documentation part and one technical part.
The documentation part is just the first commit (d76538c).
The technical part are all commits after this one.
The technical part is already ready to merge in my opinion. However the documentation part still needs to be reviewed a lot. I put most options into the categories on guessing.

Requested Reviewers

Of course everyone is most welcome to do his review, I would still like a review from @bagder on the selected categories (sorry for 200+ files :P) as he probably knows them the best and a review from @dfandrich as he is the inventor of this idea.

TODO

The advanced help menu logic
Tests
Category review

gvanem · 2020-07-14T13:09:45Z

Handling the categories are like traversing a directory. So it would be handy to have an interactive completion mode in
curl --help (or whatever) to navigate through all the options (since it's hard to know what to look for).

With the Python prompt_toolkit it's very easy to write fancy prompts like in this little demo:

#!/usr/bin/env python

from prompt_toolkit.shortcuts  import CompleteStyle, prompt
from prompt_toolkit.completion import WordCompleter

curl_completer = WordCompleter ([ "auth", "connection", "curl", "dns", "file", "ftp", "http", "imap",
                                  "misc", "output", "pop3", "post", "proxy", "scp", "sftp", "smtp",
                                  "ssh", "telnet", "tftp", "tls", "upload", "verbose" ],
                                ignore_case = True)

category = prompt ("Give me a curl category. Press <TAB> for all: ",
                   completer = curl_completer, complete_while_typing = True,
                   complete_style = CompleteStyle.MULTI_COLUMN)

print ("Got category: %s" % category)

It looks like this when I press the <TAB>:

The idea was then to give another prompt based on a match to those added Category: lines in cmdline-opts/*.d.
And most users have Python installed nowadays, no?

emilengler · 2020-07-14T13:23:15Z

Handling the categories are like traversing a directory. So it would be handy to have an interactive completion mode in

curl --help (or whatever) to navigate through all the options (since it's hard to know what to look for).

With the Python prompt_toolkit it's very easy to write fancy prompts like in this little demo:

#!/usr/bin/env python



from prompt_toolkit.shortcuts  import CompleteStyle, prompt

from prompt_toolkit.completion import WordCompleter



curl_completer = WordCompleter ([ "auth", "connection", "curl", "dns", "file", "ftp", "http", "imap",

                                  "misc", "output", "pop3", "post", "proxy", "scp", "sftp", "smtp",

                                  "ssh", "telnet", "tftp", "tls", "upload", "verbose" ],

                                ignore_case = True)



category = prompt ("Give me a curl category. Press <TAB> for all: ",

                   completer = curl_completer, complete_while_typing = True,

                   complete_style = CompleteStyle.MULTI_COLUMN)



print ("Got category: %s" % category)

It looks like this when I press the <TAB>:

The idea was then to give another prompt based on a match to those added Category: lines in cmdline-opts/*.d.

And most users have Python installed nowadays, no?

I like your idea, however making it interactive is the task of the shell, not ours. However maybe we should provide autocompletion scripts. It wouldn't be a problem to add this functionality to the gen.pl script but this is probably outside of the scope of this PR.

bagder · 2020-07-14T15:56:41Z

Test 1139 seems to fail on some CI builds (example):

 --hostpubmd5 is not in curl.1 (but in tool_getparam.c tool_help.c)

bagder · 2020-07-14T16:15:53Z

Two questions (that I didn't find answered yet, most probably because I didn't look very closely yet):

Will it cause a build error if I leave out a category for a cmdline option? I think it should.
Will it cause a build error if I misspell a category in the cmdline docs file? I think it should.

emilengler · 2020-07-14T16:31:51Z

Will it cause a build error if I leave out a category for a cmdline option? I think it should.

Not yet but I think it throws a warning that it misses a category line. Same with other required options

Will it cause a build error if I misspell a category in the cmdline docs file? I think it should.

Yes, because there won't be a bitmask #DEFINE. However if you're generate the bitmask definitions with ./gen.pl listcats then their won't

emilengler · 2020-07-14T19:17:14Z

Test 1139 seems to fail on some CI builds (example):
 --hostpubmd5 is not in curl.1 (but in tool_getparam.c tool_help.c)

Fixed in dd873f6

emilengler · 2020-07-14T19:31:53Z

I re-ordered the commits into a more logical order

sterchelen · 2020-07-17T20:23:08Z

docs/cmdline-opts/append.d

@@ -2,6 +2,7 @@ Short: a
 Long: append
 Help: Append to target file when uploading
 Protocols: FTP SFTP
+Category: ftp sftp


I would say categories

Suggested change

Category: ftp sftp

Categories: ftp sftp

I see that in most cases, categories is equal to protocols. Does it make sense to use by default the protocols field as the categories and in specific cases use the categories as an override field ?

Thanks for your review!
Most options only belong to one category. The plural would sound weird there in my opinion.

Does it make sense to use by default the protocols field as the categories

That sounds like a lever idea that will reduce a lot of duplication!

That sounds like a lever idea that will reduce a lot of duplication!

But what is with options like --user or --output which don't belong to any (specific) protocol

Most options only belong to one category. The plural would sound weird there in my opinion.

In your code you are already referencing categories:

curl/src/tool_help.c

Lines 119 to 123 in af4dd10

struct helptxt {

const char *opt;

const char *desc;

curlhelp_t categories;

};

Protocols is in plural form and b/c category(ies) is relatively equal to protocols I think we need to stay consistent.

But what is with options like --user or --output which don't belong to any (specific) protocol

We could you use this logic to define the category(ies):

my_category(ies): category(ies) | default(protocols) | default("curl")

What do you think?

@bagder I'm not sure to understand you point of view 🤔 ; tell me if I am wrong:

each file has a category: field

and over all just make sure they match, so you have to duplicate the protocols in the categories to make sure it is correct?

Yes I think:

each .d file gets a category: field, that can be empty or list additional categories apart from the protocols which are used by default

The category curl should rather be renamed tool and should be explicitly mentioned as a category if desired.

This way, the used categories are all visible in each file (just that they're also in the protocols line) and we don't need to repeat the protocols in the category field.

Sounds good to me, your position @emilengler 😃 ?

I will have a deeper look on this at Sunday or Monday. I am currently on vacation

The change from tool to curl makes sense. I will change this.
I also agree on the part that the category field can be empty where the item would fall into the misc category then.
However I still don't think it is a good idea to mix up with protocols. Of course this saves some work but I still think it adds more confusion also AFAIK the protocol field is not omnipresent so far

src/tool_help.c

emilengler · 2020-08-13T12:17:41Z

I implemented @bagder's improvements. I will now put the commits into a more logical order

emilengler · 2020-08-13T12:26:57Z

I've also added a new test which tests the case insensitivity.

sterchelen · 2020-08-21T19:24:02Z

@emilengler a lot of tests are failing.

tool_help.c:891:13: error: function declaration isn’t a prototype [-Werror=strict-prototypes]
 static void get_categories()

emilengler · 2020-08-21T19:45:41Z

@emilengler a lot of tests are failing.


tool_help.c:891:13: error: function declaration isn’t a prototype [-Werror=strict-prototypes]

 static void get_categories()

Thanks for your testing! For me they all worked fine o.O
How you have run them? Just make test?

sterchelen · 2020-08-21T20:06:43Z

@emilengler a lot of tests are failing.
tool_help.c:891:13: error: function declaration isn’t a prototype [-Werror=strict-prototypes]

 static void get_categories()
Thanks for your testing! For me they all worked fine o.O
How you have run them? Just make test?

By looking on the checks of your PR

emilengler · 2020-08-21T20:08:56Z

@emilengler a lot of tests are failing.
tool_help.c:891:13: error: function declaration isn’t a prototype [-Werror=strict-prototypes]

 static void get_categories()
Thanks for your testing! For me they all worked fine o.O
How you have run them? Just make test?
By looking on the checks of your PR

I will have a deeper a look soon.

emilengler · 2020-08-22T09:26:07Z

Should be fixed now

emilengler · 2020-08-23T06:50:29Z

Someone knows what type I should use? I need at least 32-bits of space for the bit mask. However fixed size data types are a C99 thing

bagder · 2020-08-23T16:08:55Z

You mean for an unsigned 32 bit variable? unsigned int will be good enough. curl only builds on >= 32 bit systems and they all have 32 bit ints...

emilengler · 2020-08-24T06:26:52Z

You mean for an unsigned 32 bit variable? unsigned int will be good enough. curl only builds on >= 32 bit systems and they all have 32 bit ints...

Ok will change that

emilengler · 2020-08-24T06:28:56Z

Updated

emilengler · 2020-08-24T06:57:48Z

I think the failed test is very likely a false positive

emilengler · 2020-09-02T18:24:00Z

Merge conflicts solved

bagder

I think this looks great. This is a big step forward for --help. I think the categories you've picked seem perfect. We can always fix them later if we find mistakes.

I just have three minor things.

src/tool_help.c

SuperSandro2000 · 2020-09-03T06:50:10Z

Could we get a bash completion script for that? I specially asking not for a command line option cause this either makes she'll startup slow or decouples the auto completion version from the fuel version.

bagder · 2020-09-03T06:55:00Z

I generally like the bash completion for curl provided by the bash-completion package on Debian. It is maintained outside of the curl project.

sterchelen

eager to see this great addition in the next release 💪

Closes curl#5680

emilengler · 2020-09-03T15:27:49Z

@bagder Changes from you applied. I also rephrased the category descriptions to be more descriptive.

emilengler · 2020-09-03T15:29:07Z

Note to the maintainer who will merge this:
Would it be possible to merge the 5 commits as they are without squashing them?
I think it's better to have 5 different commits which each do another part. This is simpler to maintain rather than having one big diff.

bagder · 2020-09-03T15:40:23Z

Sure, that's possible. But are they really stand-alone enough to warrant being individual commits? They certainly don't have proper individual commit messages right now...

emilengler · 2020-09-03T15:44:54Z

Sure, that's possible. But are they really stand-alone enough to warrant being individual commits? They certainly don't have proper individual commit messages right now...

Yes I also thought about that while writing the comment. I thought about adding a prefix or add that line to the commit message:

This commit is a part of "--help-me-if-you-can"; #5680

emilengler · 2020-09-03T18:25:44Z

The failing tests on Windows are a lower level problems. It is because of line endings. Adding the Windows line ending to the test cases will make them to not work on Windows. Fixing this would be outside of the scope of this PR.

bagder · 2020-09-03T20:18:06Z

Fixing it is a matter of setting mode=text" in the <stdout> tag. Like: <stdout mode="text">

This is the begin of the implementation of "--help me if you can" by Daniel Fandrich presented at curl up 2020. See: https://bit.ly/2C92PvR This commit is a part of "--help me if you can"; curl#5680

This commit is a part of "--help me if you can"; curl#5680

This commit is a part of "--help me if you can"; curl#5680 Closes curl#5680

emilengler · 2020-09-04T09:43:06Z

Tests should be fixed now

emilengler · 2020-09-04T09:43:28Z

Sure, that's possible. But are they really stand-alone enough to warrant being individual commits? They certainly don't have proper individual commit messages right now...

I made something that helps in that case. See the commit messages

emilengler · 2020-09-04T11:49:17Z

I think the CI failures are fine. As far as I think they are unrelated to this PR

bagder · 2020-09-04T13:40:07Z

I've taken the liberty of squashing them into three logical commits: docs, tool and tests. Merging now.

Many thanks for your hard work on this!

This commit is a part of "--help me if you can" Closes #5680

emilengler · 2020-09-04T14:22:21Z

I've taken the liberty of squashing them into three logical commits: docs, tool and tests. Merging now.

Many thanks for your hard work on this!

Was a pleasure for me!

dfandrich · 2020-09-07T06:43:10Z

Fantastic work! Thanks for making this dream a reality.

ghost approved these changes Jul 14, 2020

View reviewed changes

emilengler force-pushed the 2020-07-help-me-if-you-can branch from dd873f6 to af4dd10 Compare July 14, 2020 19:31

sterchelen reviewed Jul 17, 2020

View reviewed changes

jay added the cmdline tool label Jul 20, 2020

emilengler mentioned this pull request Jul 21, 2020

tests: Add percent progress bar to runtests.pl #5588

Closed

bagder added the feature-window A merge of this requires an open feature window label Jul 27, 2020

bagder reviewed Aug 13, 2020

View reviewed changes

src/tool_help.c Outdated Show resolved Hide resolved

src/tool_help.c Outdated Show resolved Hide resolved

src/tool_help.c Outdated Show resolved Hide resolved

emilengler force-pushed the 2020-07-help-me-if-you-can branch 2 times, most recently from 32cd3f4 to 0237466 Compare August 13, 2020 12:26

emilengler force-pushed the 2020-07-help-me-if-you-can branch from 0237466 to bf24e39 Compare August 22, 2020 09:26

emilengler force-pushed the 2020-07-help-me-if-you-can branch from bf24e39 to b63bc8a Compare August 24, 2020 06:28

bagder approved these changes Sep 3, 2020

View reviewed changes

src/tool_help.c Outdated Show resolved Hide resolved

src/tool_help.c Outdated Show resolved Hide resolved

src/tool_help.c Show resolved Hide resolved

sterchelen approved these changes Sep 3, 2020

View reviewed changes

emilengler added a commit to emilengler/curl that referenced this pull request Sep 3, 2020

tests: Add tests for new --help

682e4a8

Closes curl#5680

emilengler force-pushed the 2020-07-help-me-if-you-can branch from e63de0e to 682e4a8 Compare September 3, 2020 15:27

emilengler added 5 commits September 4, 2020 11:41

docs: Add categories to all cmdline opts

bbaee16

This is the begin of the implementation of "--help me if you can" by Daniel Fandrich presented at curl up 2020. See: https://bit.ly/2C92PvR This commit is a part of "--help me if you can"; curl#5680

docs: Adjust gen.pl for Category and add listcats

3e58b84

This commit is a part of "--help me if you can"; curl#5680

tool: Update tool_help.c

c1f2572

This commit is a part of "--help me if you can"; curl#5680

help: Implementation of --help me if you can

b02064b

This commit is a part of "--help me if you can"; curl#5680

tests: Add tests for new --help

ac93efd

This commit is a part of "--help me if you can"; curl#5680 Closes curl#5680

emilengler force-pushed the 2020-07-help-me-if-you-can branch from 682e4a8 to ac93efd Compare September 4, 2020 09:42

bagder closed this in 5dddc1d Sep 4, 2020

bagder pushed a commit that referenced this pull request Sep 4, 2020

tool: update --help with categories

aa8777f

This commit is a part of "--help me if you can" Closes #5680

bagder pushed a commit that referenced this pull request Sep 4, 2020

tests: Add tests for new --help

a86cc7e

This commit is a part of "--help me if you can" Closes #5680

gvanem mentioned this pull request Sep 7, 2020

"curl -h" truncated #5547

Closed

apnorton mentioned this pull request Dec 10, 2020

Installer detection of TLS 1.2 support fails for curl 7.73.0+ rust-lang/rustup#2603

Closed

	struct helptxt {
	const char *opt;
	const char *desc;
	curlhelp_t categories;
	};

--help me if you can #5680

--help me if you can #5680

Conversation

emilengler commented Jul 14, 2020 • edited

Introduction

Reviewing

Requested Reviewers

TODO

gvanem commented Jul 14, 2020 • edited

emilengler commented Jul 14, 2020

bagder commented Jul 14, 2020

bagder commented Jul 14, 2020

emilengler commented Jul 14, 2020

emilengler commented Jul 14, 2020

emilengler commented Jul 14, 2020

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilengler commented Aug 13, 2020

emilengler commented Aug 13, 2020

sterchelen commented Aug 21, 2020

emilengler commented Aug 21, 2020

sterchelen commented Aug 21, 2020

emilengler commented Aug 21, 2020

emilengler commented Aug 22, 2020

emilengler commented Aug 23, 2020

bagder commented Aug 23, 2020

emilengler commented Aug 24, 2020

emilengler commented Aug 24, 2020

emilengler commented Aug 24, 2020

emilengler commented Sep 2, 2020

bagder left a comment • edited

Choose a reason for hiding this comment

SuperSandro2000 commented Sep 3, 2020

bagder commented Sep 3, 2020

sterchelen left a comment

Choose a reason for hiding this comment

emilengler commented Sep 3, 2020

emilengler commented Sep 3, 2020

bagder commented Sep 3, 2020

emilengler commented Sep 3, 2020

emilengler commented Sep 3, 2020

bagder commented Sep 3, 2020

emilengler commented Sep 4, 2020

emilengler commented Sep 4, 2020

emilengler commented Sep 4, 2020

bagder commented Sep 4, 2020

emilengler commented Sep 4, 2020

dfandrich commented Sep 7, 2020 via email

emilengler commented Jul 14, 2020 •

edited

gvanem commented Jul 14, 2020 •

edited

bagder left a comment •

edited