fix: bug #2346 "ER-attribute comments not work"

cypress/integration/rendering/erDiagram.spec.js

@@ -183,11 +183,58 @@ describe('Entity Relationship Diagram', () => {
     cy.get('svg');
   });
 
+  it('should render entities with keys', () => {
+    renderGraph(
+      `
+    erDiagram
+      AUTHOR_WITH_LONG_ENTITY_NAME {
+        string name PK
+      }
+      AUTHOR_WITH_LONG_ENTITY_NAME }|..|{ BOOK : writes
+      BOOK { 
+          float price
+          string author FK 
+          string title PK
+        }
+      `,
+      { logLevel: 1 }
+    );
+    cy.get('svg');
+  });
+
+  it('should render entities with comments', () => {
+    renderGraph(
+      `
+    erDiagram
+      AUTHOR_WITH_LONG_ENTITY_NAME {
+        string name "comment"
+      }
+      AUTHOR_WITH_LONG_ENTITY_NAME }|..|{ BOOK : writes
+      BOOK { 
+          string author 
+          string title "author comment"
+          float price "price comment"
+        }
+      `,
+      { logLevel: 1 }
+    );
+    cy.get('svg');
+  });
+
   it('should render entities with keys and comments', () => {
     renderGraph(
       `
     erDiagram
-        BOOK { string title PK "comment"}
+      AUTHOR_WITH_LONG_ENTITY_NAME {
+        string name PK "comment"
+      }
+      AUTHOR_WITH_LONG_ENTITY_NAME }|..|{ BOOK : writes
+      BOOK { 
+          string description
+          float price "price comment"
+          string title PK "title comment"
+          string author FK 
+        }
       `,
       { logLevel: 1 }
     );

docs/entityRelationshipDiagram.md

@@ -137,20 +137,20 @@ The `type` and `name` values must begin with an alphabetic character and may con
 
 #### Attribute Keys and Comments
 
-Attributes may also have a `key` or comment defined. Keys can be "PK" or "FK", for Primary Key or Foreign Key. And a `comment` is defined by quotes at the end of an attribute. Comments themselves cannot have quote characters in them.
+Attributes may also have a `key` or comment defined. Keys can be "PK" or "FK", for Primary Key or Foreign Key. And a `comment` is defined by double quotes at the end of an attribute. Comments themselves cannot have double-quote characters in them.
 
 ```mermaid-example
 erDiagram
     CAR ||--o{ NAMED-DRIVER : allows
     CAR {
-        string allowedDriver FK 'The license of the allowed driver'
+        string allowedDriver FK "The license of the allowed driver"
         string registrationNumber
         string make
         string model
     }
     PERSON ||--o{ NAMED-DRIVER : is
     PERSON {
-        string driversLicense PK 'The license #'
+        string driversLicense PK "The license #"
         string firstName
         string lastName
         int age

src/diagrams/er/erRenderer.js

@@ -40,7 +40,6 @@ const drawAttributes = (groupNode, entityTextNode, attributes) => {
   const attributeNodes = []; // Intermediate storage for attribute nodes created so that we can do a second pass
   let hasKeyType = false;
   let hasComment = false;
-  let maxWidth = 0;
   let maxTypeWidth = 0;
   let maxNameWidth = 0;
   let maxKeyWidth = 0;
@@ -48,9 +47,19 @@ const drawAttributes = (groupNode, entityTextNode, attributes) => {
   let cumulativeHeight = labelBBox.height + heightPadding * 2;
   let attrNum = 1;
 
+  // Check to see if any of the attributes has a key or a comment
+  attributes.forEach((item) => {
+    if (item.attributeKeyType !== undefined) {
+      hasKeyType = true;
+    }
+
+    if (item.attributeComment !== undefined) {
+      hasComment = true;
+    }
+  });
+
   attributes.forEach((item) => {
     const attrPrefix = `${entityTextNode.node().id}-attr-${attrNum}`;
-    let nodeWidth = 0;
     let nodeHeight = 0;
 
     // Add a text node for the attribute type
@@ -91,16 +100,14 @@ const drawAttributes = (groupNode, entityTextNode, attributes) => {
     const nameBBox = nameNode.node().getBBox();
     maxTypeWidth = Math.max(maxTypeWidth, typeBBox.width);
     maxNameWidth = Math.max(maxNameWidth, nameBBox.width);
-    nodeWidth += typeBBox.width;
-    nodeWidth += nameBBox.width;
 
     nodeHeight = Math.max(typeBBox.height, nameBBox.height);
 
-    if (hasKeyType || item.attributeKeyType !== undefined) {
+    if (hasKeyType) {
       const keyTypeNode = groupNode
         .append('text')
         .attr('class', 'er entityLabel')
-        .attr('id', `${attrPrefix}-name`)
+        .attr('id', `${attrPrefix}-key`)
         .attr('x', 0)
         .attr('y', 0)
         .attr('dominant-baseline', 'middle')
@@ -113,17 +120,15 @@ const drawAttributes = (groupNode, entityTextNode, attributes) => {
 
       attributeNode.kn = keyTypeNode;
       const keyTypeBBox = keyTypeNode.node().getBBox();
-      nodeWidth += keyTypeBBox.width;
+      maxKeyWidth = Math.max(maxKeyWidth, keyTypeBBox.width);
-      maxKeyWidth = Math.max(maxKeyWidth, nodeWidth);
       nodeHeight = Math.max(nodeHeight, keyTypeBBox.height);
-      hasKeyType = true;
     }
 
-    if (hasComment || item.attributeComment !== undefined) {
+    if (hasComment) {
       const commentNode = groupNode
         .append('text')
         .attr('class', 'er entityLabel')
-        .attr('id', `${attrPrefix}-name`)
+        .attr('id', `${attrPrefix}-comment`)
         .attr('x', 0)
         .attr('y', 0)
         .attr('dominant-baseline', 'middle')
@@ -136,25 +141,35 @@ const drawAttributes = (groupNode, entityTextNode, attributes) => {
 
       attributeNode.cn = commentNode;
       const commentNodeBBox = commentNode.node().getBBox();
-      nodeWidth += commentNodeBBox.width;
+      maxCommentWidth = Math.max(maxCommentWidth, commentNodeBBox.width);
-      maxCommentWidth = Math.max(nodeWidth, nameBBox.width);
       nodeHeight = Math.max(nodeHeight, commentNodeBBox.height);
-      hasComment = true;
     }
 
     attributeNode.height = nodeHeight;
     // Keep a reference to the nodes so that we can iterate through them later
     attributeNodes.push(attributeNode);
-    maxWidth = Math.max(maxWidth, nodeWidth);
     cumulativeHeight += nodeHeight + heightPadding * 2;
     attrNum += 1;
   });
 
+  let widthPaddingFactor = 4;
+  if (hasKeyType) {
+    widthPaddingFactor += 2;
+  }
+  if (hasComment) {
+    widthPaddingFactor += 2;
+  }
+
+  const maxWidth = maxTypeWidth + maxNameWidth + maxKeyWidth + maxCommentWidth;
+
   // Calculate the new bounding box of the overall entity, now that attributes have been added
   const bBox = {
     width: Math.max(
       conf.minEntityWidth,
-      Math.max(labelBBox.width + conf.entityPadding * 2, maxWidth + widthPadding * 4)
+      Math.max(
+        labelBBox.width + conf.entityPadding * 2,
+        maxWidth + widthPadding * widthPaddingFactor
+      )
     ),
     height:
       attributes.length > 0
@@ -162,10 +177,13 @@ const drawAttributes = (groupNode, entityTextNode, attributes) => {
         : Math.max(conf.minEntityHeight, labelBBox.height + conf.entityPadding * 2),
   };
 
-  // There might be some spare width for padding out attributes if the entity name is very long
-  const spareWidth = Math.max(0, bBox.width - maxWidth - widthPadding * 4);
-
   if (attributes.length > 0) {
+    // There might be some spare width for padding out attributes if the entity name is very long
+    const spareColumnWidth = Math.max(
+      0,
+      (bBox.width - maxWidth - widthPadding * widthPaddingFactor) / (widthPaddingFactor / 2)
+    );
+
     // Position the entity label near the top of the entity bounding box
     entityTextNode.attr(
       'transform',
@@ -180,9 +198,10 @@ const drawAttributes = (groupNode, entityTextNode, attributes) => {
       // Calculate the alignment y co-ordinate for the type/name of the attribute
       const alignY = heightOffset + heightPadding + attributeNode.height / 2;
 
-      // Position the type of the attribute
+      // Position the type attribute
       attributeNode.tn.attr('transform', 'translate(' + widthPadding + ',' + alignY + ')');
 
+      // TODO Handle spareWidth in attr('width')
       // Insert a rectangle for the type
       const typeRect = groupNode
         .insert('rect', '#' + attributeNode.tn.node().id)
@@ -192,65 +211,73 @@ const drawAttributes = (groupNode, entityTextNode, attributes) => {
         .attr('stroke', conf.stroke)
         .attr('x', 0)
         .attr('y', heightOffset)
-        .attr('width', maxTypeWidth * 2 + spareWidth / 2)
+        .attr('width', maxTypeWidth + widthPadding * 2 + spareColumnWidth)
-        .attr('height', attributeNode.tn.node().getBBox().height + heightPadding * 2);
+        .attr('height', attributeNode.height + heightPadding * 2);
 
-      // Position the name of the attribute
+      const nameXOffset = parseFloat(typeRect.attr('x')) + parseFloat(typeRect.attr('width'));
+
+      // Position the name attribute
       attributeNode.nn.attr(
         'transform',
-        'translate(' + (parseFloat(typeRect.attr('width')) + widthPadding) + ',' + alignY + ')'
+        'translate(' + (nameXOffset + widthPadding) + ',' + alignY + ')'
       );
 
       // Insert a rectangle for the name
-      groupNode
+      const nameRect = groupNode
         .insert('rect', '#' + attributeNode.nn.node().id)
         .attr('class', `er ${attribStyle}`)
         .attr('fill', conf.fill)
         .attr('fill-opacity', '100%')
         .attr('stroke', conf.stroke)
-        .attr('x', `${typeRect.attr('x') + typeRect.attr('width')}`)
+        .attr('x', nameXOffset)
         .attr('y', heightOffset)
-        .attr('width', maxNameWidth + widthPadding * 2 + spareWidth / 2)
+        .attr('width', maxNameWidth + widthPadding * 2 + spareColumnWidth)
-        .attr('height', attributeNode.nn.node().getBBox().height + heightPadding * 2);
+        .attr('height', attributeNode.height + heightPadding * 2);
+
+      let keyTypeAndCommentXOffset =
+        parseFloat(nameRect.attr('x')) + parseFloat(nameRect.attr('width'));
 
       if (hasKeyType) {
-        // Position the name of the attribute
+        // Position the key type attribute
         attributeNode.kn.attr(
           'transform',
-          'translate(' + (parseFloat(typeRect.attr('width')) + widthPadding) + ',' + alignY + ')'
+          'translate(' + (keyTypeAndCommentXOffset + widthPadding) + ',' + alignY + ')'
         );
 
-        // Insert a rectangle for the name
+        // Insert a rectangle for the key type
-        groupNode
+        const keyTypeRect = groupNode
           .insert('rect', '#' + attributeNode.kn.node().id)
           .attr('class', `er ${attribStyle}`)
           .attr('fill', conf.fill)
           .attr('fill-opacity', '100%')
           .attr('stroke', conf.stroke)
-          .attr('x', `${typeRect.attr('x') + typeRect.attr('width')}`)
+          .attr('x', keyTypeAndCommentXOffset)
           .attr('y', heightOffset)
-          .attr('width', maxKeyWidth + widthPadding * 2 + spareWidth / 2)
+          .attr('width', maxKeyWidth + widthPadding * 2 + spareColumnWidth)
-          .attr('height', attributeNode.kn.node().getBBox().height + heightPadding * 2);
+          .attr('height', attributeNode.height + heightPadding * 2);
+
+        keyTypeAndCommentXOffset =
+          parseFloat(keyTypeRect.attr('x')) + parseFloat(keyTypeRect.attr('width'));
       }
 
       if (hasComment) {
-        // Position the name of the attribute
+        // Position the comment attribute
         attributeNode.cn.attr(
           'transform',
-          'translate(' + (parseFloat(typeRect.attr('width')) + widthPadding) + ',' + alignY + ')'
+          'translate(' + (keyTypeAndCommentXOffset + widthPadding) + ',' + alignY + ')'
         );
 
-        // Insert a rectangle for the name
+        // Insert a rectangle for the comment
         groupNode
           .insert('rect', '#' + attributeNode.cn.node().id)
           .attr('class', `er ${attribStyle}`)
           .attr('fill', conf.fill)
           .attr('fill-opacity', '100%')
           .attr('stroke', conf.stroke)
-          .attr('x', `${typeRect.attr('x') + typeRect.attr('width')}`)
+          .attr('x', keyTypeAndCommentXOffset)
           .attr('y', heightOffset)
-          .attr('width', maxCommentWidth + widthPadding * 2 + spareWidth / 2)
+          .attr('width', maxCommentWidth + widthPadding * 2 + spareColumnWidth)
-          .attr('height', attributeNode.cn.node().getBBox().height + heightPadding * 2);
+          .attr('height', attributeNode.height + heightPadding * 2);
       }
 
       // Increment the height offset to move to the next row

src/diagrams/er/parser/erDiagram.jison

@@ -98,8 +98,8 @@ attributes
 attribute
     : attributeType attributeName { $$ = { attributeType: $1, attributeName: $2 }; }
     | attributeType attributeName attributeKeyType { $$ = { attributeType: $1, attributeName: $2, attributeKeyType: $3 }; }
-    | attributeType attributeName COMMENT { $$ = { attributeType: $1, attributeName: $2, attributeComment: $3 }; }
+    | attributeType attributeName attributeComment { $$ = { attributeType: $1, attributeName: $2, attributeComment: $3 }; }
-    | attributeType attributeName attributeKeyType COMMENT { $$ = { attributeType: $1, attributeName: $2, attributeKeyType: $3, attributeComment: $4 }; }
+    | attributeType attributeName attributeKeyType attributeComment { $$ = { attributeType: $1, attributeName: $2, attributeKeyType: $3, attributeComment: $4 }; }
     ;
 
 attributeType
@@ -114,6 +114,10 @@ attributeKeyType
     : ATTRIBUTE_KEY { $$=$1; }
     ;
 
+attributeComment
+    : COMMENT { $$=$1.replace(/"/g, ''); }
+    ;
+
 relSpec
     : cardinality relType cardinality
       {

src/diagrams/er/parser/erDiagram.spec.js

@@ -59,6 +59,7 @@ describe('when parsing ER diagram it...', function () {
     const entities = erDb.getEntities();
     expect(Object.keys(entities).length).toBe(1);
     expect(entities[entity].attributes.length).toBe(1);
+    expect(entities[entity].attributes[0].attributeComment).toBe('comment');
   });
 
   it('should allow an entity with a single attribute to be defined with a key and a comment', function () {