datnguye / dbterd

Generate the ERD as a code from dbt artifacts
https://dbterd.datnguyen.de/
MIT License
198 stars 28 forks source link

fix: mermaid complex types #119

Closed cobac closed 1 month ago

cobac commented 2 months ago

Previously, from a complex nested type like a<b<c d, e f>>, the mermaid adapter would ignore a and capture b, c d and , e f. Now it captures a and b<c d, e f>, which is discarded as far as I can see.

I've simplified the regex expression and tested with the obvious cases, but I'm not sure if the new regex might miss some edge cases. It'd be nice to add more tests if you think I might be missing something :)

I'm pushing twice, without the fix implemented and then with the fix, so that you can see the testing logs in both cases. well workflow needs approval anyway 😅

Thanks for your work!

Checklist

datnguye commented 2 months ago

Hi @cobac thanks for promptly fixing this!

Let me review and get back to you! But first sign, I think it does make sense with the changes when looking into the tests. By the way, @syou6162 do you mind to collab with this PR in the meantime.

Thanks

datnguye commented 2 months ago

With the previous regex: (\w+)<(\w+\s+\w+(\s*,\s*\w+\s+\w+)*)>

image

Current proposal: (\w+)<.*>

image

Given that result, I think the previous regex is done to a specific type struct only, and this proposal is somewhat better if we try to stick to that "only get the root type".

Currently, I'm wondering if we can try to do more as follows:

Let me know what you think? If not, and @syou6162 won't disagree by tmr, I shall release this to v1.15

Thanks

cobac commented 2 months ago

Thanks for the quick reply!

I have no particular opinions about how to represent the complex type in the diagram.

A release soon would be ideal since I'm trying to put this in production 😊 .

datnguye commented 1 month ago

v1.15.0 is now GA 🥳