ronantakizawa/codereview-bench huggingface dataset:
Repository: mermaid-js/mermaid
File: packages/mermaid/src/utils/imperativeState.ts (TypeScript)

Code before:
 */
export class ImperativeState<S extends Record<string, unknown>> {
  init: () => S;
  records: S;

  /**
   * @param init - Function that creates the default state.
   */
  constructor(init: () => S) {
    this.init = init;
    this.records = init();
  }

  reset() {
    Object.keys(this.records).forEach((key) => {
      delete this.records[key];
    });
    Object.entries(this.init()).forEach(
      ([key, value]: [
        keyof S,
        any // eslint-disable-line @typescript-eslint/no-explicit-any
      ]) => {
        this.records[key] = value;
      }
    );
  }
}


Review comment (suggestion):
```suggestion
    this.records = this.init();
```

I remember your reply to the last comment, but is there any particular reason for keeping the reference equality after reset? Because reassigning the variable is much cleaner than deleting and assigning individual properties.

> I want to keep reference equality after reset, but it is not a big deal


Code after:
/**
 * Resettable state storage.
 * @example
 * ```
 * const state = new ImperativeState(() => {
 *   foo: undefined as string | undefined,
 *   bar: [] as number[],
 *   baz: 1 as number | undefined,
 * });
 *
 * state.records.foo = "hi";
 * console.log(state.records.foo); // prints "hi";
 * state.reset();
 * console.log(state.records.foo); // prints "default";
 *
 * // typeof state.records:
 * // {
 * //   foo: string | undefined, // actual: undefined
 * //   bar: number[],           // actual: []
 * //   baz: number | undefined, // actual: 1
 * // }
 * ```
 */
export class ImperativeState<S extends Record<string, unknown>> {
  public records: S;

  /**
   * @param init - Function that creates the default state.
   */
  constructor(private init: () => S) {
    this.records = this.init();
  }

  reset() {
    this.records = this.init();
  }
}
Repository: mermaid-js/mermaid
File: packages/mermaid/src/utils/imperativeState.ts (TypeScript)

Code before:
 *   foo: undefined as string | undefined,
 *   bar: [] as number[],
 *   baz: 1 as number | undefined,
 * });
 *
 * state.records.foo = "hi";
 * console.log(state.records.foo); // prints "hi";
 * state.reset();
 * console.log(state.records.foo); // prints "default";
 *
 * // typeof state.records:
 * // {
 * //   foo: string | undefined, // actual: undefined
 * //   bar: number[],           // actual: []
 * //   baz: number | undefined, // actual: 1
 * // }
 * ```
 */
export class ImperativeState<S extends Record<string, unknown>> {
  init: () => S;
  records: S;

  /**
   * @param init - Function that creates the default state.
   */
  constructor(init: () => S) {
    this.init = init;
    this.records = init();
  }

  reset() {
    Object.keys(this.records).forEach((key) => {
      delete this.records[key];
    });
    Object.entries(this.init()).forEach(
      ([key, value]: [
        keyof S,
        any // eslint-disable-line @typescript-eslint/no-explicit-any
      ]) => {
        this.records[key] = value;
      }
    );
  }
}


Review comment (suggestion):
```suggestion
  records: S;

  /**
   * @param init - Function that creates the default state.
   */
  constructor(private init: () => S) {
```

Code after:
 *   foo: undefined as string | undefined,
 *   bar: [] as number[],
 *   baz: 1 as number | undefined,
 * });
 *
 * state.records.foo = "hi";
 * console.log(state.records.foo); // prints "hi";
 * state.reset();
 * console.log(state.records.foo); // prints "default";
 *
 * // typeof state.records:
 * // {
 * //   foo: string | undefined, // actual: undefined
 * //   bar: number[],           // actual: []
 * //   baz: number | undefined, // actual: 1
 * // }
 * ```
 */
export class ImperativeState<S extends Record<string, unknown>> {
  public records: S;

  /**
   * @param init - Function that creates the default state.
   */
  constructor(private init: () => S) {
    this.records = this.init();
  }

  reset() {
    this.records = this.init();
  }
}
Repository: mermaid-js/mermaid
File: packages/mermaid/src/dagre-wrapper/clusters.js (JavaScript)

Code before:
    node.diff = (bbox.width - node.width) / 2 - node.padding / 2;
  } else {
    node.diff = -node.padding / 2;
  }

  log.trace('Data ', node, JSON.stringify(node));
  // center the rect around its coordinate
  rect
    .attr('style', node.style)
    .attr('rx', node.rx)
    .attr('ry', node.ry)
    .attr('x', node.x - width / 2)
    .attr('y', node.y - node.height / 2 - halfPadding)
    .attr('width', width)
    .attr('height', node.height + padding);

  const { subGraphTitleTopMargin } = getSubGraphTitleMargins();
  if (useHtmlLabels) {
    label.attr(
      'transform',
      // This puts the labal on top of the box instead of inside it
      'translate(' +
        (node.x - bbox.width / 2) +
        ', ' +
        (node.y - node.height / 2 + subGraphTitleTopMargin) +
        ')'
    );
  } else {
    label.attr(
      'transform',
      // This puts the labal on top of the box instead of inside it
      'translate(' + node.x + ', ' + (node.y - node.height / 2 + subGraphTitleTopMargin) + ')'
    );
  }
  // Center the label

  const rectBox = rect.node().getBBox();
  node.width = rectBox.width;
  node.height = rectBox.height;

  node.intersect = function (point) {
    return intersectRect(node, point);
  };

  return shapeSvg;
};

/**
 * Non visible cluster where the note is group with its
 *
 * @param {any} parent

Review comment (suggestion):
```suggestion
      `translate(${node.x - bbox.width / 2}, ${node.y - node.height / 2 + subGraphTitleTopMargin})`
```

Code after:
  if (node.width <= bbox.width + padding) {
    node.diff = (bbox.width - node.width) / 2 - node.padding / 2;
  } else {
    node.diff = -node.padding / 2;
  }

  log.trace('Data ', node, JSON.stringify(node));
  // center the rect around its coordinate
  rect
    .attr('style', node.style)
    .attr('rx', node.rx)
    .attr('ry', node.ry)
    .attr('x', node.x - width / 2)
    .attr('y', node.y - node.height / 2 - halfPadding)
    .attr('width', width)
    .attr('height', node.height + padding);

  const { subGraphTitleTopMargin } = getSubGraphTitleMargins(siteConfig);
  if (useHtmlLabels) {
    label.attr(
      'transform',
      // This puts the labal on top of the box instead of inside it
      `translate(${node.x - bbox.width / 2}, ${node.y - node.height / 2 + subGraphTitleTopMargin})`
    );
  } else {
    label.attr(
      'transform',
      // This puts the labal on top of the box instead of inside it
      `translate(${node.x}, ${node.y - node.height / 2 + subGraphTitleTopMargin})`
    );
  }
  // Center the label

  const rectBox = rect.node().getBBox();
  node.width = rectBox.width;
  node.height = rectBox.height;

  node.intersect = function (point) {
    return intersectRect(node, point);
  };

  return shapeSvg;
};

/**
 * Non visible cluster where the note is group with its
 *
 * @param {any} parent
 * @param {any} node
 * @returns {any} ShapeSvg
 */
Repository: mermaid-js/mermaid
File: packages/mermaid/src/diagram.spec.ts (TypeScript)

Code before:
-----------------------^
Expecting 'TXT', got 'NEWLINE'"
		`);
  });

  test('should throw the right error for unregistered diagrams', async () => {
    await expect(getDiagramFromText('thor TD; A-->B')).rejects.toThrowErrorMatchingInlineSnapshot(
      '"No diagram type detected matching given configuration for text: thor TD; A-->B"'
    );
  });

  test('should consider entity codes when present in diagram defination', async () => {
    const diagram = await getDiagramFromText(`sequenceDiagram
    A->>B: I #9829; you!
    B->>A: I #9829; you #infin; times more!`);
    // @ts-ignore: we need to add types for sequenceDb which will be done in separate PR
    const messages = diagram.db?.getMessages?.();
    if (!messages) {
      throw new Error('Messages not found!');
    }

    expect(messages[0].message).toBe('I ﬂ°°9829¶ß you!');
    expect(messages[1].message).toBe('I ﬂ°°9829¶ß you ﬂ°infin¶ß times more!');
  });

  test('should clean up comments when present in diagram defination', async () => {
    const diagram = await getDiagramFromText(
      `flowchart LR
    %% this is a comment A -- text --> B{node}
    A -- text --> B -- text2 --> C`
    );
    expect(diagram).toBeInstanceOf(Diagram);
    expect(diagram.type).toBe('flowchart-v2');
  });
});


Review comment (refactor):
```suggestion
  test('should clean up comments inside getDiagramFromText when present in diagram definition', async () => {
```

Code after:
-----------------------^
Expecting 'TXT', got 'NEWLINE'"
		`);
  });

  test('should throw the right error for unregistered diagrams', async () => {
    await expect(getDiagramFromText('thor TD; A-->B')).rejects.toThrowErrorMatchingInlineSnapshot(
      '"No diagram type detected matching given configuration for text: thor TD; A-->B"'
    );
  });

  test('should consider entity codes when present in diagram defination', async () => {
    const diagram = await getDiagramFromText(`sequenceDiagram
    A->>B: I #9829; you!
    B->>A: I #9829; you #infin; times more!`);
    // @ts-ignore: we need to add types for sequenceDb which will be done in separate PR
    const messages = diagram.db?.getMessages?.();
    if (!messages) {
      throw new Error('Messages not found!');
    }

    expect(messages[0].message).toBe('I ﬂ°°9829¶ß you!');
    expect(messages[1].message).toBe('I ﬂ°°9829¶ß you ﬂ°infin¶ß times more!');
  });
});
Repository: mermaid-js/mermaid
File: packages/mermaid/src/mermaidAPI.spec.ts (TypeScript)

Code before:
      diagramTypesAndExpectations.forEach((testedDiagram) => {
        describe(`${testedDiagram.textDiagramType}`, () => {
          const diagramType = testedDiagram.textDiagramType;
          const diagramText = `${diagramType}\n accTitle: ${a11yTitle}\n accDescr: ${a11yDescr}\n`;
          const expectedDiagramType = testedDiagram.expectedType;

          it('should set aria-roledscription to the diagram type AND should call addSVGa11yTitleDescription', async () => {
            const a11yDiagramInfo_spy = vi.spyOn(accessibility, 'setA11yDiagramInfo');
            const a11yTitleDesc_spy = vi.spyOn(accessibility, 'addSVGa11yTitleDescription');
            await mermaidAPI.render(id, diagramText);
            expect(a11yDiagramInfo_spy).toHaveBeenCalledWith(
              expect.anything(),
              expectedDiagramType
            );
            expect(a11yTitleDesc_spy).toHaveBeenCalled();
          });
        });
      });
    });
  });

  describe('getDiagramFromText', () => {
    it('should clean up comments when present in diagram definition', async () => {
      const diagram = await mermaidAPI.getDiagramFromText(
        `flowchart LR
      %% this is a comment A -- text --> B{node}
      A -- text --> B -- text2 --> C`
      );
      expect(diagram).toBeInstanceOf(Diagram);
      expect(diagram.type).toBe('flowchart-v2');
    });
  });
});


Review comment (suggestion):
```suggestion
      %% This is a comment A -- text --> B{node}
```

Code after:
      diagramTypesAndExpectations.forEach((testedDiagram) => {
        describe(`${testedDiagram.textDiagramType}`, () => {
          const diagramType = testedDiagram.textDiagramType;
          const diagramText = `${diagramType}\n accTitle: ${a11yTitle}\n accDescr: ${a11yDescr}\n`;
          const expectedDiagramType = testedDiagram.expectedType;

          it('should set aria-roledscription to the diagram type AND should call addSVGa11yTitleDescription', async () => {
            const a11yDiagramInfo_spy = vi.spyOn(accessibility, 'setA11yDiagramInfo');
            const a11yTitleDesc_spy = vi.spyOn(accessibility, 'addSVGa11yTitleDescription');
            await mermaidAPI.render(id, diagramText);
            expect(a11yDiagramInfo_spy).toHaveBeenCalledWith(
              expect.anything(),
              expectedDiagramType
            );
            expect(a11yTitleDesc_spy).toHaveBeenCalled();
          });
        });
      });
    });
  });

  describe('getDiagramFromText', () => {
    it('should clean up comments when present in diagram definition', async () => {
      const diagram = await mermaidAPI.getDiagramFromText(
        `flowchart LR
      %% This is a comment A -- text --> B{node}
      A -- text --> B -- text2 --> C`
      );
      expect(diagram).toBeInstanceOf(Diagram);
      expect(diagram.type).toBe('flowchart-v2');
    });
  });
});
Repository: mermaid-js/mermaid
File: packages/mermaid/src/docs/.vitepress/components/TopBar.vue (Vue)

Code before:
<template>
  <div
    class="w-full top-bar bg-gradient-to-r from-[#bd34fe] to-[#ff3670] flex items-center text-center justify-center p-1 text-white"
  >
    <p class="flex-grow text-center tracking-wide text-text">
      Happy Holidays! Get AI, team collaboration, storage, and more with
      <strong>Mermaid Chart Pro</strong>. Use promo code: <strong>HOLIDAYS2023 </strong>to get
      <strong>25%</strong> off.<a
        href="https://www.mermaidchart.com/holiday-promo"
        target="_blank"
        rel="noopener noreferrer"
        class="unstyled ml-2 text-white underline"
        >View more details here.</a
      >
    </p>
  </div>
</template>


Review comment (suggestion):

```suggestion
```

Otherwise we won't know where the traffic came from.

Code after:
<template>
  <div
    class="w-full top-bar bg-gradient-to-r from-[#bd34fe] to-[#ff3670] flex items-center text-center justify-center p-1 text-white"
  >
    <p class="flex-grow text-center tracking-wide text-text">
      <a
        href="https://www.mermaidchart.com/holiday-promo"
        target="_blank"
        class="unstyled text-white"
      >
        Get AI, team collaboration, storage, and more with
        <span class="font-bold underline"
          >Mermaid Chart Pro. Start free trial today & get 25% off.</span
        >
      </a>
    </p>
  </div>
</template>
Repository: mermaid-js/mermaid
File: packages/mermaid/src/diagrams/class/classDb.ts (TypeScript)

Code before:
    return;
  }
  for (const name of classNames) {
    const { className } = splitClassNameAndType(name);
    classes[className].parent = id;
    namespaces[id].classes[className] = classes[className];
  }
};

export const setCssStyle = function (id: string, style: string[]) {
  if (style !== undefined && style !== null) {
    const thisClass = classes[id];
    if (thisClass !== undefined) {
      style.forEach(function (s) {
        if (s.includes(',')) {
          const styles = s.split(',');
          styles.forEach(function (newStyle) {
            thisClass.styles.push(newStyle);
          });
        } else {
          thisClass.styles.push(s);
        }
      });
    }
  }
};

export default {
  setAccTitle,
  getAccTitle,
  getAccDescription,
  setAccDescription,
  getConfig: () => getConfig().class,
  addClass,
  bindFunctions,
  clear,
  getClass,
  getClasses,
  getNotes,
  addAnnotation,
  addNote,
  getRelations,
  addRelation,
  getDirection,
  setDirection,
  addMember,
  addMembers,
  cleanupLabel,
  lineType,
  relationType,
  setClickEvent,

Review comment (suggestion):
```suggestion
export const setCssStyle = function (id: string, styles: string[]) {
  const thisClass = classes[id];
  if (!styles || !thisClass) {
    return;
  }
  for (const s of styles) {
    if (s.includes(',')) {
      thisClass.styles = thisClass.styles.concat(s.split(','));
    } else {
      thisClass.styles.push(s);
    }
  }
};
```

Code after:
    return;
  }
  for (const name of classNames) {
    const { className } = splitClassNameAndType(name);
    classes[className].parent = id;
    namespaces[id].classes[className] = classes[className];
  }
};

export const setCssStyle = function (id: string, styles: string[]) {
  const thisClass = classes[id];
  if (!styles || !thisClass) {
    return;
  }
  for (const s of styles) {
    if (s.includes(',')) {
      thisClass.styles.push(...s.split(','));
    } else {
      thisClass.styles.push(s);
    }
  }
};

export default {
  setAccTitle,
  getAccTitle,
  getAccDescription,
  setAccDescription,
  getConfig: () => getConfig().class,
  addClass,
  bindFunctions,
  clear,
  getClass,
  getClasses,
  getNotes,
  addAnnotation,
  addNote,
  getRelations,
  addRelation,
  getDirection,
  setDirection,
  addMember,
  addMembers,
  cleanupLabel,
  lineType,
  relationType,
  setClickEvent,
  setCssClass,
  setLink,
  getTooltip,
  setTooltip,
Repository: mermaid-js/mermaid
File: packages/mermaid/src/diagrams/class/classDb.ts (TypeScript)

Code before:
 *
 * @param id - Id of the namespace to add
 * @param classNames - Ids of the class to add
 * @public
 */
export const addClassesToNamespace = function (id: string, classNames: string[]) {
  if (namespaces[id] === undefined) {
    return;
  }
  for (const name of classNames) {
    const { className } = splitClassNameAndType(name);
    classes[className].parent = id;
    namespaces[id].classes[className] = classes[className];
  }
};

export const setCssStyle = function (id: string, styles: string[]) {
  const thisClass = classes[id];
  if (!styles || !thisClass) {
    return;
  }
  for (const s of styles) {
    if (s.includes(',')) {
      thisClass.styles = [...thisClass.styles, ...s.split(',')];
    } else {
      thisClass.styles?.push(s);
    }
  }
};

export default {
  setAccTitle,
  getAccTitle,
  getAccDescription,
  setAccDescription,
  getConfig: () => getConfig().class,
  addClass,
  bindFunctions,
  clear,
  getClass,
  getClasses,
  getNotes,
  addAnnotation,
  addNote,
  getRelations,
  addRelation,
  getDirection,
  setDirection,
  addMember,
  addMembers,
  cleanupLabel,

Review comment (suggestion):
```suggestion
      thisClass.styles.push(s);
```

If `styles` is no longer optional, I think we can remove the `?` field.

Code after:
 *
 * @param id - Id of the namespace to add
 * @param classNames - Ids of the class to add
 * @public
 */
export const addClassesToNamespace = function (id: string, classNames: string[]) {
  if (namespaces[id] === undefined) {
    return;
  }
  for (const name of classNames) {
    const { className } = splitClassNameAndType(name);
    classes[className].parent = id;
    namespaces[id].classes[className] = classes[className];
  }
};

export const setCssStyle = function (id: string, styles: string[]) {
  const thisClass = classes[id];
  if (!styles || !thisClass) {
    return;
  }
  for (const s of styles) {
    if (s.includes(',')) {
      thisClass.styles.push(...s.split(','));
    } else {
      thisClass.styles.push(s);
    }
  }
};

export default {
  setAccTitle,
  getAccTitle,
  getAccDescription,
  setAccDescription,
  getConfig: () => getConfig().class,
  addClass,
  bindFunctions,
  clear,
  getClass,
  getClasses,
  getNotes,
  addAnnotation,
  addNote,
  getRelations,
  addRelation,
  getDirection,
  setDirection,
  addMember,
  addMembers,
  cleanupLabel,
Repository: mermaid-js/mermaid
File: packages/mermaid/src/diagrams/class/classDb.ts (TypeScript)

Code before:
/**
 * Function called by parser when a namespace definition has been found.
 *
 * @param id - Id of the namespace to add
 * @param classNames - Ids of the class to add
 * @public
 */
export const addClassesToNamespace = function (id: string, classNames: string[]) {
  if (namespaces[id] === undefined) {
    return;
  }
  for (const name of classNames) {
    const { className } = splitClassNameAndType(name);
    classes[className].parent = id;
    namespaces[id].classes[className] = classes[className];
  }
};

export const setCssStyle = function (id: string, styles: string[]) {
  const thisClass = classes[id];
  if (!styles || !thisClass) {
    return;
  }
  for (const s of styles) {
    if (s.includes(',')) {
      thisClass.styles = [...thisClass.styles, ...s.split(',')];
    } else {
      thisClass.styles?.push(s);
    }
  }
};

export default {
  setAccTitle,
  getAccTitle,
  getAccDescription,
  setAccDescription,
  getConfig: () => getConfig().class,
  addClass,
  bindFunctions,
  clear,
  getClass,
  getClasses,
  getNotes,
  addAnnotation,
  addNote,
  getRelations,
  addRelation,
  getDirection,
  setDirection,
  addMember,

Review comment (suggestion):
For consistency with https://github.com/mermaid-js/mermaid/blob/b92b344844256287f038c7455210ca8546f9f308/packages/mermaid/src/diagrams/class/classDb.ts#L469, we could instead use `Array.push(...items)`, which will modify the existing array in-place, instead of creating a new array.

```suggestion
      thisClass.styles.push(...s.split(','));
```

Code after:
/**
 * Function called by parser when a namespace definition has been found.
 *
 * @param id - Id of the namespace to add
 * @param classNames - Ids of the class to add
 * @public
 */
export const addClassesToNamespace = function (id: string, classNames: string[]) {
  if (namespaces[id] === undefined) {
    return;
  }
  for (const name of classNames) {
    const { className } = splitClassNameAndType(name);
    classes[className].parent = id;
    namespaces[id].classes[className] = classes[className];
  }
};

export const setCssStyle = function (id: string, styles: string[]) {
  const thisClass = classes[id];
  if (!styles || !thisClass) {
    return;
  }
  for (const s of styles) {
    if (s.includes(',')) {
      thisClass.styles.push(...s.split(','));
    } else {
      thisClass.styles.push(s);
    }
  }
};

export default {
  setAccTitle,
  getAccTitle,
  getAccDescription,
  setAccDescription,
  getConfig: () => getConfig().class,
  addClass,
  bindFunctions,
  clear,
  getClass,
  getClasses,
  getNotes,
  addAnnotation,
  addNote,
  getRelations,
  addRelation,
  getDirection,
  setDirection,
  addMember,