Skip to content
This repository has been 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
merged 1 commit into from
Aug 1, 2021

Conversation

ChristianMurphy
Copy link
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
Copy link
Member

kamiazya commented Aug 1, 2021

@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
Copy link
Member

kamiazya commented Aug 1, 2021

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

@allcontributors
Copy link
Contributor

@kamiazya

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

@kamiazya
Copy link
Member

kamiazya commented Aug 1, 2021

@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
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