Skip to content
This repository was archived by the owner on Dec 12, 2022. It is now read-only.

export convert method to allow downstream adopters to leverage it - #26

Merged
kamiazya merged 1 commit into
ts-graphviz:mainfrom
ChristianMurphy:export-convert-method
Aug 1, 2021
Merged

kamiazya merged 1 commit into
ts-graphviz:mainfrom
ChristianMurphy:export-convert-method

Conversation

@ChristianMurphy

Copy link
Copy Markdown
Contributor

What was a problem

Working on redotjs/redot#22
I'm considering building on top of ts-graphviz.
A particular use case I have is to be able to go from a manually generated/updated AST back to a dot (text) file.
https://github.com/ts-graphviz/ts-graphviz supports this through toDot, but this requires the class based version of the dot graph.
Exporting convert allows for AST -- convert -> Graph -- toDot --> Dot file flow, without copying and pasting convert method into another project.

How this PR fixes the problem

By exporting the method so it can be leveraged down stream

Check lists

  • Test passed
  • Coding style (indentation, etc)

Additional Comments (if any)

An alternative path, to compile the AST directly, rather than going through the class/graph representation, could also work.
But could be more complex, and cause duplication.

@ChristianMurphy ChristianMurphy changed the title export convert method to allow downstream adopters leverage export convert method to allow downstream adopters to leverage it Jul 31, 2021
@kamiazya
kamiazya self-requested a review August 1, 2021 07:13
@kamiazya kamiazya added the enhancement New feature or request label Aug 1, 2021
@kamiazya

kamiazya commented Aug 1, 2021

Copy link
Copy Markdown
Member

@ChristianMurphy
Thank you for considering my project!

I'm pleased that this project can contribute to the unified/redot suite.

I agree with exporting the convert function.
However, I'm also considering changing the namespace to util and exporting, which I might fix after this pull request has been merged.

@kamiazya
kamiazya merged commit 7d59f2a into ts-graphviz:main Aug 1, 2021
@kamiazya

kamiazya commented Aug 1, 2021

Copy link
Copy Markdown
Member

@all-contributors please add @ChristianMurphy for code, ideas, doc

@allcontributors

Copy link
Copy Markdown
Contributor

@kamiazya

I've put up a pull request to add @ChristianMurphy! 🎉

@kamiazya

kamiazya commented Aug 1, 2021

Copy link
Copy Markdown
Member

@ChristianMurphy

The API may change, but I've released the current main branch content as v0.5.0-1, so use it if you need it to get started on development.

@ChristianMurphy
ChristianMurphy deleted the export-convert-method branch August 2, 2021 05:00
@kamiazya

Copy link
Copy Markdown
Member

@ChristianMurphy

With the release of v0.6.0, I released the AST.stringify function that outputs DOT language strings without going through the ts-graphviz model.

With this release, you can now be mentioned in the alternatives.

As pointed out, it can cause duplication, but there are cases in the DOT language where duplication should be tolerated, and I think it can be used in such cases.

For example, when specifying a common attribute for node etc., it is a case where it is affected by the declaration position.

graph {
  node [
    shape=circle;
  ]
  a;

  node [
    shape=diamond;
    color=blue;
  ]
  b;
}

If it fits your design philosophy, please use it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants