Add mdox-gen-file by saswatamcode · Pull Request #19 · bwplotka/mdox

saswatamcode · 2021-03-27T13:05:24Z

This PR adds an mdox-gen-file option which allows fetching code from local files into documentation like below. Addresses #18

```go mdox-gen-file="main.go" mdox-gen-lines=47-56

This results in the following when formatted,

```go mdox-gen-file="main.go" mdox-gen-lines=47-56
	switch logFormat {
	case logFormatJson:
		return level.NewFilter(log.NewJSONLogger(log.NewSyncWriter(os.Stderr)), lvl)
	case logFormatLogfmt:
		return level.NewFilter(log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr)), lvl)
	case logFormatCLILog:
		fallthrough
	default:
		return level.NewFilter(clilog.New(log.NewSyncWriter(os.Stderr)), lvl)
	}
. . .

If no line numbers are given like below, entire file is brought in by default (useful for blogs),

```go mdox-gen-file="main.go" mdox-gen-lines=47-56

bwplotka

Amazing! Some suggestions, plus some test needed, otherwise LGTM!

pkg/mdformatter/mdgen/mdgen.go

bwplotka · 2021-04-17T18:10:21Z

pkg/mdformatter/mdgen/mdgen.go

+		case true:
+			val := strings.Split(linesToCopy, "-")
+			start, errStart := strconv.Atoi(val[0])
+			if errStart != nil {


Suggested change

if errStart != nil {

if err != nil {

Usually it's fine to shadow err variable if in short scope.

bwplotka · 2021-04-17T18:10:56Z

pkg/mdformatter/mdgen/mdgen.go

+
+		switch lineOk {
+		case true:
+			val := strings.Split(linesToCopy, "-")


I would propose the same semantics as Go slice, WDYT?

Suggested change

val := strings.Split(linesToCopy, "-")

val := strings.Split(linesToCopy, ":")

Sure! I think this would be better!

bwplotka · 2021-04-17T18:11:27Z

pkg/mdformatter/mdgen/mdgen.go

+					text = append(text, "\n"...)
+				}
+				line++
+			}


Let's check scanner.Err() after this loop (same below)

bwplotka · 2021-04-17T18:12:07Z

pkg/mdformatter/mdgen/mdgen.go

+				}
+				line++
+			}
+		case false:


Can we reuse the above, but just use start 0 and end = math.MaxInt64? (:

Sure. Will make the necessary changes!

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

bwplotka

Nice, almost there 🤗

bwplotka · 2021-04-21T11:53:02Z

pkg/mdformatter/mdgen/mdgen.go

+			line++
+		}
+		if err := scanner.Err(); err != nil {
+			return nil, errors.Errorf("scanner couldn't read: %v", err)


Suggested change

return nil, errors.Errorf("scanner couldn't read: %v", err)

return nil, errors.Wrap(err, "scanner read")

bwplotka · 2021-04-21T11:53:38Z

pkg/mdformatter/mdgen/mdgen_test.go

+)
+
 func TestFormat_FormatSingle_CodeBlockTransformer(t *testing.T) {
 	f := mdformatter.New(context.Background(), mdformatter.WithCodeBlockTransformer(NewCodeBlockTransformer()))


Why not putting this in our e2e framework with input output markdown files? (:

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

bwplotka

Just noticed 🙈

To make sure we have one way of doing things... what's stops us without this PR to have mdox doing the same with:

```go mdox-gen-exec="sed -n '3,6p' main.go"
```

🤔

While this PR is great, I fear it would be best to decouple this logic and allow better tools sed to be used 🙈 WDYT?

IF you agree, we could add test case with sed to ensure this works instead of full code.

saswatamcode · 2021-05-12T08:31:19Z

Sure! Will close this and open a new PR with the sed test case!

bwplotka reviewed Apr 17, 2021

View reviewed changes

saswatamcode added 2 commits April 18, 2021 18:22

Add mdox-gen-file

bdb477d

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

Refactor and add test

145c26b

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

saswatamcode force-pushed the file-gen branch from 84ce40d to 145c26b Compare April 18, 2021 12:53

saswatamcode requested a review from bwplotka April 18, 2021 12:55

bwplotka approved these changes Apr 21, 2021

View reviewed changes

Refactor test

8cbec7a

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

bwplotka requested changes May 12, 2021

View reviewed changes

saswatamcode closed this May 12, 2021

saswatamcode mentioned this pull request May 13, 2021

Add sed testcase #25

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mdox-gen-file#19

Add mdox-gen-file#19
saswatamcode wants to merge 3 commits intobwplotka:masterfrom
saswatamcode:file-gen

saswatamcode commented Mar 27, 2021

Uh oh!

bwplotka left a comment

Uh oh!

Uh oh!

bwplotka Apr 17, 2021

Uh oh!

bwplotka Apr 17, 2021

Uh oh!

saswatamcode Apr 18, 2021

Uh oh!

bwplotka Apr 17, 2021

Uh oh!

bwplotka Apr 17, 2021

Uh oh!

saswatamcode Apr 18, 2021

Uh oh!

bwplotka left a comment

Uh oh!

bwplotka Apr 21, 2021

Uh oh!

bwplotka Apr 21, 2021

Uh oh!

bwplotka left a comment

Uh oh!

saswatamcode commented May 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	val := strings.Split(linesToCopy, "-")
	val := strings.Split(linesToCopy, ":")

	return nil, errors.Errorf("scanner couldn't read: %v", err)
	return nil, errors.Wrap(err, "scanner read")

Conversation

saswatamcode commented Mar 27, 2021

Uh oh!

bwplotka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwplotka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwplotka left a comment

Choose a reason for hiding this comment

Uh oh!

saswatamcode commented May 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants