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

cli: add play cmd #2242

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

Conversation

turkmenkaan
Copy link

Fix #1213 - Added play command to cli that opens the playground with the given diagram, theme and sketch flag.

Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

nice! Can you add a line to next.md and also update the man page as well?

@@ -38,6 +39,7 @@ Subcommands:
%[1]s layout [name] - Display long help for a particular layout engine, including its configuration options
%[1]s themes - Lists available themes
%[1]s fmt file.d2 ... - Format passed files
%[1]s play file.d2 - Opens the file in playground
Copy link
Collaborator

Choose a reason for hiding this comment

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

playground might be unknown to folks. I'd add a description like Opens the file in the d2 playground, an online web viewer

@@ -154,6 +154,8 @@ func Run(ctx context.Context, ms *xmain.State) (err error) {
return nil
case "fmt":
return fmtCmd(ctx, ms)
case "play":
return playSubcommand(ctx, ms)
Copy link
Collaborator

Choose a reason for hiding this comment

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

playCmd to conform with above naming standards

Comment on lines +14 to +16
if len(ms.Opts.Flags.Args()) != 2 {
return xmain.UsageErrorf("play must be passed one file to open")
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

it should also allow stdin

return err
}

url := fmt.Sprintf("https://play.d2lang.com/?l=&script=%s&sketch=%d&theme=%d&", encoded, sketchNumber, theme)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the l=?

Copy link
Author

Choose a reason for hiding this comment

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

This was in the URL provided in the issue. I'll remove.

d2cli/play.go Show resolved Hide resolved
@turkmenkaan
Copy link
Author

nice! Can you add a line to next.md and also update the man page as well?

I'm assuming you'd consider this a feature? Or should I put it under improvements?

@alixander
Copy link
Collaborator

yeah that's a feature

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.

d2 play input.d2
2 participants