Skip to content
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

Expose some private functions for printing help #283

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

atishpatel
Copy link

@atishpatel atishpatel commented Mar 14, 2022

I'm building a cli tool with some custom help output but want it to look similar to the default help output. It would be nice if these private functions were exposed so it's easier to use these help functions to build something custom. This PR doesn't change any functionality and is purely for exposing private functions.

Please feel free to edit or let me know what to change. I can also extract these to a separate package if that is more desirable.

P.S. Thanks for building and maintaining this awesome library. 🙏

help.go Outdated
}
return w.Write(ctx.Stdout)
}

func printApp(w *helpWriter, app *Application) {
func PrintApp(w *HelpWriter, app *Application) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it going to be useful to expose these Print* functions? Unless you want segments in exactly the same format, I'm not sure it will be that useful. WDYT?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I didn't want to flood the public functions but the format of these functions is a bit confusing. My main use case is I want to get the flags output and the arguments output without anything else. Here is the output i'm going for and I would like to get the sections under ARUGMENTS and FLAGS with a function call instead of trying to manipulate the stdout.

─ sq ski:debug:app --help                                                      ─╯
USAGE
   sq ski:debug:app <app-name> --env <ENV> --region <REGION>
   sq ski:debug:app -a <app-name> --env <ENV> --region <REGION>

ARGUMENTS
   [<app-arg>]

FLAGS
   -h, --help             Show context-sensitive help.
       --summary
   -a, --app=STRING
   -e, --env=STRING       environment i.e. dev, development, prod, production
   -r, --region=STRING    region i.e. west, us-west-2, east, us-east-1

SUMMARY
   Run an automatic debug on an application that will help highlight common/known issues.

SUPPORT
   Ask in #kubernetes

Note the USAGE, SUMMARY, and SUPPORT sections are static strings created by engineers.

Ideally, i could just call WritePositionals and WriteFlagsForCommand to a string writer get the output i want and plug it in where i need.

help.go Outdated
@@ -174,24 +174,8 @@ func printNodeDetail(w *helpWriter, node *Node, hide bool) {
w.Print("Arguments:")
writePositionals(w.Indent(), node.Positional)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

writePositionals -> PrintPositionals?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went in the opposite direction since it made more sense to me. Print -> Write for the new public funcs since it writes to WriteHelper but i can switch to print if you want.

help.go Outdated
}
}

func writeCommandList(cmds []*Node, iw *helpWriter) {
func writeCommandList(cmds []*Node, iw *HelpWriter) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if some of these functions are exposed, they probably all should be no?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was avoiding this to prevent too many public functions but i went ahead and added these too.

@alecthomas
Copy link
Owner

Overall LGTM, but let's move it into a separate package, perhaps helpwriter.

@alecthomas
Copy link
Owner

Ping on last comment?

@atishpatel
Copy link
Author

atishpatel commented Apr 10, 2022

Sorry been a little distracted and am current in India for a few weeks. I'll get around to this in May. Feel free to take charge of the PR if you want to make this happen sooner.

@alecthomas
Copy link
Owner

All good, no hurry :)

@atishpatel
Copy link
Author

So, i tried to split apart the code but it doesn't really work nicely. For example, HelpWriter depends on Flag, HelpValueFormatter, HelpOptions, Node, and Positional but there isn't really an easy way to avoid a circular dependency without breaking changes. If i move these structs into a shared types package, it would be a breaking change. I could add interfaces that return each value HelpWriter needs via a method call and then add the methods to the objects but that is pretty ugly imo. Do you want me to go down the interfaces route or would you rather leave this as is?

@atishpatel
Copy link
Author

One alternative thing i can try to do is to make the WriteX functions into methods so func WriteFlags(w *HelpWriter, groups [][]*Flag) would become func (w *HelpWriter) WriteFlags(groups [][]*Flag). This would reduce the global space pollution.

@atishpatel
Copy link
Author

I did what i was saying above with making functions into methods and pushed the code. LMK if you like it and hopefully the linter passes. 🤞

@Dids
Copy link

Dids commented Aug 7, 2022

Any news on this? I'm assuming this would help solve #33?

EDIT: Commented on a workaround here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants