Fix mdox-gen-exec issues with --help commands #17

saswatamcode · 2021-03-18T07:10:48Z

This PR fixes the issue where commands with non 0 exit codes (usually --help commands) fail to format. Fixes #10
Before this fix, the markdown below results in an error,

```bash mdox-gen-exec="go --help"
.
.
.
error:  first formatting phase for /Users/saswatamukherjee/web/mdox/test.md: run go --help: exit status 2

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,

```bash mdox-nonzero-exit-code mdox-gen-exec="go --help"
Go is a tool for managing Go source code.

Usage:

	go <command> [arguments]
...

This would also be suitable in case an error code block example needs to be generated.

Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>

bwplotka

Epic work, thanks!

Wonder... what would be the best experience here. Some suggestions:

What about opposite: have option expect-exit-code = 2 and by default don't check exit code?
Can we run quick test to ensure this? 🤗

bwplotka · 2021-03-19T15:01:22Z

pkg/mdformatter/mdgen/mdgen.go

@@ -19,6 +19,7 @@ const (
 	infoStringKeyLang = "mdox-gen-lang"
 	infoStringKeyType = "mdox-gen-type"
 	infoStringKeyExec = "mdox-gen-exec"
+	skipExitCodeCheck = "mdox-nonzero-exit-code"


I wonder... should we just don't care about exit codes? 🤔

Also variable name is not telling the same as string

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?

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? 🤗

Yup! That would be best. Implementing that along with a quick test! 😊

Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>

bwplotka

Amazing, looks good. Just some suggestions still.

bwplotka · 2021-03-25T10:30:33Z

pkg/mdformatter/mdgen/mdgen.go

+	infoStringKeyLang     = "mdox-gen-lang"
+	infoStringKeyType     = "mdox-gen-type"
+	infoStringKeyExec     = "mdox-gen-exec"
+	infoStringKeyExitCode = "expect-exit-code"


Suggested change

infoStringKeyExitCode = "expect-exit-code"

infoStringKeyExitCode = "mdox-expect-exit-code"

bwplotka · 2021-03-25T10:30:49Z

pkg/mdformatter/mdgen/mdgen.go

@@ -38,12 +40,12 @@ func (t *genCodeBlockTransformer) TransformCodeBlock(ctx mdformatter.SourceConte
 	}
 	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 {


Time for some switch here (:

bwplotka · 2021-03-25T10:30:59Z

pkg/mdformatter/mdgen/mdgen.go

 			if i == 0 {
 				return nil, errors.Errorf("missing language info in fenced code block. Got info string %q", string(infoString))
 			}
 			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>

bwplotka

Nice, thanks!

LGTM 💪🏽

Make mdox-gen-exec execute --help

ea57853

Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>

bwplotka reviewed Mar 19, 2021

View reviewed changes

Add expect-exit-code tag

1484fee

Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>

saswatamcode requested a review from bwplotka March 21, 2021 13:43

bwplotka approved these changes Mar 25, 2021

View reviewed changes

Add switch

f1dc342

Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>

bwplotka reviewed Mar 25, 2021

View reviewed changes

bwplotka merged commit 1d5f977 into bwplotka:master Mar 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix mdox-gen-exec issues with --help commands #17

Fix mdox-gen-exec issues with --help commands #17

saswatamcode commented Mar 18, 2021

bwplotka left a comment

bwplotka Mar 19, 2021

bwplotka Mar 19, 2021

saswatamcode Mar 19, 2021

bwplotka Mar 20, 2021

saswatamcode Mar 20, 2021

bwplotka left a comment

bwplotka Mar 25, 2021

bwplotka Mar 25, 2021

bwplotka Mar 25, 2021

bwplotka left a comment

	infoStringKeyExitCode = "expect-exit-code"
	infoStringKeyExitCode = "mdox-expect-exit-code"

Fix mdox-gen-exec issues with --help commands #17

Fix mdox-gen-exec issues with --help commands #17

Conversation

saswatamcode commented Mar 18, 2021

bwplotka left a 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

bwplotka left a 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

bwplotka left a comment

Choose a reason for hiding this comment