From ab81a15cfb2fca1a4f351fb04adaadba34e1bee9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlio=20C=C3=A9sar=20Batista?= Date: Fri, 14 Apr 2023 19:44:01 -0300 Subject: [PATCH] pr comments --- d2cli/export.go | 30 ++++++++----- d2cli/export_test.go | 93 ++++++++++++++++++++++++--------------- d2cli/main.go | 14 +++--- e2etests-cli/main_test.go | 8 ++++ 4 files changed, 93 insertions(+), 52 deletions(-) diff --git a/d2cli/export.go b/d2cli/export.go index a1e84db6f..845b215e4 100644 --- a/d2cli/export.go +++ b/d2cli/export.go @@ -1,34 +1,42 @@ package d2cli import ( + "fmt" "path/filepath" ) type exportExtension string -const GIF = ".gif" -const PNG = ".png" -const PPTX = ".pptx" -const PDF = ".pdf" -const SVG = ".svg" +const GIF exportExtension = ".gif" +const PNG exportExtension = ".png" +const PPTX exportExtension = ".pptx" +const PDF exportExtension = ".pdf" +const SVG exportExtension = ".svg" -var KNOWN_EXTENSIONS = []string{SVG, PNG, PDF, PPTX, GIF} +var SUPPORTED_EXTENSIONS = []exportExtension{SVG, PNG, PDF, PPTX, GIF} -func getExportExtension(outputPath string) exportExtension { +func getExportExtension(outputPath string) (exportExtension, error) { ext := filepath.Ext(outputPath) - for _, kext := range KNOWN_EXTENSIONS { - if kext == ext { - return exportExtension(ext) + if ext == ".ppt" { + return "", fmt.Errorf("D2 does not support ppt exports, did you mean \"pptx\"?") + } + for _, kext := range SUPPORTED_EXTENSIONS { + if kext == exportExtension(ext) { + return exportExtension(ext), nil } } // default is svg - return exportExtension(SVG) + return exportExtension(SVG), nil } func (ex exportExtension) supportsAnimation() bool { return ex == SVG || ex == GIF } +func (ex exportExtension) requiresAnimationInterval() bool { + return ex == GIF +} + func (ex exportExtension) requiresPNGRenderer() bool { return ex == PNG || ex == PDF || ex == PPTX || ex == GIF } diff --git a/d2cli/export_test.go b/d2cli/export_test.go index c3c6e0434..7d84c9b2d 100644 --- a/d2cli/export_test.go +++ b/d2cli/export_test.go @@ -8,66 +8,87 @@ import ( func TestOutputFormat(t *testing.T) { type testCase struct { - outputPath string - extension exportExtension - supportsDarkTheme bool - supportsAnimation bool - requiresPngRender bool + outputPath string + extension exportExtension + supportsDarkTheme bool + supportsAnimation bool + requiresAnimationInterval bool + requiresPngRender bool } testCases := []testCase{ { - outputPath: "/out.svg", - extension: ".svg", - supportsDarkTheme: true, - supportsAnimation: true, - requiresPngRender: false, + outputPath: "/out.svg", + extension: SVG, + supportsDarkTheme: true, + supportsAnimation: true, + requiresAnimationInterval: false, + requiresPngRender: false, }, { // assumes SVG by default - outputPath: "/out", - extension: ".svg", - supportsDarkTheme: true, - supportsAnimation: true, - requiresPngRender: false, + outputPath: "/out", + extension: SVG, + supportsDarkTheme: true, + supportsAnimation: true, + requiresAnimationInterval: false, + requiresPngRender: false, }, { - outputPath: "-", - extension: ".svg", - supportsDarkTheme: true, - supportsAnimation: true, - requiresPngRender: false, + outputPath: "-", + extension: SVG, + supportsDarkTheme: true, + supportsAnimation: true, + requiresAnimationInterval: false, + requiresPngRender: false, }, { - outputPath: "/out.png", - extension: ".png", - supportsDarkTheme: false, - supportsAnimation: false, - requiresPngRender: true, + outputPath: "/out.png", + extension: PNG, + supportsDarkTheme: false, + supportsAnimation: false, + requiresAnimationInterval: false, + requiresPngRender: true, }, { - outputPath: "/out.pptx", - extension: ".pptx", - supportsDarkTheme: false, - supportsAnimation: false, - requiresPngRender: true, + outputPath: "/out.pptx", + extension: PPTX, + supportsDarkTheme: false, + supportsAnimation: false, + requiresAnimationInterval: false, + requiresPngRender: true, }, { - outputPath: "/out.pdf", - extension: ".pdf", - supportsDarkTheme: false, - supportsAnimation: false, - requiresPngRender: true, + outputPath: "/out.pdf", + extension: PDF, + supportsDarkTheme: false, + supportsAnimation: false, + requiresAnimationInterval: false, + requiresPngRender: true, + }, + { + outputPath: "/out.gif", + extension: GIF, + supportsDarkTheme: false, + supportsAnimation: true, + requiresAnimationInterval: true, + requiresPngRender: true, }, } for _, tc := range testCases { tc := tc t.Run(tc.outputPath, func(t *testing.T) { - extension := getExportExtension(tc.outputPath) + extension, err := getExportExtension(tc.outputPath) + assert.NoError(t, err) assert.Equal(t, tc.extension, extension) assert.Equal(t, tc.supportsAnimation, extension.supportsAnimation()) assert.Equal(t, tc.supportsDarkTheme, extension.supportsDarkTheme()) assert.Equal(t, tc.requiresPngRender, extension.requiresPNGRenderer()) }) } + + // unsupported format + _, err := getExportExtension("/out.ppt") + assert.NotNil(t, err) + assert.Equal(t, "D2 does not support ppt exports, did you mean \"pptx\"?", err.Error()) } diff --git a/d2cli/main.go b/d2cli/main.go index 804505e5d..e73b35978 100644 --- a/d2cli/main.go +++ b/d2cli/main.go @@ -187,12 +187,15 @@ func Run(ctx context.Context, ms *xmain.State) (err error) { inputPath = filepath.Join(inputPath, "index.d2") } } - outputFormat := getExportExtension(outputPath) + outputFormat, err := getExportExtension(outputPath) + if err != nil { + return xmain.UsageErrorf(err.Error()) + } if outputPath != "-" { outputPath = ms.AbsPath(outputPath) if *animateIntervalFlag > 0 && !outputFormat.supportsAnimation() { return xmain.UsageErrorf("-animate-interval can only be used when exporting to SVG or GIF.\nYou provided: %s", filepath.Ext(outputPath)) - } else if *animateIntervalFlag <= 0 && outputFormat == GIF { + } else if *animateIntervalFlag <= 0 && outputFormat.requiresAnimationInterval() { return xmain.UsageErrorf("-animate-interval must be greater than 0 for GIF outputs.\nYou provided: %d", *animateIntervalFlag) } } @@ -353,7 +356,7 @@ func compile(ctx context.Context, ms *xmain.State, plugin d2plugin.Plugin, rende return nil, false, err } - ext := getExportExtension(outputPath) + ext, _ := getExportExtension(outputPath) switch ext { case GIF: svg, pngs, err := renderPNGsForGIF(ctx, ms, plugin, renderOpts, ruler, page, diagram) @@ -424,7 +427,7 @@ func compile(ctx context.Context, ms *xmain.State, plugin d2plugin.Plugin, rende var out []byte if len(boards) > 0 { out = boards[0] - if animateInterval > 0 && ext != GIF { + if animateInterval > 0 { out, err = d2animate.Wrap(diagram, boards, renderOpts, int(animateInterval)) if err != nil { return nil, false, err @@ -622,7 +625,8 @@ func render(ctx context.Context, ms *xmain.State, compileDur time.Duration, plug } func _render(ctx context.Context, ms *xmain.State, plugin d2plugin.Plugin, opts d2svg.RenderOpts, outputPath string, bundle, forceAppendix bool, page playwright.Page, ruler *textmeasure.Ruler, diagram *d2target.Diagram) ([]byte, error) { - toPNG := getExportExtension(outputPath) == PNG + ext, _ := getExportExtension(outputPath) + toPNG := ext == PNG svg, err := d2svg.Render(diagram, &d2svg.RenderOpts{ Pad: opts.Pad, Sketch: opts.Sketch, diff --git a/e2etests-cli/main_test.go b/e2etests-cli/main_test.go index 63a58f178..8ba39dbe3 100644 --- a/e2etests-cli/main_test.go +++ b/e2etests-cli/main_test.go @@ -246,6 +246,14 @@ layers: { testdataIgnoreDiff(t, ".pdf", pdf) }, }, + { + name: "export_ppt", + run: func(t *testing.T, ctx context.Context, dir string, env *xos.Env) { + writeFile(t, dir, "x.d2", `x -> y`) + err := runTestMain(t, ctx, dir, env, "x.d2", "x.ppt") + assert.ErrorString(t, err, `failed to wait xmain test: e2etests-cli/d2: bad usage: D2 does not support ppt exports, did you mean "pptx"?`) + }, + }, { name: "how_to_solve_problems_pptx", skipCI: true,