Fix mdox-gen-exec issues with --help commands#17
Conversation
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
bwplotka
left a comment
There was a problem hiding this comment.
Epic work, thanks!
Wonder... what would be the best experience here. Some suggestions:
- What about opposite: have option
expect-exit-code=2and by default don't check exit code? - Can we run quick test to ensure this? 🤗
pkg/mdformatter/mdgen/mdgen.go
Outdated
| infoStringKeyLang = "mdox-gen-lang" | ||
| infoStringKeyType = "mdox-gen-type" | ||
| infoStringKeyExec = "mdox-gen-exec" | ||
| skipExitCodeCheck = "mdox-nonzero-exit-code" |
There was a problem hiding this comment.
I wonder... should we just don't care about exit codes? 🤔
There was a problem hiding this comment.
Also variable name is not telling the same as string
There was a problem hiding this comment.
Initially, I did think that not checking for exit codes would be better, but then user wouldn’t be able to tell which command in a md code block had an actual error and that might get formatted without the user noticing. For eg npm —versionn is a typo which would come up as an error if we check the exit code. However, if we skip that then something entirely different than the version number might get formatted.
Would the better option be to have an expect-exit-code flag which will be matched if non 0 exit code occurs?
There was a problem hiding this comment.
I think tag makes sense more like you did.
So what about by default expecting status code 0, but allow expect-exit-code=X to change that? 🤗
There was a problem hiding this comment.
Yup! That would be best. Implementing that along with a quick test! 😊
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
bwplotka
left a comment
There was a problem hiding this comment.
Amazing, looks good. Just some suggestions still.
pkg/mdformatter/mdgen/mdgen.go
Outdated
| infoStringKeyLang = "mdox-gen-lang" | ||
| infoStringKeyType = "mdox-gen-type" | ||
| infoStringKeyExec = "mdox-gen-exec" | ||
| infoStringKeyExitCode = "expect-exit-code" |
There was a problem hiding this comment.
| infoStringKeyExitCode = "expect-exit-code" | |
| infoStringKeyExitCode = "mdox-expect-exit-code" |
pkg/mdformatter/mdgen/mdgen.go
Outdated
| infoStringAttr := map[string]string{} | ||
| for i, field := range infoFiels { | ||
| if val := strings.Split(field, "="); val[0] == infoStringKeyLang || val[0] == infoStringKeyType || val[0] == infoStringKeyExec { | ||
| if val := strings.Split(field, "="); val[0] == infoStringKeyLang || val[0] == infoStringKeyType || val[0] == infoStringKeyExec || val[0] == infoStringKeyExitCode { |
pkg/mdformatter/mdgen/mdgen.go
Outdated
| } | ||
| if len(val) != 2 { | ||
| return nil, errors.Errorf("got %q without variable. Expected format is e.g ```yaml %q=<value> %q=<value2> . Got info string %q", val[0], infoStringKeyLang, infoStringKeyType, string(infoString)) | ||
| return nil, errors.Errorf("got %q without variable. Expected format is e.g ```yaml %q=<value> %q=<value2> %q=<value2>. Got info string %q", val[0], infoStringKeyLang, infoStringKeyExitCode, infoStringKeyType, string(infoString)) |
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
This PR fixes the issue where commands with non 0 exit codes (usually
--helpcommands) fail to format. Fixes #10Before this fix, the markdown below results in an error,
A workaround is using something like
mdox-gen-exec="sh -c 'go --help || exit 0'"but this is not preferable.With this fix, a flag can be passed to allow non zero exit codes to format successfully, like below,
This would also be suitable in case an error code block example needs to be generated.