ronantakizawa/codereview-bench huggingface dataset:
Repository: marktext/marktext
File: src/editor/contentState/selectionCtrl.js (JavaScript)

Code before:
const selectionCtrl = ContentState => {
  ContentState.prototype.tableCellHandler = function (event) {
    const { start } = this.cursor
    const block = this.getBlock(start.key)
    if (/th|td/.test(block.type)) {
      event.preventDefault()
      event.stopPropagation()
      this.cursor = {
        start: { key: start.key, offset: 0 },
        end: { key: start.key, offset: block.text.length }
      }
      this.partialRender()
    }
  }
}

export default selectionCtrl


Review comment (suggestion):
Please use `deconstruction` syntax.

```javascript
const { type, text, key } = block
```

this will make codes shorter and more readable.


Code after:
const selectionCtrl = ContentState => {
  // Returns the table from the table cell:
  //   table <-- thead or tbody <-- tr <-- th or td (cell)
  ContentState.prototype.getTableFromTableCell = function (block) {
    const table = this.getParent(this.getParent(this.getParent(block)))
    if (table && table.type !== 'table') {
      return null
    }
    return table
  }

  ContentState.prototype.tableCellHandler = function (event) {
    const { start, end } = this.cursor
    const startBlock = this.getBlock(start.key)
    const endBlock = this.getBlock(end.key)
    const { type: startType } = startBlock
    const { type: endType } = endBlock
    if (/th|td/.test(startType) && /th|td/.test(endType)) {
      if (start.key === end.key) {
        const { text, key } = startBlock
        this.cursor = {
          start: { key, offset: 0 },
          end: { key, offset: text.length }
        }
      } else {
        const startTable = this.getTableFromTableCell(startBlock)
        const endTable = this.getTableFromTableCell(endBlock)
        // Check whether both blocks are in the same table.
        if (!startTable || !endTable) {
Repository: marktext/marktext
File: src/editor/contentState/selectionCtrl.js (JavaScript)

Code before:
    if (/th|td/.test(startType) && /th|td/.test(endType)) {
      if (start.key === end.key) {
        const { text, key } = startBlock
        this.cursor = {
          start: { key, offset: 0 },
          end: { key, offset: text.length }
        }
      } else {
        const startTable = this.getTableFromTableCell(startBlock)
        const endTable = this.getTableFromTableCell(endBlock)
        // Check whether both blocks are in the same table.
        if (!startTable || !endTable) {
          console.error('No table found or invalid type.')
          return
        } else if (startTable.key !== endTable.key) {
          // Select entire document
          return
        }
        const firstTableBlock = this.firstInDescendant(startTable)
        const lastTableBlock = this.lastInDescendant(startTable)
        if (!firstTableBlock || !/th|td/.test(firstTableBlock.type) ||
          !lastTableBlock || !/th|td/.test(lastTableBlock.type)) {
          console.error('No table cell found or invalid type.')
          return
        }
        const { key: startKey } = firstTableBlock
        const { key: endKey, text } = lastTableBlock
        this.cursor = {
          start: { key: startKey, offset: 0 },
          end: { key: endKey, offset: text.length }
        }
      }

      event.preventDefault()
      event.stopPropagation()
      this.partialRender()
    }
  }
}

export default selectionCtrl


Review comment (bug):
Set the `cursor.start` in the first cell of the table, will solve the `text disappear` bug?

Code after:
    if (/th|td/.test(startType) && /th|td/.test(endType)) {
      if (start.key === end.key) {
        const { text, key } = startBlock
        this.cursor = {
          start: { key, offset: 0 },
          end: { key, offset: text.length }
        }
      } else {
        const startTable = this.getTableFromTableCell(startBlock)
        const endTable = this.getTableFromTableCell(endBlock)
        // Check whether both blocks are in the same table.
        if (!startTable || !endTable) {
          console.error('No table found or invalid type.')
          return
        } else if (startTable.key !== endTable.key) {
          // Select entire document
          return
        }
        const firstTableCell = this.firstInDescendant(startTable)
        const lastTableCell = this.lastInDescendant(startTable)
        if (!firstTableCell || !/th|td/.test(firstTableCell.type) ||
          !lastTableCell || !/th|td/.test(lastTableCell.type)) {
          console.error('No table cell found or invalid type.')
          return
        }
        const { key: startKey } = firstTableCell
        const { key: endKey, text } = lastTableCell
        this.cursor = {
          start: { key: startKey, offset: 0 },
          end: { key: endKey, offset: text.length }
        }
      }

      event.preventDefault()
      event.stopPropagation()
      this.partialRender()
    }
  }
}

export default selectionCtrl
Repository: marktext/marktext
File: src/editor/index.js (JavaScript)

Code before:
    this.preferLooseListItem = preferLooseListItem
    this.contentState.preferLooseListItem = preferLooseListItem
  }

  setTabSize (tabSize) {
    this.tabSize = tabSize
    this.contentState.tabSize = tabSize
  }

  updateParagraph (type) {
    this.contentState.updateParagraph(type)
  }

  blur () {
    this.container.blur()
  }

  showAutoImagePath (files) {
    const list = files.map(f => {
      const iconClass = f.type === 'directory' ? 'icon-folder' : 'icon-image'
      return Object.assign(f, { iconClass, text: f.file + (f.type === 'directory' ? '/' : '') })
    })
    this.contentState.showAutoImagePath(list)
  }

  format (type) {
    this.contentState.format(type)
  }

  insertImage (url) {
    this.contentState.insertImage(url)
  }

  search (value, opt) {
    const { selectHighlight } = opt
    this.contentState.search(value, opt)
    this.contentState.render(!!selectHighlight)
    return this.contentState.searchMatches
  }

  replace (value, opt) {
    this.contentState.replace(value, opt)
    this.contentState.render(false)
    return this.contentState.searchMatches
  }

  find (action/* pre or next */) {
    this.contentState.find(action)
    this.contentState.render(false)
    return this.contentState.searchMatches
  }

Review comment (suggestion):
We verify `tabSize` here so we don't have to verify everything when we use it. If this `tabSize` is invalid, give a default value.

Code after:
    }
    this.focusMode = bool
  }

  setTheme (name) {
    if (!name) return
    if (name === 'dark') {
      codeMirrorConfig.theme = 'railscasts'
    } else {
      delete codeMirrorConfig.theme
    }
    this.theme = name
    this.contentState.render()
  }

  setFont ({ fontSize, lineHeight }) {
    if (fontSize) this.fontSize = parseInt(fontSize, 10)
    if (lineHeight) this.lineHeight = lineHeight
  }

  setListItemPreference (preferLooseListItem) {
    this.preferLooseListItem = preferLooseListItem
    this.contentState.preferLooseListItem = preferLooseListItem
  }

  setTabSize (tabSize) {
    if (!tabSize || typeof tabSize !== 'number') {
      tabSize = 4
    } else if (tabSize < 1) {
      tabSize = 1
    }
    this.tabSize = tabSize
    this.contentState.tabSize = tabSize
  }

  updateParagraph (type) {
    this.contentState.updateParagraph(type)
  }

  blur () {
    this.container.blur()
  }

  showAutoImagePath (files) {
    const list = files.map(f => {
      const iconClass = f.type === 'directory' ? 'icon-folder' : 'icon-image'
      return Object.assign(f, { iconClass, text: f.file + (f.type === 'directory' ? '/' : '') })
    })
    this.contentState.showAutoImagePath(list)
  }
Repository: marktext/marktext
File: src/main/preference.js (JavaScript)

Code before:
    }
  }

  writeJson (json, async = true) {
    const { userDataPath } = this
    return new Promise((resolve, reject) => {
      const content = fs.readFileSync(userDataPath, 'utf-8')
      const tokens = content.split('```')
      const newContent = tokens[0] +
        '```json\n' +
        JSON.stringify(json, null, 2) +
        '\n```' +
        tokens[2]
      if (async) {
        fs.writeFile(userDataPath, newContent, 'utf-8', err => {
          if (err) reject(err)
          else resolve(json)
        })
      } else {
        fs.writeFileSync(userDataPath, newContent, 'utf-8')
        resolve(json)
      }
    })
  }
}

const preference = new Preference(staticPath, userDataPath)

ipcMain.on('AGANI::ask-for-user-preference', e => {
  const win = BrowserWindow.fromWebContents(e.sender)
  win.webContents.send('AGANI::user-preference', preference.getAll())
})

ipcMain.on('AGANI::set-user-preference', (e, pre) => {
  Object.keys(pre).map(key => {
    preference.setItem(key, pre[key])
      .then(() => {
        for (const win of windows.values()) {
          win.webContents.send('AGANI::user-preference', { [key]: pre[key] })
        }
      })
      .catch(log)
  })
})

export default preference


Review comment (suggestion):
Hi, @fxha we can use [`json schema`](http://json-schema.org/) to valid user setting! but I think we can put it after version 1.0.0(or you want to do before version 1.0.0). So you can add a TODO?

webpack used [ajv](https://github.com/epoberezkin/ajv)

Code after:
      return null
    }
  }

  writeJson (json, async = true) {
    const { userDataPath } = this
    return new Promise((resolve, reject) => {
      const content = fs.readFileSync(userDataPath, 'utf-8')
      const tokens = content.split('```')
      const newContent = tokens[0] +
        '```json\n' +
        JSON.stringify(json, null, 2) +
        '\n```' +
        tokens[2]
      if (async) {
        fs.writeFile(userDataPath, newContent, 'utf-8', err => {
          if (err) reject(err)
          else resolve(json)
        })
      } else {
        fs.writeFileSync(userDataPath, newContent, 'utf-8')
        resolve(json)
      }
    })
  }

  /**
   * workaround for issue #265
   *   expects: settings != null
   * @param  {Object} settings preferences object
   */
  validateSettings (settings) {
    if (!settings) {
      log('Broken settings detected: invalid settings object.')
      return
    }

    let brokenSettings = false
    if (!settings.theme || (settings.theme && !/dark|light/.test(settings.theme))) {
      brokenSettings = true
      settings.theme = 'light'
    }
    if (!settings.bulletListMarker ||
      (settings.bulletListMarker && !/\+|-|\*/.test(settings.bulletListMarker))) {
      brokenSettings = true
      settings.bulletListMarker = '-'
    }
    if (brokenSettings) {
      log('Broken settings detected; fallback to default value(s).')
    }
  }
Repository: marktext/marktext
File: src/renderer/components/editorWithTabs/editor.vue (Vue)

Code before:
<template>
  <div
    class="editor-wrapper"
    :class="[{ 'typewriter': typewriter, 'focus': focus, 'source': sourceCode }, theme]"
    :style="{ 'color': theme === 'dark' ? darkColor : lightColor, 'lineHeight': lineHeight, 'fontSize': fontSize,
    'font-family': editorFontFamily ? `${editorFontFamily}, ${defaultFontFamily}` : `${defaultFontFamily}`,
    'direction': textDirection }"
  >
    <div
      ref="editor"
      class="editor-component"
    ></div>
    <el-dialog
      :visible.sync="dialogTableVisible"
      :show-close="isShowClose"
      :modal="true"
      custom-class="ag-dialog-table"
      width="454px"
      center
    >
      <div slot="title" class="dialog-title">
        <svg class="icon" aria-hidden="true">
          <use xlink:href="#icon-table-3d"></use>
        </svg>
      </div>
      <el-form :model="tableChecker" :inline="true">
        <el-form-item label="Rows">
          <el-input-number
            ref="rowInput"
            size="mini"
            v-model="tableChecker.rows"
            controls-position="right"

Review comment (bug):
I think we should use the HTML `dir` attribute instead of css style. Please see https://stackoverflow.com/a/5375907.

Code after:
<template>
  <div
    class="editor-wrapper"
    :class="[{ 'typewriter': typewriter, 'focus': focus, 'source': sourceCode }, theme]"
    :style="{ 'color': theme === 'dark' ? darkColor : lightColor, 'lineHeight': lineHeight, 'fontSize': fontSize,
    'font-family': editorFontFamily ? `${editorFontFamily}, ${defaultFontFamily}` : `${defaultFontFamily}` }"
    :dir="textDirection"
  >
    <div
      ref="editor"
      class="editor-component"
    ></div>
    <el-dialog
      :visible.sync="dialogTableVisible"
      :show-close="isShowClose"
      :modal="true"
      custom-class="ag-dialog-table"
      width="454px"
      center
      dir='ltr'
    >
      <div slot="title" class="dialog-title">
        <svg class="icon" aria-hidden="true">
          <use xlink:href="#icon-table-3d"></use>
        </svg>
      </div>
      <el-form :model="tableChecker" :inline="true">
        <el-form-item label="Rows">
          <el-input-number
            ref="rowInput"
            size="mini"
            v-model="tableChecker.rows"
Repository: marktext/marktext
File: src/main/app.js (JavaScript)

Code before:
import { app } from 'electron'
import appMenu from './menu'
import appWindow from './window'
import { isOsx } from './config'
import { isMarkdownFile } from './utils'
import { watchers } from './utils/imagePathAutoComplement'

class App {
  constructor () {
    this.openFilesCache = []
  }

  init () {
    // Enable these features to use `backdrop-filter` css rules!
    if (isOsx) {
      app.commandLine.appendSwitch('enable-experimental-web-platform-features', 'true')
    }

    // Enable vscode chrome extension debugger connection
    if (process.env.NODE_ENV !== 'production' || process.env.NODE_ENV === 'development') {
      app.commandLine.appendSwitch('remote-debugging-port', '8315')
    }

    app.on('open-file', this.openFile.bind(this))

    app.on('ready', this.ready.bind(this))

    app.on('window-all-closed', () => {
      app.removeListener('open-file', this.openFile.bind(this))
      // close all the image path watcher
      for (const watcher of watchers.values()) {
        watcher.close()
      }
      if (!isOsx) {
        appWindow.clear()
        app.quit()
      }
    })

    app.on('activate', () => {
      // On OS X it's common to re-create a window in the app when the
      // dock icon is clicked and there are no other windows open.
      if (appWindow.windows.size === 0) {
        this.ready()
      }

Review comment (security):
Personally I would check for `process.env.NODE_ENV === 'development'` only. First of all because of security reasons and it's redundant.

Code after:
import { app } from 'electron'
import appMenu from './menu'
import appWindow from './window'
import { isOsx } from './config'
import { isMarkdownFile } from './utils'
import { watchers } from './utils/imagePathAutoComplement'

class App {
  constructor () {
    this.openFilesCache = []
  }

  init () {
    // Enable these features to use `backdrop-filter` css rules!
    if (isOsx) {
      app.commandLine.appendSwitch('enable-experimental-web-platform-features', 'true')
    }

    // Enable vscode chrome extension debugger connection
    if (process.env.NODE_ENV === 'development') {
      app.commandLine.appendSwitch('remote-debugging-port', '8315')
    }

    app.on('open-file', this.openFile.bind(this))

    app.on('ready', this.ready.bind(this))

    app.on('window-all-closed', () => {
      app.removeListener('open-file', this.openFile.bind(this))
      // close all the image path watcher
      for (const watcher of watchers.values()) {
        watcher.close()
      }
      if (!isOsx) {
        appWindow.clear()
        app.quit()
      }
    })

    app.on('activate', () => {
      // On OS X it's common to re-create a window in the app when the
      // dock icon is clicked and there are no other windows open.
      if (appWindow.windows.size === 0) {
        this.ready()
      }
Repository: marktext/marktext
File: src/muya/lib/contentState/clickCtrl.js (JavaScript)

Code before:

    // update '```xxx' to code block when you click other place or use press arrow key.
    if (block && start.key !== this.cursor.start.key) {
      const oldBlock = this.getBlock(this.cursor.start.key)
      if (oldBlock) {
        needRender = needRender || this.codeBlockUpdate(oldBlock)
      }
    }

    // change active status when paragraph changed
    if (
      start.key !== this.cursor.start.key ||
      end.key !== this.cursor.end.key
    ) {
      needRender = true
    }

    const needMarkedUpdate = this.checkNeedRender(this.cursor) || this.checkNeedRender({ start, end })

    if (needRender) {
      this.cursor = { start, end }
      return this.partialRender()
    } else if (needMarkedUpdate) {
      // Fix: whole select can not be canceled #613
      requestAnimationFrame(() => {
        this.cursor = selection.getCursorRange()
        return this.partialRender()
      })
    } else {
      this.cursor = { start, end }
    }
  }
}

export default clickCtrl


Review comment (suggestion):
Please check that `start` and `end` are not `null`.

Code after:

    // update '```xxx' to code block when you click other place or use press arrow key.
    if (block && start.key !== this.cursor.start.key) {
      const oldBlock = this.getBlock(this.cursor.start.key)
      if (oldBlock) {
        needRender = needRender || this.codeBlockUpdate(oldBlock)
      }
    }

    // change active status when paragraph changed
    if (
      start.key !== this.cursor.start.key ||
      end.key !== this.cursor.end.key
    ) {
      needRender = true
    }

    const needMarkedUpdate = this.checkNeedRender(this.cursor) || this.checkNeedRender({ start, end })

    if (needRender) {
      this.cursor = { start, end }
      return this.partialRender()
    } else if (needMarkedUpdate) {
      // Fix: whole select can not be canceled #613
      requestAnimationFrame(() => {
        const cursor = selection.getCursorRange()
        if (!cursor.start || !cursor.end) {
          return
        }
        this.cursor = cursor
        return this.partialRender()
      })
    } else {
      this.cursor = { start, end }
    }
  }
}

export default clickCtrl
Repository: marktext/marktext
File: src/renderer/store/project.js (JavaScript)

Code before:
import { PATH_SEPARATOR } from '../config'
import notice from '../services/notification'
import { getFileStateFromData } from './help'

const state = {
  activeItem: {},
  createCache: {},
  // Use to cache newly created filename, for open iimmediately.
  newFileNameCache: '',
  renameCache: null,
  clipboard: null,
  projectTree: null
}

const getters = {
  fileList: state => {
    const files = []
    const travel = folder => {
      files.push(...folder.files.filter(f => f.isMarkdown))
      for (const childFolder of folder.folders) {
        travel(childFolder)
      }
    }

    if (state.projectTree) travel(state.projectTree)
    files.sort((f1, f2) => {
      const lowerName1 = f1.name.toLowerCase()
      const lowerName2 = f2.name.toLowerCase()

      if (lowerName1 < lowerName2) {
        return -1
      } else if (lowerName1 === lowerName2) {
        return 0
      } else {
        return 1
      }
    })

    return files
  }
}

const mutations = {
  SET_PROJECT_TREE (state, pathname) {
    let name = path.basename(pathname)
    if (!name) {
      // Root directory such "/" or "C:\"
      name = pathname
    }

    state.projectTree = {

Review comment (suggestion):
I would suggest to use the build-in method `localeCompare`:

```js
files.sort((f1, f2) => f1.name.localeCompare(f2.name))
```

Code after:
import { PATH_SEPARATOR } from '../config'
import notice from '../services/notification'
import { getFileStateFromData } from './help'

const state = {
  activeItem: {},
  createCache: {},
  // Use to cache newly created filename, for open iimmediately.
  newFileNameCache: '',
  renameCache: null,
  clipboard: null,
  projectTree: null
}

const getters = {
  fileList: state => {
    const files = []
    const travel = folder => {
      files.push(...folder.files.filter(f => f.isMarkdown))
      for (const childFolder of folder.folders) {
        travel(childFolder)
      }
    }

    if (state.projectTree) travel(state.projectTree)
    files.sort((f1, f2) => f1.name.localeCompare(f2.name))

    return files
  }
}

const mutations = {
  SET_PROJECT_TREE (state, pathname) {
    let name = path.basename(pathname)
    if (!name) {
      // Root directory such "/" or "C:\"
      name = pathname
    }

    state.projectTree = {
      // Root full path
      pathname: path.normalize(pathname),
      // Root directory name
      name,
      isDirectory: true,
      isFile: false,
      isMarkdown: false,
      folders: [],
      files: []
    }
  },
Repository: marktext/marktext
File: src/renderer/prefComponents/common/textBox/index.vue (Vue)

Code before:
<template>
  <section class="pref-text-box-item" :class="{'ag-underdevelop': disable}">
    <div class="description">
      <span>{{description}}</span>
      <i class="el-icon-info" v-if="more"
        @click="handleMoreClick"
      ></i>
    </div>
    <el-input
      class="input"
      :class="{error: invalidInput}"
      :placeholder="defaultValue"
      v-model="inputText"
      @change="handleInputChange"
      clearable>
    </el-input>
  </section>
</template>

<script>
import { shell } from 'electron'

export default {
  data () {
    return {
      inputText: this.input,
      invalidInput: false
    }
  },
  props: {
    description: String,
    input: String,
    onChange: Function,
    more: String,
    disable: {
      type: Boolean,
      default: false
    },
    defaultValue: {

Review comment (suggestion):
It's better to handle `input` event? Because `change` event only fired when the input loose focus. we can listen the `input` event and add a throttle.

Code after:
<template>
  <section class="pref-text-box-item" :class="{'ag-underdevelop': disable}">
    <div class="description">
      <span>{{description}}</span>
      <i class="el-icon-info" v-if="more"
        @click="handleMoreClick"
      ></i>
    </div>
    <el-input
      class="input"
      :class="{error: invalidInput}"
      :placeholder="defaultValue"
      v-model="inputText"
      @input="handleInput"
      style="width: 240px"
      size="small"
      clearable>
    </el-input>
  </section>
</template>

<script>
import { shell } from 'electron'

export default {
  data () {
    this.inputTimer = null
    return {
      inputText: this.input,
      invalidInput: false
    }
  },
  props: {
    description: String,
    input: String,
    onChange: Function,
    more: String,
    disable: {
      type: Boolean,