Skip to content

Replace almost all pp.Combine() into pp.Regex()#60

Open
embar- wants to merge 147 commits intoVanderhoof:masterfrom
embar-:combine2regex
Open

Replace almost all pp.Combine() into pp.Regex()#60
embar- wants to merge 147 commits intoVanderhoof:masterfrom
embar-:combine2regex

Conversation

@embar-
Copy link
Copy Markdown

@embar- embar- commented Mar 1, 2025

Majority of pp.Combine() are replaced into pp.Regex() for optimization.
Only this one line is not changed:

enum_name = pp.Combine(name("schema") + '.' + name("name")) | name("name")

This will contribute to #59 solving

Some metrics:

Screenshot_20250301_183646- Screenshot_20250301_183823+
Before After
diagram- diagram+
Before After

Vanderhoof and others added 26 commits August 11, 2024 09:19
- Integrated `prepare_text_for_dbml` into `default_to_str` for consistent handling of special characters in DBML column strings.
- Updated the test suite to include a new test case for handling binary string input (`b'0'`).
…default-escapse

Fix bug in DBML column default rendering with single quotes
…brackets

fix: line breaks in column and index options are allowed (Vanderhoof#48)
fix: quote tablegroup and project name on dbml render Vanderhoof#46
…_other_index_types

feat(indexes): support all Postgresql index types
@Vanderhoof
Copy link
Copy Markdown
Owner

So I took a look, there are 19 failing tests after applying this PR. But what concerns me most is that using pp.Regex negatively impacts readability of the code. After all, why use Pyparsing then, if we end up with regex's anyway. But you are right, current PyDBML performance is terrible.

But thanks a lot for the contribution! I'll have to invest some time into the problem, and will use your changes as a basis

@embar-
Copy link
Copy Markdown
Author

embar- commented Mar 12, 2025

Regarding better readability – you can always add comments for it.

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.

10 participants