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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

First release of the kroki CLI #1

Merged
merged 17 commits into from
Feb 17, 2019
Merged

First release of the kroki CLI #1

merged 17 commits into from
Feb 17, 2019

Conversation

ggrossetie
Copy link
Member

@ggrossetie ggrossetie commented Feb 16, 2019

Usage

kroki convert hello.dot

By default the output format is svg and the diagram type is inferred from the file name. The above command is equivalent to:

kroki convert hello.dot --type dot --format svg

Read from stdin, output to stdout and use svg as the default output format.

cat hello.dot | kroki convert --type dot -

Read from stdin, output to out.png and use png as output format.

cat hello.dot | kroki convert --type dot -o out.png -

Read the file hello.dot and output to out.png.

kroki convert hello.dot -o out.png

Read the file hello.dot and output to stdout.

kroki convert hello.dot -o -

TODO

  • Allow to configure the endpoint (ie. target URL)
    • From environment variable KROKI_ENDPOINT
    • From a config file $HOME/kroki.yml
    • From a config file /etc/kroki.yml
    • From a config file kroki.yml
  • Allow to specify an alternate config file --config /path/to/kroki.yml
  • Add tests 馃槄
  • Allow to configure the out-file

Copy link
Collaborator

@mcorbin mcorbin left a comment

Choose a reason for hiding this comment

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

you committed a main binary.

go.mod Outdated Show resolved Hide resolved
@mcorbin
Copy link
Collaborator

mcorbin commented Feb 16, 2019

You could avoid global variables by defining the flags in the command:

var convertCmd = &cobra.Command{
	Use:           "convert file",
	Short:         "Convert text diagram to image",
        Flags: []cli.Flag{ 
        cli.StringFlag{
			Name:  "config",
			Usage: "description",
                         // use Value: "foo" for default values
         }
},

And then:

config := (cmd.String("config"))

cmd/root.go Outdated Show resolved Hide resolved
cmd/root.go Outdated Show resolved Hide resolved
cmd/root.go Outdated Show resolved Hide resolved
cmd/root.go Show resolved Hide resolved
@ggrossetie
Copy link
Member Author

you committed a main binary.

Oops 馃槄

cmd/convert.go Outdated
}
}

func GraphFormatFromFile(filePath string) (result kroki.GraphFormat, err error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need to name the return values, (kroki.GraphFormat, error) is enough.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought it was mandatory, removed!
Named return values is "just" for documentation purpose ?

Copy link
Collaborator

@mcorbin mcorbin Feb 17, 2019

Choose a reason for hiding this comment

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

Yes, it's for documentation, and you can use them to avoid returning the values: https://tour.golang.org/basics/7

@ggrossetie
Copy link
Member Author

@mcorbin I think we should use io.Reader and io.Writer abstraction to read the diagram source and write the result.
It's also easier to test as you can replace the Reader and/or the Writer with a Buffer.

I think kroki-go should provide the following functions:

func (c *Client) FromReader(reader io.Reader, diagramType DiagramType, imageFormat ImageFormat) (string, error) {}

func (c *Client) Write(writer io.Writer, result string) error {}

@ggrossetie ggrossetie marked this pull request as ready for review February 17, 2019 14:00
@ggrossetie
Copy link
Member Author

@mcorbin Should be ready to go.

After this pull request is merged, I will make a few changes to kroki-go then release a new version and adjust the CLI code.

@ggrossetie ggrossetie merged commit 5195743 into master Feb 17, 2019
@ggrossetie ggrossetie deleted the feat/first-release branch February 17, 2019 20:48
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.

None yet

2 participants