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

Code before:
  darkColor: 'rgb(217, 217, 217)', // color in dark theme
  autoSave: false,
  preferLooseListItem: true, // prefer loose or tight list items
  autoPairBracket: true,
  autoPairMarkdownSyntax: true,
  autoPairQuote: true,
  // edit mode
  typewriter: false, // typewriter mode
  focus: false, // focus mode
  sourceCode: false, // source code mode
  pathname: '',
  isSaved: true,
  markdown: '',
  isUtf8BomEncoded: false,
  cursor: null,
  windowActive: true,
  wordCount: {
    paragraph: 0,
    word: 0,
    character: 0,
    all: 0
  },
  platform: process.platform
}

const getMarkdownWithEncoding = state => {
  const { markdown, isUtf8BomEncoded } = state
  if (isUtf8BomEncoded) {
    return '\uFEFF' + markdown
  }
  return markdown
}

const mutations = {
  SET_MODE (state, { type, checked }) {
    state[type] = checked
  },
  SET_SEARCH (state, value) {
    state.searchMatches = value
  },
  SET_WIN_STATUS (state, status) {
    state.windowActive = status
  },
  SET_FILENAME (state, filename) {
    state.filename = filename
  },
  SET_PATHNAME (state, pathname) {
    window.__dirname = path.dirname(pathname)
    state.pathname = pathname
  },
  SET_SAVE_STATUS (state, status) {

Review comment (suggestion):
I think put `getMarkdownWithEncoding ` in `main` folder is better,

So `add/remove` `0xFEFF` are at the same place.

Code after:
  darkColor: 'rgb(217, 217, 217)', // color in dark theme
  autoSave: false,
  preferLooseListItem: true, // prefer loose or tight list items
  autoPairBracket: true,
  autoPairMarkdownSyntax: true,
  autoPairQuote: true,
  // edit mode
  typewriter: false, // typewriter mode
  focus: false, // focus mode
  sourceCode: false, // source code mode
  pathname: '',
  isSaved: true,
  markdown: '',
  isUtf8BomEncoded: false,
  cursor: null,
  windowActive: true,
  wordCount: {
    paragraph: 0,
    word: 0,
    character: 0,
    all: 0
  },
  platform: process.platform
}

const mutations = {
  SET_MODE (state, { type, checked }) {
    state[type] = checked
  },
  SET_SEARCH (state, value) {
    state.searchMatches = value
  },
  SET_WIN_STATUS (state, status) {
    state.windowActive = status
  },
  SET_FILENAME (state, filename) {
    state.filename = filename
  },
  SET_PATHNAME (state, pathname) {
    window.__dirname = path.dirname(pathname)
    state.pathname = pathname
  },
  SET_SAVE_STATUS (state, status) {
    state.isSaved = status
  },
  SET_MARKDOWN (state, markdown) {
    state.markdown = markdown
  },
  SET_IS_UTF8_BOM_ENCODED (state, isUtf8BomEncoded) {
    state.isUtf8BomEncoded = isUtf8BomEncoded
  },
Repository: marktext/marktext
File: src/editor/utils/index.js (JavaScript)

Code before:
    }
  }
  return result
}

export const deepCopy = object => {
  // return JSON.parse(JSON.stringify(object))
  const obj = {}
  Object.keys(object).forEach(key => {
    if (typeof object[key] === 'object' && object[key] !== null) {
      if (Array.isArray(object[key])) {
        obj[key] = deepCopyArray(object[key])
      } else {
        obj[key] = deepCopy(object[key])
      }
    } else {
      obj[key] = object[key]
    }
  })
  return obj
}

export const loadImage = (url, detectContentType) => {
  return new Promise((resolve, reject) => {
    try {
      if (detectContentType && !checkImageContentType(url)) {
        reject(new Error('not a image'))
        return
      }

      const image = new Image()
      image.onload = () => {
        resolve(url)
      }
      image.onerror = err => {
        reject(err)
      }
      image.src = url
    } catch (err) {
      reject(err)
    }
  })
}

export const checkImageContentType = async url => {
  try {
    const res = await axios.head(url)
    const contentType = res.headers['content-type']

    console.log(`http: ${res.status} -> ${contentType}`)


Review comment (suggestion):
checkImageContentType(url) will return a promise, a promise is a `object`, So `! checkImageContentType` is always be `false`.

You should not put a promise in `if conditions`.

```javascript
const loadImage = async (url, detectContentType) => {
  const isImage = await checkImageContentType(url)
  if (detectContentType && isImage) {
    // other codes
  }
  // other codes
}
```

Code after:
}

export const loadImage = async (url, detectContentType) => {
  if (detectContentType) {
    const isImage = await checkImageContentType(url)
    if (!isImage) throw new Error('not an image')
  }
  return new Promise((resolve, reject) => {
    const image = new Image()
    image.onload = () => {
      resolve(url)
    }
    image.onerror = err => {
      reject(err)
    }
    image.src = url
  })
}

export const checkImageContentType = async url => {
  try {
    const res = await axios.head(url)
    const contentType = res.headers['content-type']
    if (res.status === 200 && /^image\/(?:jpeg|png|gif|svg\+xml|webp)$/.test(contentType)) {
      return true
    }
    return false
  } catch (err) {
    return false
  }
}

export const collectImportantComments = css => {
  const once = new Set()
  const cleaned = css.replace(/(\/\*![\s\S]*?\*\/)\n*/gm, (match, p1) => {
    once.add(p1)
    return ''
  })
  const combined = Array.from(once)
  combined.push(cleaned)
  return combined.join('\n')
}

export const getImageInfo = src => {
  const EXT_REG = /\.(jpeg|jpg|png|gif|svg|webp)(?=\?|$)/i
  // http[s] (domain or IPv4 or localhost or IPv6) [port] /not-white-space
  const URL_REG = /^http(s)?:\/\/([a-z0-9\-._~]+\.[a-z]{2,}|[0-9.]+|localhost|\[[a-f0-9.:]+\])(:[0-9]{1,5})?\/[\S]+/i
  const imageExtension = EXT_REG.test(src)
  const isUrl = URL_REG.test(src)
  if (imageExtension) {
    if (isUrl || !window.__dirname) {
Repository: marktext/marktext
File: src/editor/utils/index.js (JavaScript)

Code before:
    }
  })
  return obj
}

export const loadImage = async (url, detectContentType) => {
  if (detectContentType && await !checkImageContentType(url)) {
    throw new Error('not a image')
  }
  return new Promise((resolve, reject) => {
    const image = new Image()
    image.onload = () => {
      resolve(url)
    }
    image.onerror = err => {
      reject(err)
    }
    image.src = url
  })
}

export const checkImageContentType = async url => {
  try {
    const res = await axios.head(url)
    const contentType = res.headers['content-type']
    if (res.status === 200 && (contentType === 'image/jpeg' ||
      contentType === 'image/png' ||
      contentType === 'image/gif' ||
      contentType === 'image/svg+xml' ||
      contentType === 'image/webp')) {
      return true
    }
    return false
  } catch (err) {
    return false
  }
}

export const collectImportantComments = css => {
  const once = new Set()
  const cleaned = css.replace(/(\/\*![\s\S]*?\*\/)\n*/gm, (match, p1) => {
    once.add(p1)
    return ''
  })
  const combined = Array.from(once)
  combined.push(cleaned)
  return combined.join('\n')
}

export const getImageInfo = src => {
  const EXT_REG = /\.(jpeg|jpg|png|gif|svg|webp)(?=\?|$)/i

Review comment (suggestion):
Regexp is preferred.

```javascript
/^image\/(?:jpeg|png|gif|svg\+xml|webp)$/.test(contentType) // not test yet!!!
```

Code after:
    }
  })
  return obj
}

export const loadImage = async (url, detectContentType) => {
  if (detectContentType) {
    const isImage = await checkImageContentType(url)
    if (!isImage) throw new Error('not an image')
  }
  return new Promise((resolve, reject) => {
    const image = new Image()
    image.onload = () => {
      resolve(url)
    }
    image.onerror = err => {
      reject(err)
    }
    image.src = url
  })
}

export const checkImageContentType = async url => {
  try {
    const res = await axios.head(url)
    const contentType = res.headers['content-type']
    if (res.status === 200 && /^image\/(?:jpeg|png|gif|svg\+xml|webp)$/.test(contentType)) {
      return true
    }
    return false
  } catch (err) {
    return false
  }
}

export const collectImportantComments = css => {
  const once = new Set()
  const cleaned = css.replace(/(\/\*![\s\S]*?\*\/)\n*/gm, (match, p1) => {
    once.add(p1)
    return ''
  })
  const combined = Array.from(once)
  combined.push(cleaned)
  return combined.join('\n')
}

export const getImageInfo = src => {
  const EXT_REG = /\.(jpeg|jpg|png|gif|svg|webp)(?=\?|$)/i
  // http[s] (domain or IPv4 or localhost or IPv6) [port] /not-white-space
  const URL_REG = /^http(s)?:\/\/([a-z0-9\-._~]+\.[a-z]{2,}|[0-9.]+|localhost|\[[a-f0-9.:]+\])(:[0-9]{1,5})?\/[\S]+/i
  const imageExtension = EXT_REG.test(src)
Repository: marktext/marktext
File: src/editor/utils/index.js (JavaScript)

Code before:
    } else {
      result.push(array[i])
    }
  }
  return result
}

// TODO: @jocs rewrite deepCopy
export const deepCopy = object => {
  const obj = {}
  Object.keys(object).forEach(key => {
    if (typeof object[key] === 'object' && object[key] !== null) {
      if (Array.isArray(object[key])) {
        obj[key] = deepCopyArray(object[key])
      } else {
        obj[key] = deepCopy(object[key])
      }
    } else {
      obj[key] = object[key]
    }
  })
  return obj
}

export const loadImage = async (url, detectContentType) => {
  if (detectContentType && await !checkImageContentType(url)) {
    throw new Error('not a image')
  }
  return new Promise((resolve, reject) => {
    const image = new Image()
    image.onload = () => {
      resolve(url)
    }
    image.onerror = err => {
      reject(err)
    }
    image.src = url
  })
}

export const checkImageContentType = async url => {
  try {
    const res = await axios.head(url)
    const contentType = res.headers['content-type']
    if (res.status === 200 && (contentType === 'image/jpeg' ||
      contentType === 'image/png' ||
      contentType === 'image/gif' ||
      contentType === 'image/svg+xml' ||
      contentType === 'image/webp')) {
      return true
    }

Review comment (performance):
Do not use `await` statements in if condition, you can use a variable to cache this `await` statement result.

```javascript
if (detectContentType) {
    const isImage = await checkImageContentType(url)
    if (!isImage) return Promise.reject('not an image')
}
return new Promise((resolve, reject) => { // other codes })
// other codes
```

Code after:
    } else {
      result.push(array[i])
    }
  }
  return result
}

// TODO: @jocs rewrite deepCopy
export const deepCopy = object => {
  const obj = {}
  Object.keys(object).forEach(key => {
    if (typeof object[key] === 'object' && object[key] !== null) {
      if (Array.isArray(object[key])) {
        obj[key] = deepCopyArray(object[key])
      } else {
        obj[key] = deepCopy(object[key])
      }
    } else {
      obj[key] = object[key]
    }
  })
  return obj
}

export const loadImage = async (url, detectContentType) => {
  if (detectContentType) {
    const isImage = await checkImageContentType(url)
    if (!isImage) throw new Error('not an image')
  }
  return new Promise((resolve, reject) => {
    const image = new Image()
    image.onload = () => {
      resolve(url)
    }
    image.onerror = err => {
      reject(err)
    }
    image.src = url
  })
}

export const checkImageContentType = async url => {
  try {
    const res = await axios.head(url)
    const contentType = res.headers['content-type']
    if (res.status === 200 && /^image\/(?:jpeg|png|gif|svg\+xml|webp)$/.test(contentType)) {
      return true
    }
    return false
  } catch (err) {
    return false
Repository: marktext/marktext
File: src/editor/utils/index.js (JavaScript)

Code before:
  }
  return result
}

// TODO: @jocs rewrite deepCopy
export const deepCopy = object => {
  const obj = {}
  Object.keys(object).forEach(key => {
    if (typeof object[key] === 'object' && object[key] !== null) {
      if (Array.isArray(object[key])) {
        obj[key] = deepCopyArray(object[key])
      } else {
        obj[key] = deepCopy(object[key])
      }
    } else {
      obj[key] = object[key]
    }
  })
  return obj
}

export const loadImage = url => {
  const image = new Image()
  return new Promise((resolve, reject) => {
    image.onload = () => {
      resolve(url)
    }
    image.onerror = err => {
      reject(err)
    }
    image.src = url
  })
}

export const collectImportantComments = css => {
  const once = new Set()
  const cleaned = css.replace(/(\/\*![\s\S]*?\*\/)\n*/gm, (match, p1) => {
    once.add(p1)
    return ''
  })
  const combined = Array.from(once)
  combined.push(cleaned)
  return combined.join('\n')
}

export const getImageSrc = src => {
  const EXT_REG = /\.(jpeg|jpg|png|gif|svg|webp)(?=\?|$)/i
  const HTTP_REG = /^http(s)?:/
  if (EXT_REG.test(src)) {
    return (HTTP_REG.test(src) || !window.__dirname)
      ? src

Review comment (suggestion):
I think you use `throw new Error()` just want to make the promise(loadImage returned) rejected so that we can catch it in `.catch()` block. but why don't you use `Promise.reject()` directly?

```javascript
if (!isImage) return Promise.reject(new Error('not an image'))
```

Code after:
    }
  }
  return result
}

// TODO: @jocs rewrite deepCopy
export const deepCopy = object => {
  const obj = {}
  Object.keys(object).forEach(key => {
    if (typeof object[key] === 'object' && object[key] !== null) {
      if (Array.isArray(object[key])) {
        obj[key] = deepCopyArray(object[key])
      } else {
        obj[key] = deepCopy(object[key])
      }
    } else {
      obj[key] = object[key]
    }
  })
  return obj
}

export const loadImage = async (url, detectContentType) => {
  if (detectContentType) {
    const isImage = await checkImageContentType(url)
    if (!isImage) throw new Error('not an image')
  }
  return new Promise((resolve, reject) => {
    const image = new Image()
    image.onload = () => {
      resolve(url)
    }
    image.onerror = err => {
      reject(err)
    }
    image.src = url
  })
}

export const checkImageContentType = async url => {
  try {
    const res = await axios.head(url)
    const contentType = res.headers['content-type']
    if (res.status === 200 && /^image\/(?:jpeg|png|gif|svg\+xml|webp)$/.test(contentType)) {
      return true
    }
    return false
  } catch (err) {
    return false
  }
}
Repository: marktext/marktext
File: src/main/config.js (JavaScript)

Code before:
  'jpeg',
  'jpg',
  'png',
  'gif',
  'svg',
  'webp'
]

export const BLACK_LIST = [
  '$RECYCLE.BIN'
]

export const EXTENSION_HASN = {
  styledHtml: '.html',
  html: '.html',
  pdf: '.pdf'
}

export const VIEW_MENU_ITEM = {
  'Source Code Mode': false,
  'Typewriter Mode': false,
  'Focus Mode': false
}

export const LINE_ENDING_REG = /(\r\n|\n)/g
export const LF_LINE_ENDING_REG = /([^\r]\n)/
export const CRLF_LINE_ENDING_REG = /(\r\n)/


Review comment (suggestion):
If a file only have one line, and only line is `\n`?

is the line break is LF?

So is this RegExp expression better.

```javascript
export const LF_LINE_ENDING_REG = /(^\n|[^\r]\n)/
```

But I am not test the regexp bellow. 

Code after:
  'jpeg',
  'jpg',
  'png',
  'gif',
  'svg',
  'webp'
]

export const BLACK_LIST = [
  '$RECYCLE.BIN'
]

export const EXTENSION_HASN = {
  styledHtml: '.html',
  html: '.html',
  pdf: '.pdf'
}

export const VIEW_MENU_ITEM = {
  'Source Code Mode': false,
  'Typewriter Mode': false,
  'Focus Mode': false
}

export const LINE_ENDING_REG = /(?:\r\n|\n)/g
export const LF_LINE_ENDING_REG = /(?:[^\r]\n)|(?:^\n$)/
export const CRLF_LINE_ENDING_REG = /\r\n/
Repository: marktext/marktext
File: src/main/menus/edit.js (JavaScript)

Code before:
      actions.edit(browserWindow, 'aidou')
    }
  }, {
    label: 'Insert Image',
    submenu: [{
      label: 'Absolute Path',
      click (menuItem, browserWindow) {
        actions.insertImage(browserWindow, 'absolute')
      }
    }, {
      label: 'Relative Path',
      click (menuItem, browserWindow) {
        actions.insertImage(browserWindow, 'relative')
      }
    }, {
      label: 'Upload to Cloud (EXP)',
      click (menuItem, browserWindow) {
        actions.insertImage(browserWindow, 'upload')
      }
    }]
  }, {
    type: 'separator'
  }, {
    label: 'Line Ending',
    submenu: [{
      label: 'Carriage return and line feed (CRLF)',
      click (menuItem, browserWindow) {
        actions.lineEnding(browserWindow, 'crlf')
      }
    }, {
      label: 'Line feed (LF)',
      click (menuItem, browserWindow) {
        actions.lineEnding(browserWindow, 'lf')
      }
    }]
  }]
}


Review comment (suggestion):
'Carriage return and line feed (CRLF)' and 'Line feed (LF)' are two opposite menu item. So I think these two menu item should be type `radio`, in others words, you can only select one of them.

like this bellow:

```javascript
label: 'Carriage return and line feed (CRLF)',
type: 'radio'
// other codes
```

this will add a `checked marker` at the beginning of the selected menu item.

Another thing is.

The selected status should also be the identity of the line break of the current edit document in active window.

For example, if the current edit file is `LF` and the `Line feed (LF)` menu item should be selected. and I changed to another `CRLF` file and the `'Carriage return and line feed (CRLF)'` menu item should be selected.

Code after:
      actions.edit(browserWindow, 'aidou')
    }
  }, {
    label: 'Insert Image',
    submenu: [{
      label: 'Absolute Path',
      click (menuItem, browserWindow) {
        actions.insertImage(browserWindow, 'absolute')
      }
    }, {
      label: 'Relative Path',
      click (menuItem, browserWindow) {
        actions.insertImage(browserWindow, 'relative')
      }
    }, {
      label: 'Upload to Cloud (EXP)',
      click (menuItem, browserWindow) {
        actions.insertImage(browserWindow, 'upload')
      }
    }]
  }, {
    type: 'separator'
  }, {
    label: 'Line Ending',
    submenu: [{
      id: 'crlfLineEndingMenuEntry',
      label: 'Carriage return and line feed (CRLF)',
      type: 'radio',
      click (menuItem, browserWindow) {
        actions.lineEnding(browserWindow, 'crlf')
      }
    }, {
      id: 'lfLineEndingMenuEntry',
      label: 'Line feed (LF)',
      type: 'radio',
      click (menuItem, browserWindow) {
        actions.lineEnding(browserWindow, 'lf')
      }
    }]
  }]
}
Repository: marktext/marktext
File: src/editor/contentState/selectionCtrl.js (JavaScript)

Code before:
import selection from '../selection'

const selectionCtrl = ContentState => {
  ContentState.prototype.selectionHandler = function (event) {
    const { start } = this.cursor
    const block = this.getBlock(start.key)
    if (/th|td/.test(block.type)) {
      event.preventDefault()
      console.log('table detected!') // TODO: #DEBUG

      // @Jocs Selects nothing...
      selection.setCursorRange({
        start: start,
        end: start
      })

      // @Jocs Selects full text...
      // const target = event.target
      // const textLen = target.textContent.length
      // selection.importSelection({
      //   start: 0,
      //   end: textLen
      // }, target)
    }
  }
}

export default selectionCtrl


Review comment (suggestion):
@fxha You need to set the `this.cursor` firstly.

```
this.cursor = {
    start: { key: start.key, offset: 0 },
    end: { key: start.key, offset: block.text.length}
}
// then after other codes
this.partialRender()
```

code above just a example. it maybe help you? If you have any other questions about `editor-core`, I am very happy to help you！

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) {
          console.error('No table found or invalid type.')
          return
        } else if (startTable.key !== endTable.key) {
          // Select entire document
          return
        }
        const firstTableCell = this.firstInDescendant(startTable)
Repository: marktext/marktext
File: src/editor/contentState/selectionCtrl.js (JavaScript)

Code before:
import selection from '../selection'

const selectionCtrl = ContentState => {
  ContentState.prototype.selectionHandler = function (event) {
    const { start } = this.cursor
    const block = this.getBlock(start.key)
    if (/th|td/.test(block.type)) {
      event.preventDefault()
      console.log('table detected!') // TODO: #DEBUG

      // @Jocs Selects nothing...
      selection.setCursorRange({
        start: start,
        end: start
      })

      // @Jocs Selects full text...
      // const target = event.target
      // const textLen = target.textContent.length
      // selection.importSelection({
      //   start: 0,
      //   end: textLen
      // }, target)
    }
  }
}

export default selectionCtrl


Review comment (suggestion):
We should not use `setCursorRange` directly, `render` or `partialRender` will help us to render the cursor. all we need to do is set the cursor by `this.cursor = newCursor`.

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) {
          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)