-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add synopsis autogenerator #2785
Conversation
oooOooo, fancy! you should throw in a couple tests |
I will. |
@whyrusleeping tell me if that is enough, if it is not then I will work more on it. |
That works for me. |
@RichardLitt I would like to hear your opinion and also possible improvements if you see any. |
This looks great to me! I can't think of any improvements, at the moment -- would be good to run this and add the output for each command to this PR? That way we can see how it works and upload the changes now, as opposed to after merging. |
If we remove synopsis from command now it will be replaced with the generated one. We could remove all of them from code, generate all help texts and go through them improving them (with |
Sounds good to me. I think that makes more sense than merging this before doing that. |
All synopsis in existence: https://gist.github.com/Kubuxu/09b0ef346b469001ca51f6e345d79a33 |
8644a2a
to
e5010e2
Compare
Synopsis lines can get quite long, should we do anything about it?
|
Now with synopsis generator also would be great to go through codebase and make longer options definitions the primary ones ( Or I could also check which is the longest and use that one in the generator. What do you think? |
e5010e2
to
15f05d2
Compare
@Kubuxu i think we should standardize on the options being specified a certain way. Making the 'first' one the primary, and the second the alternate |
1df32ed
to
c509973
Compare
WHOO, finally tests are green. Will work on moving those parameters around now. |
@RichardLitt re: gist: I could make the description in case of bool options a |
That should work. The default value should always be specified by .Default(false) for bools - we can see if there's any issue with that once generated? Happy to go through each and nitpick. |
Only problem I have is how to show it? |
How about just Like this:
|
the synopsis generation LGTM. I'm still not super sure what i think of the |
002930d
to
61b832b
Compare
61b832b
to
666a788
Compare
@whyrusleeping removed the no- prefixing. |
@Kubuxu i pulled this down to try it out, but |
(the few commands i ran that didnt hang looked great 👍 ) |
@Kubuxu could you rebase this one? |
Generates synopsis automagically License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
As synopsis is only one line long it is overkill License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
666a788
to
1c0f18d
Compare
Rebased. |
LGTM! |
YASSSSSSSS |
Never, please never use |
I think we need our short usage back, like
|
this could be explicitly chosen per command, and not auto generated, it's ok. |
Generates synopsis automagically
Few examples:
ipfs add [--recursive | -r] [--quiet | -q] [--silent] [--[no-]progress] [--trickle | -t] [--only-hash | -n] [--wrap-with-directory | -w] [--hidden | -H] [--chunker=<chunker> | -s] [--[no-]pin] [--] <path>...
ipfs pin ls [--type=<type> | -t] [--quiet | -q] [--] [<ipfs-path>...]
License: MIT
Signed-off-by: Jakub Sztandera kubuxu@protonmail.ch