Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/5123 allow attribute names to be escaped on er diagram #5138

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
+26 −0
Open

Bug/5123 allow attribute names to be escaped on er diagram #5138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bf457b3
Allow special chars in attributeName using backticks
ejscribner Dec 12, 2023
f09c1e0
Add an example of special characters in demos/er.html
ejscribner Dec 12, 2023
119b9ef
Test to cover backticked attributeNames
ejscribner Dec 12, 2023
4b9f10f
Merge branch 'develop' into bug/5123_allow-attribute-names-to-be-esca…
ejscribner Jan 22, 2024
96c3fea
Remove quotation marks from BACKTICK_STRING
ejscribner Jan 22, 2024
bc5517a
Test More Special Characters
ejscribner Jan 22, 2024
779178c
Merge branch 'bug/5123_allow-attribute-names-to-be-escaped-on-ER-diag…
ejscribner Jan 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions demos/er.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,18 @@
</pre>
<hr />

<pre class="mermaid">
erDiagram
hotel {
String address
String `city, state`
String `alias?`
String `"checkin"`
String `geo.accuracy`
}
</pre>
<hr />

<script type="module">
import mermaid from './mermaid.esm.mjs';
mermaid.initialize({
Expand Down
2 changes: 2 additions & 0 deletions packages/mermaid/src/diagrams/er/parser/erDiagram.jison
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ accDescr\s*"{"\s* { this.begin("acc_descr_multili
<block>\b((?:PK)|(?:FK)|(?:UK))\b return 'ATTRIBUTE_KEY'
<block>(.*?)[~](.*?)*[~] return 'ATTRIBUTE_WORD';
<block>[\*A-Za-z_][A-Za-z0-9\-_\[\]\(\)]* return 'ATTRIBUTE_WORD'
<block>\`[^`\n]*\` return 'BACKTICK_STRING';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to add backtick within attribute name as well? I am asking because we are trying to allow basically any characters within attribute name with this PR. Minor

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding one more backtick in the name causes a parse error, and adding a second one breaks it into two attributes. For example:

String `geo.`acc`uracy`

becomes:
Screenshot 2024-01-22 at 12 10 04 PM

I'm not sure if there is a way around this behavior, other than maybe changing the escape characters from backticks to something else. Let me know if you have suggestions here 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should be escaped then. Common practice is to use escapement character, like \ (\\ and \` respectively for \ and `) or duplicating characters, like "hello""quotes" (csv uses that approach), or using another character as wrapper (like double backtick for markdown)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure which approach to pick up, they all have some flaws but it's better to think about such a feature beforehand because it is a real pain to introduce something like this later without braking backwards compatibility

<block>\"[^"]*\" return 'COMMENT';
<block>[\n]+ /* nothing */
<block>"}" { this.popState(); return 'BLOCK_STOP'; }
Expand Down Expand Up @@ -138,6 +139,7 @@ attributeType

attributeName
: ATTRIBUTE_WORD { $$=$1; }
| BACKTICK_STRING { $$=$1.replace(/\`/g, ''); }
;

attributeKeyTypeList
Expand Down
12 changes: 12 additions & 0 deletions packages/mermaid/src/diagrams/er/parser/erDiagram.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,18 @@ describe('when parsing ER diagram it...', function () {
}).toThrow();
});
});

it('should allow special characters inside of backticks', () => {
const entity = 'BOOK';
const specialChars = '?!@#$%^&*".,{}[]()';
[...specialChars].forEach((specialChar, i) => {
const attribute = `String \`author${specialChar}firstname\``;
erDiagram.parser.parse(`erDiagram\n${entity} {\n${attribute}}`);
const entities = erDb.getEntities();
expect(Object.keys(entities).length).toBe(1);
expect(entities[entity].attributes.length).toBe(i + 1);
});
});
});

it('should allow an entity with a single attribute to be defined', function () {
Expand Down
Loading