search

BurntSushi / erd

Translates a plain text description of a relational database schema to a graphical entity-relationship diagram.
The Unlicense
1.79k stars 154 forks source link

Allow spaces in identifiers quoted with backticks #9

Closed eric-brechemier closed 5 years ago

eric-brechemier commented 9 years ago

I have a sqlite database where tables have been imported from CSV files; the header names, which contain spaces, are used as field names.

In SQL, field names are allowed to have spaces when quoted with backticks. I have added a new definition to the grammar, identQuoted, to support this extension of the format.

I have added an example of field and table names with spaces in simple.er. The identifier is correctly rendered with spaces in the picture, and the relationship using a quoted identifier is rendered as expected.

BurntSushi commented 9 years ago

This is a fantastic PR. I love the small diff. :-)

I want to try it before merging, but others looks great! Thanks!

(I see your other PR too. Thanks for splitting them up!)

eric-brechemier commented 9 years ago

Thanks :) I am glad that you are interested in merging this change.

I have pushed some more commits to allow mostly any character in quoted identifiers, from my use case: -?(){}.

I have excluded only '' (quoting delimiter) and ASCII control characters (including newlines). A possible enhancement would be to restrict the range to characters allowed in SQL standard; I have not found a definite answer so far. I also noticed in the meantime that different DBMS use different characters for quoting identifiers; another enhancement may be to allow"and'` as quoting delimiters as well.

I have separated the commits to group related changes and leave you more leeway in what you want to merge. Feel free to cherry-pick.

jlmichael commented 8 years ago

This PR would be huge for me, and any other dev working in a warehousing environment where field names are meant to be human readable, used as column headers in reports, etc. Any chance it can get merged?

taybin commented 6 years ago

What's the holdup on this PR? The README.md conflict? Maybe just fix that and merge it if the original author isn't interested anymore?

mmzx commented 5 years ago

Currently, I am working my way through this PR (the final diff is a bit weird) ...

mmzx commented 5 years ago

After merging this PR the resulted graph (in pdf) matched exactly with the original. The feature works.

mmzx commented 5 years ago

This merge conflict is not against the current master branch anymore. From this point this PR has become "a bit" detached: the README had evolved since then.

eric-brechemier commented 5 years ago

Thank you for seeing this through!