ronantakizawa/codereview-bench huggingface dataset:
Repository: marktext/marktext
File: src/main/createWindow.js (JavaScript)

Code before:
'use strict'

import fs from 'fs'
import path from 'path'
import { BrowserWindow, app } from 'electron'
import windowStateKeeper from 'electron-window-state'

export const windows = new Map()

const createWindow = (pathname, options = {}) => {
  const TITLE_BAR_HEIGHT = 21
  const mainWindowState = windowStateKeeper({
    defaultWidth: 1200,
    defaultHeight: 800
  })

  const { x, y, width, height } = mainWindowState
  const winOpt = Object.assign({ x, y, width, height }, {
    webPreferences: {
      webSecurity: false
    },
    useContentSize: true,
    show: false,
    frame: (process.platform === 'linux'),
    titleBarStyle: 'hidden'
  }, options)
  let win = new BrowserWindow(winOpt)

  const winURL = process.env.NODE_ENV === 'development'
    ? `http://localhost:9080`
    : `file://${__dirname}/index.html`

  win.loadURL(winURL)
  win.setSheetOffset(TITLE_BAR_HEIGHT) // 21 is the title bar height
  win.once('ready-to-show', () => {
    win.show()

    if (pathname) {
      app.addRecentDocument(pathname)
      const filename = path.basename(pathname)
      fs.readFile(path.resolve(pathname), 'utf-8', (err, file) => {
        if (err) return console.log(err)
        win.webContents.send('AGANI::file-loaded', {
          file,
          filename,
          pathname
        })
      })
    }

Review comment (suggestion):
This PR is helpful to the Linux users, One more thing, I think the `(` and `)` are not needed.

Can you remove them, then I will merge this PR.

Code after:
'use strict'

import fs from 'fs'
import path from 'path'
import { BrowserWindow, app } from 'electron'
import windowStateKeeper from 'electron-window-state'

export const windows = new Map()

const createWindow = (pathname, options = {}) => {
  const TITLE_BAR_HEIGHT = 21
  const mainWindowState = windowStateKeeper({
    defaultWidth: 1200,
    defaultHeight: 800
  })

  const { x, y, width, height } = mainWindowState
  const winOpt = Object.assign({ x, y, width, height }, {
    webPreferences: {
      webSecurity: false
    },
    useContentSize: true,
    show: false,
    frame: process.platform === 'linux',
    titleBarStyle: 'hidden'
  }, options)
  let win = new BrowserWindow(winOpt)

  const winURL = process.env.NODE_ENV === 'development'
    ? `http://localhost:9080`
    : `file://${__dirname}/index.html`

  win.loadURL(winURL)
  win.setSheetOffset(TITLE_BAR_HEIGHT) // 21 is the title bar height
  win.once('ready-to-show', () => {
    win.show()

    if (pathname) {
      app.addRecentDocument(pathname)
      const filename = path.basename(pathname)
      fs.readFile(path.resolve(pathname), 'utf-8', (err, file) => {
        if (err) return console.log(err)
        win.webContents.send('AGANI::file-loaded', {
          file,
          filename,
          pathname
        })
      })
    }
Repository: marktext/marktext
File: src/main/utils.js (JavaScript)

Code before:
import { app, Menu } from 'electron'

export const getMenuItem = menuName => {
  const menus = Menu.getApplicationMenu()
  return menus.items.find(menu => menu.label === menuName)
}

export const getHomePath = () => {
  return app.getPath('home')
}


Review comment (suggestion):
Hi, thank you for your PR, I think you can provide a getPath uitl just call the app.getPath.

```javascript
export const getPath = directory => {
    return app.getPath(directory)
}
```

Code after:
import fs from 'fs'
import path from 'path'
import { app, Menu } from 'electron'

const JSON_REG = /```json(.+)```/g
const preferencePath = path.join(__static, 'preference.md')
let PREFERENCE_CACHE = null

export const getMenuItem = menuName => {
  const menus = Menu.getApplicationMenu()
  return menus.items.find(menu => menu.label === menuName)
}

export const getUserPreference = () => {
  if (PREFERENCE_CACHE) {
    return PREFERENCE_CACHE
  } else {
    const content = fs.readFileSync(preferencePath, 'utf-8')
    try {
      const userSetting = JSON_REG.exec(content.replace(/\n/g, ''))[1]
      PREFERENCE_CACHE = JSON.parse(userSetting)
      return PREFERENCE_CACHE
    } catch (err) {
      // todo notice the user
      console.log(err)
    }
  }
}

export const setUserPreference = (key, value) => {
  const preUserSetting = getUserPreference()
  const newUserSetting = PREFERENCE_CACHE = Object.assign({}, preUserSetting, { [key]: value })
Repository: marktext/marktext
File: src/main/index.js (JavaScript)

Code before:
'use strict'

import './globalSetting'
import fs from 'fs'
import { app, ipcMain, Menu } from 'electron'
import configureMenu, { dockMenu } from './menus'
import createWindow, { windows } from './createWindow'
import { hasMarkdownExtension } from './utils'
// import { autoUpdater } from "electron-updater"

let openFilesCache = []

const onReady = () => {
  if (process.platform === 'win32' && process.argv.length >= 2) {
    openFilesCache = [process.argv[1]]
  }
  if (openFilesCache.length) {
    openFilesCache.forEach(path => createWindow(path))
    openFilesCache.length = 0 // empty the open file path cache
  } else {
    createWindow()
  }
  const menu = Menu.buildFromTemplate(configureMenu({ app }))
  Menu.setApplicationMenu(menu)
  if (process.platform === 'darwin') {
    // app.dock is only for macosx
    app.dock.setMenu(dockMenu)
  }
}

Review comment (bug):
Pls put native node module `fs` before other modules from npm, but don't put it before `import './globalSetting'`, it maybe cause crash bug.

Code after:
'use strict'

import './globalSetting'
import { app, ipcMain, Menu } from 'electron'
import configureMenu, { dockMenu } from './menus'
import createWindow, { windows } from './createWindow'
import { isMarkdownFile } from './utils'
// import { autoUpdater } from "electron-updater"

let openFilesCache = []

const onReady = () => {
  if (process.platform !== 'darwin' && process.argv.length >= 2) {
    for (const arg of process.argv) {
      if (isMarkdownFile(arg)) {
        openFilesCache = [arg]
        break
      }
    }
  }
  if (openFilesCache.length) {
    openFilesCache.forEach(path => createWindow(path))
    openFilesCache.length = 0 // empty the open file path cache
  } else {
    createWindow()
  }
  const menu = Menu.buildFromTemplate(configureMenu({ app }))
  Menu.setApplicationMenu(menu)
  if (process.platform === 'darwin') {
Repository: marktext/marktext
File: src/renderer/components/about/index.vue (Vue)

Code before:
        <el-col :span="24">
          <h3 class="text fg-color-dark">{{ name }}</h3>
        </el-col>
        <el-col :span="24">
          <div class="text">{{ versionPrefix }}{{ version }}</div>
        </el-col>
        <el-col :span="24">
          <div class="text">Copyright © 2018 Jocs</div>
        </el-col>
      </el-row>
  </div>
</template>

<script>
  import { remote } from 'electron'
  import { mapState } from 'vuex'
  import bus from '../../bus'

  export default {
    data () {
      return {
        showAboutDialog: false,
        name: 'Mark Text',
        version: remote.app.getVersion(),
        versionPrefix: 'v',
        markTextLogo: require('../../../../static/logo-small.png')
      }
    },
    computed: {
      ...mapState([
        'theme'
      ])
    },
    created () {
      bus.$on('aboutDialog', this.showDialog)
    },
    methods: {
      showDialog () {
        this.showAboutDialog = true
      }
    }
  }
</script>

<style>
  .about-dialog el-row,
  .about-dialog el-col {
    display: block;
  }

  .about-dialog .logo {

Review comment (suggestion):
put `logo-small.png` in the path `src/render/assets/images/logo-small.png`, and don't need to `require` or `import`, just write it to the `src` of `img` tag.

BTY, the size of `logo-small.png` is too big, I think 150 * 150 px is enough, can you resize it, and rename it to `logo.png`

Code after:
        <el-col :span="24">
          <h3 class="text fg-color-dark">{{ name }}</h3>
        </el-col>
        <el-col :span="24">
          <div class="text">{{ versionPrefix }}{{ appVersion }}</div>
        </el-col>
        <el-col :span="24">
          <div class="text">Copyright © 2018 Jocs</div>
        </el-col>
      </el-row>
  </div>
</template>

<script>
  import { mapState } from 'vuex'
  import bus from '../../bus'

  export default {
    data () {
      return {
        showAboutDialog: false,
        name: 'Mark Text',
        versionPrefix: 'v'
      }
    },
    computed: {
      ...mapState([
        'appVersion',
        'theme'
      ])
    },
    created () {
      bus.$on('aboutDialog', this.showDialog)
    },
    beforeDestroy () {
      bus.$off('aboutDialog', this.showDialog)
    },
    methods: {
      showDialog () {
        this.showAboutDialog = true
      }
    }
  }
</script>

<style>
  .about-dialog el-row,
  .about-dialog el-col {
    display: block;
  }
Repository: marktext/marktext
File: src/editor/contentState/updateCtrl.js (JavaScript)

Code before:
import { ipcRenderer } from 'electron'
import selection from '../selection'
import { tokenizer } from '../parser/parse'
import { conflict } from '../utils'

const INLINE_UPDATE_REG = /^([*+-]\s)|^(\[[x\s]{1}\]\s)|^(\d+\.\s)|^(#{1,6})[^#]+|^(>).+|^(\*{3,}|-{3,}|_{3,})/i

let prefListItemType = 'loose'

// workaround to get preferences at startup
ipcRenderer.send('AGANI::ask-for-user-preference')
ipcRenderer.on('AGANI::user-preference', (e, preference) => {
  const { listItemType } = preference
  prefListItemType = listItemType
})

const updateCtrl = ContentState => {
  ContentState.prototype.checkNeedRender = function (block) {
    const { start: cStart, end: cEnd } = this.cursor
    const startOffset = cStart.offset
    const endOffset = cEnd.offset
    const tokens = tokenizer(block.text)
    const len = tokens.length
    const textLen = block.text.length
    let i


Review comment (refactor):
Do not use electron API in `editor` folder, Because codes in `editor` folder will be extracted to a new library in the future which can be use for web development, not just electron APP.

Put all preferences in `render/store` is a good practice.

Code after:
import selection from '../selection'
import { tokenizer } from '../parser/parse'
import { conflict } from '../utils'
import { getTextContent } from '../utils/domManipulate'
import { CLASS_OR_ID } from '../config'

const INLINE_UPDATE_REG = /^([*+-]\s)|^(\[[x\s]{1}\]\s)|^(\d+\.\s)|^(#{1,6})[^#]+|^(>).+|^(\*{3,}|-{3,}|_{3,})/i

const updateCtrl = ContentState => {
  ContentState.prototype.checkNeedRender = function (block) {
    const { start: cStart, end: cEnd } = this.cursor
    const startOffset = cStart.offset
    const endOffset = cEnd.offset
    const tokens = tokenizer(block.text)
    const len = tokens.length
    const textLen = block.text.length
    let i

    for (i = 0; i < len; i++) {
      const token = tokens[i]
      if (token.type === 'text') continue
      const { start, end } = token.range
      if (
        conflict([Math.max(0, start - 1), Math.min(textLen, end + 1)], [startOffset, startOffset]) ||
        conflict([Math.max(0, start - 1), Math.min(textLen, end + 1)], [endOffset, endOffset])
      ) {
Repository: marktext/marktext
File: src/main/preference.js (JavaScript)

Code before:
import fs from 'fs'
import path from 'path'
import { ipcMain, BrowserWindow } from 'electron'
import { windows } from './createWindow'
import { getPath, hasSameKeys, log, mkdir } from './utils'

const FILE_NAME = 'preference.md'
const staticPath = path.join(__static, FILE_NAME)
const userDataPath = path.join(getPath('userData'), FILE_NAME)

class Preference {
  constructor (staticPath, userDataPath) {
    this.cache = null
    this.staticPath = staticPath
    this.userDataPath = userDataPath
    this.requiresUpdate = false

    this.outdatedKeys = [
      {
        name: 'color',
        action: function (oldKey, oldValue) {
          return {
            removeOldKey: true,
            settings: {
              darkColor: '#ffffff',
              lightColor: '#000000'
            }
          }
        }
      }
    ]

    if (!fs.existsSync(userDataPath)) {
      mkdir(getPath('userData'))
      const content = fs.readFileSync(staticPath, 'utf-8')
      fs.writeFileSync(userDataPath, content, 'utf-8')
    } else {
      // load settings into cache
      this.getAll()

      // if needed update settings to latest version
      this.update()
    }

Review comment (suggestion):
I maybe make you confused.

I gave the example of `color`, `darkColor` and `lightColor`.

It's just an example, and we should compare the old preferences and new preferences in `userDataPath` and `staticPath`, and compute the outdate preferences and newly added preferences.

Then we delete the outdate keys and add the new ones.

Code after:
import fs from 'fs'
import path from 'path'
import { ipcMain, BrowserWindow } from 'electron'
import { windows } from './createWindow'
import { getPath, hasSameKeys, log, mkdir } from './utils'

const FILE_NAME = 'preference.md'
const staticPath = path.join(__static, FILE_NAME)
const userDataPath = path.join(getPath('userData'), FILE_NAME)

class Preference {
  constructor (staticPath, userDataPath) {
    this.cache = null
    this.staticPath = staticPath
    this.userDataPath = userDataPath
    this.requiresUpdate = false

    if (!fs.existsSync(userDataPath)) {
      mkdir(getPath('userData'))
      const content = fs.readFileSync(staticPath, 'utf-8')
      fs.writeFileSync(userDataPath, content, 'utf-8')
    } else {
      // load settings into cache
      this.getAll()

      // if needed update settings to latest version
      this.update()
    }
  }

  getAll () {
    const { cache } = this
    if (cache) {
      return cache
    } else {
      const defaultSettings = this.loadJson(staticPath)
      const userSettings = this.loadJson(userDataPath)
      if (userSettings) {
        // combine latest settings with user settings
        this.cache = Object.assign({}, defaultSettings, userSettings)
        // check if we have to update user settings
        this.requiresUpdate = !hasSameKeys(defaultSettings, userSettings)
        return this.cache
Repository: marktext/marktext
File: src/main/createWindow.js (JavaScript)

Code before:
          file,
          filename,
          pathname
        })
      })
    }
  })

  win.on('focus', () => {
    win.webContents.send('AGANI::window-active-status', { status: true })
  })

  win.on('blur', () => {
    win.webContents.send('AGANI::window-active-status', { status: false })
  })

  win.on('close', event => { // before closed
    event.preventDefault()
    win.webContents.send('AGANI::ask-for-close')
  })

  windows.set(win.id, win)
  return win
}

const ensureWindowPosition = (mainWindowState) => {
  let { x, y, width, height } = mainWindowState
  let center = false
  if (x === undefined || y === undefined) {
    center = true
  } else {
    center = !screen.getAllDisplays().map(display =>
      x >= display.bounds.x && x <= display.bounds.x + display.bounds.width &&
      y >= display.bounds.y && y <= display.bounds.y + display.bounds.height)
      .some(display => display)
  }
  if (center) {
    // win.center() and "workArea" doesn't work on Linux
    const screenArea = process.platform === 'linux' ? screen.getPrimaryDisplay().bounds : screen.getPrimaryDisplay().workArea
    x = Math.ceil(screenArea.x + (screenArea.width - width) / 2)
    y = Math.ceil(screenArea.y + (screenArea.height - height) / 2)
  }
  return {
    x,
    y,
    width,
    height
  }
}

export default createWindow

Review comment (refactor):
This utility function can be put into `utils.js` file. 
And according to the the code style, the parenthesis around the param is not needed.

Code after:
          file,
          filename,
          pathname
        })
      })
    }
  })

  win.on('focus', () => {
    win.webContents.send('AGANI::window-active-status', { status: true })
  })

  win.on('blur', () => {
    win.webContents.send('AGANI::window-active-status', { status: false })
  })

  win.on('close', event => { // before closed
    event.preventDefault()
    win.webContents.send('AGANI::ask-for-close')
  })

  windows.set(win.id, win)
  return win
}

const ensureWindowPosition = mainWindowState => {
  let { x, y, width, height } = mainWindowState
  let center = false
  if (x === undefined || y === undefined) {
    center = true
  } else {
    center = !screen.getAllDisplays().map(display =>
      x >= display.bounds.x && x <= display.bounds.x + display.bounds.width &&
      y >= display.bounds.y && y <= display.bounds.y + display.bounds.height)
      .some(display => display)
  }
  if (center) {
    // win.center() and "workArea" doesn't work on Linux
    const screenArea = process.platform === 'linux' ? screen.getPrimaryDisplay().bounds : screen.getPrimaryDisplay().workArea
    x = Math.ceil(screenArea.x + (screenArea.width - width) / 2)
    y = Math.ceil(screenArea.y + (screenArea.height - height) / 2)
  }
  return {
    x,
    y,
    width,
    height
  }
}

export default createWindow
Repository: marktext/marktext
File: src/editor/utils/exportStyledHTML.js (JavaScript)

Code before:
import axios from 'axios'
import cheerio from 'cheerio'
import { parse, toPlainObject, fromPlainObject, generate } from 'css-tree'
import { CLASS_OR_ID, DAED_REMOVE_SELECTOR } from '../config'
import { collectImportantComments, unescapeHtml } from './index'

class ExportHTML {
  async generate () {
    const html = this.getHtml()
    const style = await this.getStyle()
    const themeName = this.getTheme()
    const outputHtml = `<!DOCTYPE html>
      <html lang="en">
      <head>
        <meta charset="UTF-8">
        <title>Mark Text</title>
        <style>
        ${style}
        html, body {
          display: block;
          position: relative;
          height: 100%;
        }
        a {
          pointer-events: auto;
        }
        </style>
      </head>
      <body class="${themeName}">
        ${html}
      </body>
    </html>`
    return outputHtml
  }
  async getStyle () {
    const links = Array.from(document.querySelectorAll('link'))

Review comment (suggestion):
`src/editor` is the editor provider, and should not get the *theme* from a client `editor.vue` which use the editor.

Use `constructor` is another way to get the theme.

```javascript
  constructor (theme) {
    this.theme = theme
  }
// use
new ExportHTML(theme)
```



Code after:
import axios from 'axios'
import cheerio from 'cheerio'
import { parse, toPlainObject, fromPlainObject, generate } from 'css-tree'
import { CLASS_OR_ID, DAED_REMOVE_SELECTOR } from '../config'
import { collectImportantComments, unescapeHtml } from './index'

class ExportHTML {
  async generate (themeName) {
    const html = this.getHtml()
    const style = await this.getStyle()
    const outputHtml = `<!DOCTYPE html>
      <html lang="en">
      <head>
        <meta charset="UTF-8">
        <title>Mark Text</title>
        <style>
        ${style}
        html, body, body.fillscreen {
          display: block;
          position: relative;
          height: 100%;
        }
        a {
          pointer-events: auto;
        }
        hr {
          height: 4px;
          padding: 0;
          margin: 16px 0;
          background-color: #e7e7e7;
          border: 0 none;
          overflow: hidden;
          box-sizing: content-box;
        }
        .dark hr {
          background-color: #545454;
Repository: marktext/marktext
File: src/editor/utils/exportStyledHTML.js (JavaScript)

Code before:
import { CLASS_OR_ID, DAED_REMOVE_SELECTOR } from '../config'
import { collectImportantComments, unescapeHtml } from './index'

class ExportHTML {
  async generate () {
    const html = this.getHtml()
    const style = await this.getStyle()
    const themeName = this.getTheme()
    const outputHtml = `<!DOCTYPE html>
      <html lang="en">
      <head>
        <meta charset="UTF-8">
        <title>Mark Text</title>
        <style>
        ${style}
        html, body {
          display: block;
          position: relative;
          height: 100%;
        }
        a {
          pointer-events: auto;
        }
        </style>
      </head>
      <body class="${themeName}">
        ${html}
      </body>
    </html>`
    return outputHtml
  }
  async getStyle () {
    const links = Array.from(document.querySelectorAll('link'))
    const styles = Array.from(document.querySelectorAll('style'))
    let styleSheets = []

    const DEAD_OBVIOUS = new Set(['*', 'body', 'html'])

    const checker = selector => {
      if (DAED_REMOVE_SELECTOR.has(selector)) {
        return false
      }
      if (DEAD_OBVIOUS.has(selector)) {
        return true
      }
      if (/:-(ms|moz)-/.test(selector)) {
        return true
      }
      if (/:{1,2}(before|after)/.test(selector)) {
        return true
      }

Review comment (suggestion):
also need to add `editor-wrapper` class name to get the dark background when the theme is `dark`.
`light` theme is also need the class name.

Code after:
import { CLASS_OR_ID, DAED_REMOVE_SELECTOR } from '../config'
import { collectImportantComments, unescapeHtml } from './index'

class ExportHTML {
  async generate (themeName) {
    const html = this.getHtml()
    const style = await this.getStyle()
    const outputHtml = `<!DOCTYPE html>
      <html lang="en">
      <head>
        <meta charset="UTF-8">
        <title>Mark Text</title>
        <style>
        ${style}
        html, body, body.fillscreen {
          display: block;
          position: relative;
          height: 100%;
        }
        a {
          pointer-events: auto;
        }
        hr {
          height: 4px;
          padding: 0;
          margin: 16px 0;
          background-color: #e7e7e7;
          border: 0 none;
          overflow: hidden;
          box-sizing: content-box;
        }
        .dark hr {
          background-color: #545454;
        }
        </style>
      </head>
      <body class="editor-wrapper fillscreen ${themeName}">
        ${html}
      </body>
    </html>`
    return outputHtml
  }
  async getStyle () {
    const links = Array.from(document.querySelectorAll('link'))
    const styles = Array.from(document.querySelectorAll('style'))
    let styleSheets = []

    const DEAD_OBVIOUS = new Set(['*', 'body', 'html'])

    const checker = selector => {
      if (DAED_REMOVE_SELECTOR.has(selector)) {