ronantakizawa/codereview-bench huggingface dataset:
Repository: mermaid-js/mermaid
File: packages/mermaid/src/diagrams/sequence/sequenceRenderer.ts (TypeScript)

Code before:
            ',' +
            lineStartY +
            ' C ' +
            (startx + 60) +
            ',' +
            (lineStartY - 10) +
            ' ' +
            (startx + 60) +
            ',' +
            (lineStartY + 30) +
            ' ' +
            startx +
            ',' +
            (lineStartY + 20)
        );
    }
  } else {
    line = diagram.append('line');
    let adjustedStartx = startx;
    if (
      type === diagObj.db.LINETYPE.BIDIRECTIONAL_DOTTED ||
      type === diagObj.db.LINETYPE.BIDIRECTIONAL_SOLID
    ) {
      startx < stopx ? (adjustedStartx += 3) : (adjustedStartx -= 3);
    }
    line.attr('x1', adjustedStartx);
    line.attr('y1', lineStartY);
    line.attr('x2', stopx);
    line.attr('y2', lineStartY);
  }
  // Make an SVG Container
  // Draw the line
  if (
    type === diagObj.db.LINETYPE.DOTTED ||
    type === diagObj.db.LINETYPE.DOTTED_CROSS ||
    type === diagObj.db.LINETYPE.DOTTED_POINT ||
    type === diagObj.db.LINETYPE.DOTTED_OPEN ||
    type === diagObj.db.LINETYPE.BIDIRECTIONAL_DOTTED
  ) {
    line.style('stroke-dasharray', '3, 3');
    line.attr('class', 'messageLine1');
  } else {
    line.attr('class', 'messageLine0');
  }

  let url = '';
  if (conf.arrowMarkerAbsolute) {
    url =
      window.location.protocol +
      '//' +
      window.location.host +

Review comment (suggestion):
The startx and stopx values are calculated in the `buildMessageModel` function. 
Adding this adjustment here breaks the Separation of concern, as it makes the `drawMessage` function responsible for the adjustment, where it should only have drawn as per the dimensions calculated in `buildMessageModel`.

We should always try to solve the root cause, instead of patching the outcome. 

Code after:
            ',' +
            lineStartY +
            ' C ' +
            (startx + 60) +
            ',' +
            (lineStartY - 10) +
            ' ' +
            (startx + 60) +
            ',' +
            (lineStartY + 30) +
            ' ' +
            startx +
            ',' +
            (lineStartY + 20)
        );
    }
  } else {
    line = diagram.append('line');
    line.attr('x1', startx);
    line.attr('y1', lineStartY);
    line.attr('x2', stopx);
    line.attr('y2', lineStartY);
  }
  // Make an SVG Container
  // Draw the line
  if (
    type === diagObj.db.LINETYPE.DOTTED ||
    type === diagObj.db.LINETYPE.DOTTED_CROSS ||
    type === diagObj.db.LINETYPE.DOTTED_POINT ||
    type === diagObj.db.LINETYPE.DOTTED_OPEN ||
    type === diagObj.db.LINETYPE.BIDIRECTIONAL_DOTTED
  ) {
    line.style('stroke-dasharray', '3, 3');
    line.attr('class', 'messageLine1');
  } else {
    line.attr('class', 'messageLine0');
  }

  let url = '';
  if (conf.arrowMarkerAbsolute) {
    url =
      window.location.protocol +
      '//' +
      window.location.host +
      window.location.pathname +
      window.location.search;
    url = url.replace(/\(/g, '\\(');
    url = url.replace(/\)/g, '\\)');
  }

  line.attr('stroke-width', 2);
Repository: mermaid-js/mermaid
File: packages/mermaid/src/diagrams/gantt/ganttDb.js (JavaScript)

Code before:
  str = str.trim();

  // Test for until
  const re = /^until\s+([\d\w- ]+)/;
  const untilStatement = re.exec(str.trim());

  if (untilStatement !== null) {
    // check all until ids and take the earliest
    let earliestStartingTask = null;
    untilStatement[1].split(' ').forEach(function (id) {
      let task = findTaskById(id);
      if (task !== undefined) {
        if (!earliestStartingTask) {
          earliestStartingTask = task;
        } else {
          if (task.startTime < earliestStartingTask.startTime) {
            earliestStartingTask = task;
          }
        }
      }
    });

    if (!earliestStartingTask) {
      const dt = new Date();
      dt.setHours(0, 0, 0, 0);
      return dt;
    } else {
      return earliestStartingTask.startTime;
    }
  }

  // Check for actual date
  let mDate = dayjs(str, dateFormat.trim(), true);
  if (mDate.isValid()) {
    if (inclusive) {
      mDate = mDate.add(1, 'd');
    }
    return mDate.toDate();
  }

  let endTime = dayjs(prevTime);
  const [durationValue, durationUnit] = parseDuration(str);
  if (!Number.isNaN(durationValue)) {
    const newEndTime = endTime.add(durationValue, durationUnit);
    if (newEndTime.isValid()) {
      endTime = newEndTime;
    }
  }
  return endTime.toDate();
};


Review comment (suggestion):
Avoid abbreviations, something with meaning like `midnightDate`, `today` is preferable.

Code after:
  if (untilStatement !== null) {
    // check all until ids and take the earliest
    let earliestTask = null;
    for (const id of untilStatement.groups.ids.split(' ')) {
      let task = findTaskById(id);
      if (task !== undefined && (!earliestTask || task.startTime < earliestTask.startTime)) {
        earliestTask = task;
      }
    }

    if (earliestTask) {
      return earliestTask.startTime;
    }
    const today = new Date();
    today.setHours(0, 0, 0, 0);
    return today;
  }

  // check for actual date
  let parsedDate = dayjs(str, dateFormat.trim(), true);
  if (parsedDate.isValid()) {
    if (inclusive) {
      parsedDate = parsedDate.add(1, 'd');
    }
    return parsedDate.toDate();
  }

  let endTime = dayjs(prevTime);
  const [durationValue, durationUnit] = parseDuration(str);
  if (!Number.isNaN(durationValue)) {
    const newEndTime = endTime.add(durationValue, durationUnit);
    if (newEndTime.isValid()) {
      endTime = newEndTime;
    }
  }
  return endTime.toDate();
};

let taskCnt = 0;
const parseId = function (idStr) {
  if (idStr === undefined) {
    taskCnt = taskCnt + 1;
    return 'task' + taskCnt;
  }
  return idStr;
};
// id, startDate, endDate
// id, startDate, length
// id, after x, endDate
// id, after x, length
// startDate, endDate
Repository: mermaid-js/mermaid
File: packages/mermaid/src/diagrams/gantt/ganttDb.js (JavaScript)

Code before:
};

const getEndDate = function (prevTime, dateFormat, str, inclusive = false) {
  str = str.trim();

  // test for until
  const untilRePattern = /^until\s+(?<ids>[\d\w- ]+)/;
  const untilStatement = untilRePattern.exec(str);

  if (untilStatement !== null) {
    // check all until ids and take the earliest
    let earliestTask = null;
    for (const id of untilStatement.groups.ids.split(' ')) {
      let task = findTaskById(id);
      if (task !== undefined && (!earliestTask || task.startTime < earliestTask.startTime)) {
        earliestTask = task;
      }
    }

    if (earliestTask) {
      return earliestTask.startTime;
    } else {
      const today = new Date();
      today.setHours(0, 0, 0, 0);
      return today;
    }
  }

  // check for actual date
  let parsedDate = dayjs(str, dateFormat.trim(), true);
  if (parsedDate.isValid()) {
    if (inclusive) {
      parsedDate = parsedDate.add(1, 'd');
    }
    return parsedDate.toDate();
  }

  let endTime = dayjs(prevTime);
  const [durationValue, durationUnit] = parseDuration(str);
  if (!Number.isNaN(durationValue)) {
    const newEndTime = endTime.add(durationValue, durationUnit);
    if (newEndTime.isValid()) {
      endTime = newEndTime;
    }
  }
  return endTime.toDate();
};

let taskCnt = 0;
const parseId = function (idStr) {
  if (idStr === undefined) {

Review comment (suggestion):
Minor styling suggestion. Here and above for `latestTask`.
```suggestion
    if (earliestTask) {
      return earliestTask.startTime;
    }
    const today = new Date();
    today.setHours(0, 0, 0, 0);
    return today;
```

Code after:
};

const getEndDate = function (prevTime, dateFormat, str, inclusive = false) {
  str = str.trim();

  // test for until
  const untilRePattern = /^until\s+(?<ids>[\d\w- ]+)/;
  const untilStatement = untilRePattern.exec(str);

  if (untilStatement !== null) {
    // check all until ids and take the earliest
    let earliestTask = null;
    for (const id of untilStatement.groups.ids.split(' ')) {
      let task = findTaskById(id);
      if (task !== undefined && (!earliestTask || task.startTime < earliestTask.startTime)) {
        earliestTask = task;
      }
    }

    if (earliestTask) {
      return earliestTask.startTime;
    }
    const today = new Date();
    today.setHours(0, 0, 0, 0);
    return today;
  }

  // check for actual date
  let parsedDate = dayjs(str, dateFormat.trim(), true);
  if (parsedDate.isValid()) {
    if (inclusive) {
      parsedDate = parsedDate.add(1, 'd');
    }
    return parsedDate.toDate();
  }

  let endTime = dayjs(prevTime);
  const [durationValue, durationUnit] = parseDuration(str);
  if (!Number.isNaN(durationValue)) {
    const newEndTime = endTime.add(durationValue, durationUnit);
    if (newEndTime.isValid()) {
      endTime = newEndTime;
    }
  }
  return endTime.toDate();
};

let taskCnt = 0;
const parseId = function (idStr) {
  if (idStr === undefined) {
    taskCnt = taskCnt + 1;
Repository: mermaid-js/mermaid
File: packages/mermaid/src/docs/.vitepress/theme/Mermaid.vue (Vue)

Code before:
<template>
  <h5>Code:</h5>
  <div class="language-mermaid">
    <button class="copy"></button>
    <span class="lang">mermaid</span>
    <pre><code contenteditable="true" @input="updateCode"  @keydown.meta.enter="renderChart" ref="editableContent" class="editable-code"></code></pre>
    <div class="buttons-container">
      <span>{{ ctrlSymbol }} + Enter</span><span>|</span>
      <button @click="renderChart">Run ▶</button>
    </div>
  </div>
  <div v-html="svg"></div>
</template>

<script setup>
import { onMounted, onUnmounted, ref } from 'vue';
import { render } from './mermaid';

const props = defineProps({
  graph: {
    type: String,
    required: true,
  },
  id: {
    type: String,
    required: true,
  },
});

const svg = ref('');
const code = ref(decodeURIComponent(props.graph));
const ctrlSymbol = ref(navigator.platform.includes('Mac') ? '⌘' : 'Ctrl');
const editableContent = ref(null);

Review comment (suggestion):
**question(blocking)**: What does <kbd>CTRL</kbd> + <kbd>Enter</kbd> do? I've tried it, it doesn't seem to do anything.

---

**suggestion(non-blocking)**: Use [`<kbd>`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/kbd) to describe keyboard keys:

```suggestion
      <span><kbd>{{ ctrlSymbol }}</kbd> + <kbd>Enter</kbd></span><span>|</span>
```

Visually, the only difference is that `<kbd>` elements are monospaced:

### Before

![image](https://github.com/mermaid-js/mermaid/assets/19716675/9fd33e77-38ad-4c7e-87c4-a133cbfcc976)


### After

![image](https://github.com/mermaid-js/mermaid/assets/19716675/a82791c1-d6fe-408d-9591-070bae5c0edb)



Code after:
<template>
  <div v-if="props.showCode">
    <h5>Code:</h5>
    <div class="language-mermaid">
      <button class="copy"></button>
      <span class="lang">mermaid</span>
      <pre><code contenteditable="plaintext-only" @input="updateCode"  @keydown.meta.enter="renderChart" @keydown.ctrl.enter="renderChart" ref="editableContent" class="editable-code"></code></pre>
      <div class="buttons-container">
        <span>{{ ctrlSymbol }} + Enter</span><span>|</span>
        <button @click="renderChart">Run ▶</button>
      </div>
    </div>
  </div>
  <div v-html="svg"></div>
</template>

<script setup>
import { onMounted, onUnmounted, ref } from 'vue';
import { render } from './mermaid';

const props = defineProps({
  graph: {
    type: String,
    required: true,
  },
  id: {
    type: String,
    required: true,
  },
  showCode: {
    type: Boolean,
    default: true,
  },
Repository: mermaid-js/mermaid
File: packages/mermaid/src/diagrams/sequence/sequenceDb.ts (TypeScript)

Code before:
    properties: {},
    actorCnt: null,
    rectData: null,
    type: type || 'participant',
  };
  if (state.records.prevActor && state.records.actors[state.records.prevActor]) {
    state.records.actors[state.records.prevActor].nextActor = id;
  }

  if (state.records.currentBox) {
    state.records.currentBox.actorKeys.push(id);
  }
  state.records.prevActor = id;
};

const activationCount = (part: string) => {
  let i;
  let count = 0;
  if (!part) {
    return 0;
  }

  for (i = 0; i < state.records.messages.length; i++) {
    if (
      state.records.messages[i].type === LINETYPE.ACTIVE_START &&
      state.records.messages[i].from!.actor === part
    ) {
      count++;
    }
    if (
      state.records.messages[i].type === LINETYPE.ACTIVE_END &&
      state.records.messages[i].from!.actor === part
    ) {
      count--;
    }
  }
  return count;
};

export const addMessage = function (
  idFrom: Message['from'],
  idTo: Message['to'],
  message: { text: string; wrap?: boolean },
  answer: Message['answer']
) {
  state.records.messages.push({
    from: idFrom,
    to: idTo,
    message: message.text,
    wrap: (message.wrap === undefined && autoWrap()) || !!message.wrap,
    answer: answer,

Review comment (suggestion):
```suggestion
    const cnt = activationCount(idFrom?.actor ?? '');
```

Code after:
};

export const addMessage = function (
  idFrom: Message['from'],
  idTo: Message['to'],
  message: { text: string; wrap?: boolean },
  answer: Message['answer']
) {
  state.records.messages.push({
    from: idFrom,
    to: idTo,
    message: message.text,
    wrap: (message.wrap === undefined && autoWrap()) || !!message.wrap,
    answer: answer,
  });
};

export const addSignal = function (
  idFrom?: Message['from'],
  idTo?: Message['to'],
  message?: { text: string; wrap: boolean },
  messageType?: number,
  activate: boolean = false
) {
  if (messageType === LINETYPE.ACTIVE_END) {
    const cnt = activationCount(idFrom?.actor || '');
    if (cnt < 1) {
      // Bail out as there is an activation signal from an inactive participant
      const error = new Error(
        'Trying to inactivate an inactive participant (' + idFrom?.actor + ')'
      );

      // @ts-ignore: we are passing hash param to the error object, however we should define our own custom error class to make it type safe
      error.hash = {
        text: '->>-',
        token: '->>-',
        line: '1',
        loc: { first_line: 1, last_line: 1, first_column: 1, last_column: 1 },
        expected: ["'ACTIVE_PARTICIPANT'"],
      };
      throw error;
    }
  }
  state.records.messages.push({
    from: idFrom,
    to: idTo,
    message: message?.text ?? '',
    wrap: (message?.wrap === undefined && autoWrap()) || !!message?.wrap,
    type: messageType,
    activate,
  });
Repository: mermaid-js/mermaid
File: packages/mermaid/src/diagrams/sequence/sequenceDb.ts (TypeScript)

Code before:
    ) {
      count--;
    }
  }
  return count;
};

export const addMessage = function (
  idFrom: Message['from'],
  idTo: Message['to'],
  message: { text: string; wrap?: boolean },
  answer: Message['answer']
) {
  state.records.messages.push({
    from: idFrom,
    to: idTo,
    message: message.text,
    wrap: (message.wrap === undefined && autoWrap()) || !!message.wrap,
    answer: answer,
  });
};

export const addSignal = function (
  idFrom?: Message['from'],
  idTo?: Message['to'],
  message: { text?: string; wrap?: boolean } = { text: undefined, wrap: undefined },
  messageType?: number,
  activate: boolean = false
) {
  if (messageType === LINETYPE.ACTIVE_END) {
    const cnt = activationCount(idFrom?.actor || '');
    if (cnt < 1) {
      // Bail out as there is an activation signal from an inactive participant
      const error = new Error(
        'Trying to inactivate an inactive participant (' + idFrom?.actor + ')'
      );

      // @ts-ignore: we are passing hash param to the error object, however we should define our own custom error class to make it type safe
      error.hash = {
        text: '->>-',
        token: '->>-',
        line: '1',
        loc: { first_line: 1, last_line: 1, first_column: 1, last_column: 1 },
        expected: ["'ACTIVE_PARTICIPANT'"],
      };
      throw error;
    }
  }
  state.records.messages.push({
    from: idFrom,
    to: idTo,

Review comment (suggestion):
```suggestion
  message: { text?: string; wrap?: boolean } = { },
```

Code after:
      // @ts-ignore: we are passing hash param to the error object, however we should define our own custom error class to make it type safe
      error.hash = {
        text: '->>-',
        token: '->>-',
        line: '1',
        loc: { first_line: 1, last_line: 1, first_column: 1, last_column: 1 },
        expected: ["'ACTIVE_PARTICIPANT'"],
      };
      throw error;
    }
  }
  state.records.messages.push({
    from: idFrom,
    to: idTo,
    message: message?.text ?? '',
    wrap: (message?.wrap === undefined && autoWrap()) || !!message?.wrap,
    type: messageType,
    activate,
  });
  return true;
};

export const hasAtLeastOneBox = function () {
  return state.records.boxes.length > 0;
};

export const hasAtLeastOneBoxWithTitle = function () {
  return state.records.boxes.some((b) => b.name);
};

export const getMessages = function () {
  return state.records.messages;
};

export const getBoxes = function () {
  return state.records.boxes;
};
export const getActors = function () {
  return state.records.actors;
};
export const getCreatedActors = function () {
  return state.records.createdActors;
};
export const getDestroyedActors = function () {
  return state.records.destroyedActors;
};
export const getActor = function (id: string) {
  return state.records.actors[id];
};
export const getActorKeys = function () {
  return Object.keys(state.records.actors);
Repository: mermaid-js/mermaid
File: packages/mermaid/src/diagrams/gantt/styles.js (JavaScript)

Code before:
const getStyles = (options) =>
  `
  .mermaid-main-font {
        font-family: var(--mermaid-font-family), trebuchet ms, verdana, arial, sans-serif;
  }

  .exclude-range {
    fill: ${options.excludeBkgColor};
  }

  .section {
    stroke: none;
    opacity: 0.2;
  }

  .section0 {
    fill: ${options.sectionBkgColor};
  }

  .section2 {
    fill: ${options.sectionBkgColor2};
  }

  .section1,
  .section3 {
    fill: ${options.altSectionBkgColor};
    opacity: 0.2;
  }


Review comment (suggestion):
**nitpick**: The spec recommends to keep font names quoted when we are using whitespace, see https://www.w3.org/TR/2018/REC-css-fonts-3-20180920/#propdef-font-family:

> To avoid mistakes in escaping, it is recommended to quote font family names that contain white space, digits, or punctuation characters other than hyphens:

(Although I think every modern browser handles it fine).

Code after:
const getStyles = (options) =>
  `
  .mermaid-main-font {
        font-family: ${options.fontFamily};
  }

  .exclude-range {
    fill: ${options.excludeBkgColor};
  }

  .section {
    stroke: none;
    opacity: 0.2;
  }

  .section0 {
    fill: ${options.sectionBkgColor};
  }

  .section2 {
    fill: ${options.sectionBkgColor2};
  }

  .section1,
  .section3 {
    fill: ${options.altSectionBkgColor};
    opacity: 0.2;
  }
Repository: mermaid-js/mermaid
File: packages/mermaid/src/diagrams/gantt/styles.js (JavaScript)

Code before:
  }

  .sectionTitle2 {
    fill: ${options.titleColor};
  }

  .sectionTitle3 {
    fill: ${options.titleColor};
  }

  .sectionTitle {
    text-anchor: start;
    font-family: var(--mermaid-font-family), trebuchet ms, verdana, arial, sans-serif;
  }


  /* Grid and axis */

  .grid .tick {
    stroke: ${options.gridColor};
    opacity: 0.8;
    shape-rendering: crispEdges;
  }

  .grid .tick text {
    font-family: var(--mermaid-font-family), trebuchet ms, verdana, arial, sans-serif;
    fill: ${options.textColor};
  }

  .grid path {
    stroke-width: 0;
  }


  /* Today line */

  .today {
    fill: none;
    stroke: ${options.todayLineColor};
    stroke-width: 2px;
  }


  /* Task styling */

  /* Default task */

  .task {
    stroke-width: 2;
  }


Review comment (suggestion):
**issue(potentially-blocking)**:

This seems to be a breaking change to me (although it's only a minor visual change, so maybe it's not actually breaking?):

Try rendering the following diagram in mermaid.live or on GitHub, and you can see the font is different for the axis labels:

```mermaid
---
config:
    themeVariables:
        fontFamily: 'Impact'
        textColor: 'red'
---
gantt
    title A Gantt Diagram
    dateFormat  YYYY-MM-DD
    section Section
    A task           :a1, 2014-01-01, 1d
```

Your PR seems to allow changing the other vars, but not this one!

It looks like 50% of the CSS in Mermaid is using `var(--mermaid-font-family)` (aka `MermaidConfig["fontFamily"]`, but the other half seems to be using `options.fontFamily` (aka `MermaidConfig["themeVariables"]["fontFamily"]` and https://mermaid.js.org/config/theming.html#theme-variables

Code after:
  }

  .sectionTitle2 {
    fill: ${options.titleColor};
  }

  .sectionTitle3 {
    fill: ${options.titleColor};
  }

  .sectionTitle {
    text-anchor: start;
    font-family: ${options.fontFamily};
  }


  /* Grid and axis */

  .grid .tick {
    stroke: ${options.gridColor};
    opacity: 0.8;
    shape-rendering: crispEdges;
  }

  .grid .tick text {
    font-family: ${options.fontFamily};
    fill: ${options.textColor};
  }

  .grid path {
    stroke-width: 0;
  }


  /* Today line */

  .today {
    fill: none;
    stroke: ${options.todayLineColor};
    stroke-width: 2px;
  }


  /* Task styling */

  /* Default task */

  .task {
    stroke-width: 2;
  }
Repository: mermaid-js/mermaid
File: cypress/integration/rendering/flowchart-icon.spec.js (JavaScript)

Code before:
  describe('Flowchart Icon', () => {
    it(`${index + 1}-icon: verify if icons are working from fontawesome library ${theme} theme`, () => {
      imgSnapshotTest(
        `flowchart TD
            A("fab:fa-twitter Twitter") --> B("fab:fa-facebook Facebook")
            B --> C("fa:fa-coffee Coffee")
            C --> D("fa:fa-car Car")
            D --> E("fab:fa-github GitHub")
        `,
        { theme }
      );
    });
  });
});

themes.forEach((theme, index) => {
  describe('Flowchart Icon', () => {
    it(`${index + 1}-icon: verify if registered icons are working on ${theme} theme`, () => {
      imgSnapshotTest(
        `flowchart TD
            A("fa:fa-bell Bell") 
        `,
        { theme }
      );
    });
  });
});


Review comment (suggestion):
There seems to a red border around all these icons.

Is this normal (I'm like 80% sure it's just an issue with the E2E tests).

![image](https://github.com/user-attachments/assets/cff34e37-53da-44fd-9b09-2c3fbdb7c687)


Code after:
import { imgSnapshotTest } from '../../helpers/util.ts';

const themes = ['default', 'forest', 'dark', 'base', 'neutral'];

describe('when rendering flowchart with icons', () => {
  for (const theme of themes) {
    it(`should render icons from fontawesome library on theme ${theme}`, () => {
      imgSnapshotTest(
        `flowchart TD
            A("fab:fa-twitter Twitter") --> B("fab:fa-facebook Facebook")
            B --> C("fa:fa-coffee Coffee")
            C --> D("fa:fa-car Car")
            D --> E("fab:fa-github GitHub")
        `,
        { theme }
      );
    });

    it(`should render registered icons on theme ${theme}`, () => {
      imgSnapshotTest(
        `flowchart TD
            A("fa:fa-bell Bell") 
        `,
        { theme }
      );
    });
  }
});