ronantakizawa/codereview-bench huggingface dataset:
Repository: slidevjs/slidev
File: packages/client/builtin/CodeBlockWrapper.vue (Vue)

Code before:
        line.classList.toggle('dishonored', !highlighted)
      })
      if (props.maxHeight) {
        const firstHighlightedEl = target.querySelector('.line.highlighted')
        if (firstHighlightedEl)
          firstHighlightedEl.scrollIntoView({ behavior: 'smooth', block: 'center' })
      }
    }
  })
})

const { copied, copy } = useClipboard()

function copyCode() {
  const code = el.value?.querySelector('.slidev-code')?.textContent
  if (code)
    copy(code)
}
</script>

<template>
  <div
    ref="el" class="slidev-code-wrapper relative group"
    :style="{
      'max-height': props.maxHeight,
      'overflow-y': props.maxHeight ? 'scroll' : 'hidden',
    }"
  >
    <slot />
    <button
      v-if="configs.codeCopy"
      class="slidev-code-copy absolute top-0 right-0 transition opacity-0 group-hover:opacity-20 hover:!opacity-100"
      :title="copied ? 'Copied' : 'Copy'" @click="copyCode()"
    >
      <ph-check-circle v-if="copied" class="p-2 w-8 h-8" />
      <ph-clipboard v-else class="p-2 w-8 h-8" />
    </button>
  </div>
</template>


Review comment (bug):
```suggestion
      'overflow-y': props.maxHeight ? 'scroll' : undefined,
```

Code after:
        line.classList.toggle('dishonored', !highlighted)
      })
      if (props.maxHeight) {
        const firstHighlightedEl = target.querySelector('.line.highlighted')
        if (firstHighlightedEl)
          firstHighlightedEl.scrollIntoView({ behavior: 'smooth', block: 'center' })
      }
    }
  })
})

const { copied, copy } = useClipboard()

function copyCode() {
  const code = el.value?.querySelector('.slidev-code')?.textContent
  if (code)
    copy(code)
}
</script>

<template>
  <div
    ref="el" class="slidev-code-wrapper relative group"
    :style="{
      'max-height': props.maxHeight,
      'overflow-y': props.maxHeight ? 'scroll' : undefined,
    }"
  >
    <slot />
    <button
      v-if="configs.codeCopy"
      class="slidev-code-copy absolute top-0 right-0 transition opacity-0 group-hover:opacity-20 hover:!opacity-100"
      :title="copied ? 'Copied' : 'Copy'" @click="copyCode()"
    >
      <ph-check-circle v-if="copied" class="p-2 w-8 h-8" />
      <ph-clipboard v-else class="p-2 w-8 h-8" />
    </button>
  </div>
</template>
Repository: slidevjs/slidev
File: packages/slidev/node/cli.ts (TypeScript)

Code before:
      choices: ['pdf', 'png', 'md'],
      describe: 'output format',
    })
    .option('timeout', {
      default: 30000,
      type: 'number',
      describe: 'timeout for rendering the print page',
    })
    .option('range', {
      type: 'string',
      describe: 'page ranges to export, for example "1,4-5,6"',
    })
    .option('dark', {
      default: false,
      type: 'boolean',
      describe: 'export as dark theme',
    })
    .option('with-clicks', {
      alias: 'c',
      default: false,
      type: 'boolean',
      describe: 'export pages for every clicks',
    })
    .option('executable-path', {
      type: 'string',
      describe: 'executable to override playwright bundled chrome',
    })
    .strict()
    .help(),
  async ({
    entry,
    theme,
    output,
    format,
    timeout,
    range,
    dark,
    'with-clicks': withClicks,
    'executable-path': executablePath,
  }) => {
    process.env.NODE_ENV = 'production'
    const { exportSlides } = await import('./export')
    const port = await findFreePort(12445)
    const options = await resolveOptions({ entry, theme }, 'build')
    output = output || options.data.config.exportFilename || `${path.basename(entry, '.md')}-export`
    const server = await createServer(
      options,
      {
        server: { port },
        clearScreen: false,
      },

Review comment (suggestion):
```suggestion
      describe: 'executable to override playwright bundled browser',
```

Code after:
      choices: ['pdf', 'png', 'md'],
      describe: 'output format',
    })
    .option('timeout', {
      default: 30000,
      type: 'number',
      describe: 'timeout for rendering the print page',
    })
    .option('range', {
      type: 'string',
      describe: 'page ranges to export, for example "1,4-5,6"',
    })
    .option('dark', {
      default: false,
      type: 'boolean',
      describe: 'export as dark theme',
    })
    .option('with-clicks', {
      alias: 'c',
      default: false,
      type: 'boolean',
      describe: 'export pages for every clicks',
    })
    .option('executable-path', {
      type: 'string',
      describe: 'executable to override playwright bundled browser',
    })
    .strict()
    .help(),
  async ({
    entry,
    theme,
    output,
    format,
    timeout,
    range,
    dark,
    'with-clicks': withClicks,
    'executable-path': executablePath,
  }) => {
    process.env.NODE_ENV = 'production'
    const { exportSlides } = await import('./export')
    const port = await findFreePort(12445)
    const options = await resolveOptions({ entry, theme }, 'build')
    output = output || options.data.config.exportFilename || `${path.basename(entry, '.md')}-export`
    const server = await createServer(
      options,
      {
        server: { port },
        clearScreen: false,
      },
Repository: slidevjs/slidev
File: packages/types/src/types.ts (TypeScript)

Code before:
export type SlidevThemeConfig = Record<string, string | number>

export interface SlidevFeatureFlags {
  katex: boolean
  monaco: boolean
  tweet: boolean
  mermaid: boolean
}

export interface SlidevMarkdown {
  slides: SlideInfo[]
  raw: string
  config: SlidevConfig
  features: SlidevFeatureFlags
  headmatter: Record<string, unknown>

  filepath?: string
  entries?: string[]
  themeMeta?: SlidevThemeMeta
}

/**
 * Preparser modes: listing the built-in ones but it is open to extensions.
 * Modes starting with `:` are pseudo-modes (usually keeping the cursor in place) so that extensions can handle them.
*/
export type SlidevPreparserMode = (
  'content' | 'frontmatter' | 'codeblock' |
  ':start' | ':content-or-frontmatter' | ':slice' | ':sliced' | ':end' |
  string)

export interface SlidevPreparserState {
  lines: string[]
  slides: SlideInfo[]
  i: number
  start: number
  mode: SlidevPreparserMode
  modeStack: SlidevPreparserMode[]
  frontmatterPrepend: string[]
  frontmatterAppend: string[]
  contentPrepend: string[]
  contentAppend: string[]
  ext: any
}

export interface SlidevPreparserExtension {
  handle?(state: SlidevPreparserState): boolean
  handlePseudoMode?(state: SlidevPreparserState): boolean
  disabled?: boolean
}

export type PreparserExtensionLoader = (addons: string[], filepath?: string) => Promise<SlidevPreparserExtension[]>

Review comment (performance):
I am not so sure about the complexity here. I would prefer to have simple string transformation for the entire markdown file and then probably for each slide. I am expecting:

```ts
export interface SlidevPreparserExtension {
  name: string,
  transformRawMarkdown?(content: string): string | undefined,
  transformSlide?(no: number, content: string, frontmatter: object): string | undefined
}
```

Code after:
export type SlidevThemeConfig = Record<string, string | number>

export interface SlidevFeatureFlags {
  katex: boolean
  monaco: boolean
  tweet: boolean
  mermaid: boolean
}

export interface SlidevMarkdown {
  slides: SlideInfo[]
  raw: string
  config: SlidevConfig
  features: SlidevFeatureFlags
  headmatter: Record<string, unknown>

  filepath?: string
  entries?: string[]
  themeMeta?: SlidevThemeMeta
}

export interface SlidevPreparserExtension {
  name: string
  transformRawLines?(lines: string[]): Promise<void>
  transformSlide?(content: string, frontmatter: any): Promise<string | undefined>
}

export type PreparserExtensionLoader = (addons: string[], filepath?: string) => Promise<SlidevPreparserExtension[]>

// internal type?
export type PreparserExtensionFromHeadmatter = (headmatter: any, exts: SlidevPreparserExtension[], filepath: string | undefined) => Promise<SlidevPreparserExtension[]>

export type RenderContext = 'slide' | 'overview' | 'presenter' | 'previewNext'
Repository: slidevjs/slidev
File: packages/slidev/node/plugins/markdown-it-shiki.ts (TypeScript)

Code before:
    ...options,
    themes,
    darkModeThemes: darkModeThemes
      ? {
          dark: getThemeName(darkModeThemes.dark),
          light: getThemeName(darkModeThemes.light),
        }
      : undefined,
  }
}

function trimEndNewLine(code: string) {
  return code.replace(/\n$/, '')
}

const MarkdownItShiki: MarkdownIt.PluginWithOptions<ShikiOptions> = (markdownit, options = {}) => {
  const _highlighter: ShikiHighlighter = options!.highlighter!

  const { darkModeThemes } = resolveShikiOptions(options!)

  markdownit.options.highlight = (code, lang) => {
    if (darkModeThemes) {
      const trimmed = trimEndNewLine(code)
      const dark = _highlighter
        .codeToHtml(trimmed, { lang: lang || 'text', theme: darkModeThemes.dark })
        .replace('<pre class="shiki"', '<pre class="slidev-code shiki shiki-dark"')
      const light = _highlighter
        .codeToHtml(trimmed, { lang: lang || 'text', theme: darkModeThemes.light })
        .replace('<pre class="shiki"', '<pre class="slidev-code shiki shiki-light"')
      return escapeVueInCode(`<pre class="shiki-container">${dark}${light}</pre>`)
    }
    else {
      return escapeVueInCode(
        _highlighter.codeToHtml(code, { lang: lang || 'text' })
          .replace('<pre class="shiki"', '<pre class="slidev-code shiki"'),
      )
    }
  }
}

export default MarkdownItShiki


Review comment (suggestion):
I'm not sure about what comes after class="shiki in the code produced by the highlighter but it might be "safer" to swap shiki-dark and shiki in the replacement so what was coming after shiki still comes after shiki (and not shiki-dark) `class="slidev-code shiki-dark shiki'`.
This most probably won't change anything though.

Code after:
    ...options,
    themes,
    darkModeThemes: darkModeThemes
      ? {
          dark: getThemeName(darkModeThemes.dark),
          light: getThemeName(darkModeThemes.light),
        }
      : undefined,
  }
}

function trimEndNewLine(code: string) {
  return code.replace(/\n$/, '')
}

const MarkdownItShiki: MarkdownIt.PluginWithOptions<ShikiOptions> = (markdownit, options = {}) => {
  const _highlighter: ShikiHighlighter = options!.highlighter!

  const { darkModeThemes } = resolveShikiOptions(options!)

  markdownit.options.highlight = (code, lang) => {
    if (darkModeThemes) {
      const trimmed = trimEndNewLine(code)
      const dark = _highlighter
        .codeToHtml(trimmed, { lang: lang || 'text', theme: darkModeThemes.dark })
        .replace('<pre class="shiki', '<pre class="slidev-code shiki shiki-dark')
      const light = _highlighter
        .codeToHtml(trimmed, { lang: lang || 'text', theme: darkModeThemes.light })
        .replace('<pre class="shiki', '<pre class="slidev-code shiki shiki-light')
      return escapeVueInCode(`<pre class="shiki-container">${dark}${light}</pre>`)
    }
    else {
      return escapeVueInCode(
        _highlighter.codeToHtml(code, { lang: lang || 'text' })
          .replace('<pre class="shiki"', '<pre class="slidev-code shiki"'),
      )
    }
  }
}

export default MarkdownItShiki
Repository: slidevjs/slidev
File: packages/client/internals/Goto.vue (Vue)

Code before:
      <input
        ref="input"
        :value="text"
        type="text"
        :disabled="!showGotoDialog"
        class="outline-none bg-transparent"
        placeholder="Goto..."
        :class="{ 'text-red-400': !valid && text }"
        @keydown.enter="goTo"
        @keydown.escape="close"
        @keydown.down="focusDown"
        @keydown.up="focusUp"
        @input="updateText"
      >
    </div>
    <ul
      v-if="autocomplete.length > 0"
      ref="list"
      class="autocomplete-list"
      shadow="~"
      p="x-4 y-2"
      border="~ transparent rounded dark:gray-400 dark:opacity-25"
    >
      <li
        v-for="(item, index) of autocomplete"
        ref="items"
        :key="item"
        class="autocomplete"
        :class="{ selected: selectedItem === index }"
        role="button"
        tabindex="0"
        @click.stop="select(item)"
      >
        {{ item }}
      </li>
    </ul>
  </div>
</template>

<style scoped lang="postcss">
.autocomplete-list {
  @apply bg-main transform mt-1 overflow-auto;
  max-height: calc( 100vh - 100px );
}

.autocomplete {
  cursor: pointer;

  &:hover {
    text-decoration: underline
  }

Review comment (suggestion):
Having auto-complete is a great idea. And since we are inferring the title for each slide, maybe we could display them and even do a fuzzy search based on the title as well?

<img width="223" alt="image" src="https://user-images.githubusercontent.com/11247099/227497293-8e5e9972-b09c-4a92-9c27-d569ea79a626.png">


Code after:
        :value="text"
        type="text"
        :disabled="!showGotoDialog"
        class="outline-none bg-transparent"
        placeholder="Goto..."
        :class="{ 'text-red-400': !valid && text }"
        @keydown.enter="goTo"
        @keydown.escape="close"
        @keydown.down="focusDown"
        @keydown.up="focusUp"
        @input="updateText"
      >
    </div>
    <ul
      v-if="result.length > 0"
      ref="list"
      class="autocomplete-list"
      shadow="~"
      border="~ transparent rounded dark:main"
    >
      <li
        v-for="(item, index) of result"
        ref="items"
        :key="item.id"
        role="button"
        tabindex="0"
        p="x-4 y-2"
        cursor-pointer
        hover="op100"
        flex="~ gap-2"
        items-center
        :border="index === 0 ? '' : 't main'"
        :class="selectedIndex === index ? 'bg-active op100' : 'op80'"
        @click.stop="select(item.no)"
      >
        <div w-4 text-right op50 text-sm>
          {{ item.no }}
        </div>
        {{ item.title }}
      </li>
    </ul>
  </div>
</template>

<style scoped lang="postcss">
.autocomplete-list {
  @apply bg-main transform mt-1 overflow-auto;
  max-height: calc( 100vh - 100px );
}

.autocomplete {
Repository: slidevjs/slidev
File: packages/client/builtin/RenderWhen.vue (Vue)

Code before:
  return anyContext && allConditions
})

function contextMatch(context: Context) {
  if (context === currentContext?.value)
    return true
  if (context === 'main' && (currentContext?.value === 'slide' || currentContext?.value === 'presenter'))
    return true
  if (context === 'visible')
    return true
  if (context === 'print' && isPrintMode.value)
    return true
  return false
}

function conditionsMatch(context: Context) {
  if (context === 'visible')
    return targetVisible.value
  return true
}
</script>

<template>
  <div v-if="needsDomWrapper" ref="target">
    <slot v-if="shouldRender" />
    <slot v-else name="else" />
  </div>
  <slot v-else-if="shouldRender" />
  <slot v-else name="else" />
</template>


Review comment (suggestion):
Maybe `fallback`? That align with Vue better: https://vuejs.org/guide/built-ins/suspense.html#loading-state

Code after:
  return anyContext && allConditions
})

function contextMatch(context: Context) {
  if (context === currentContext?.value)
    return true
  if (context === 'main' && (currentContext?.value === 'slide' || currentContext?.value === 'presenter'))
    return true
  if (context === 'visible')
    return true
  if (context === 'print' && isPrintMode.value)
    return true
  return false
}

function conditionsMatch(context: Context) {
  if (context === 'visible')
    return targetVisible.value
  return true
}
</script>

<template>
  <div v-if="needsDomWrapper" ref="target">
    <slot v-if="shouldRender" />
    <slot v-else name="fallback" />
  </div>
  <slot v-else-if="shouldRender" />
  <slot v-else name="fallback" />
</template>
Repository: slidevjs/slidev
File: packages/client/composables/useClicks.ts (TypeScript)

Code before:
import { clamp, sum } from '@antfu/utils'
import type { ClicksContext, SlideRoute } from '@slidev/types'
import type { Ref } from 'vue'
import { ref, shallowReactive } from 'vue'
import { normalizeAtProp } from '../logic/utils'
import { routeForceRefresh } from '../logic/route'

export function createClicksContextBase(
  current: Ref<number>,
  clicksStart: number,
  clicksTotalOverrides?: number,
): ClicksContext {
  const relativeOffsets: ClicksContext['relativeOffsets'] = new Map()
  const map: ClicksContext['map'] = shallowReactive(new Map())

  return {
    get current() {
      // Here we haven't know clicksTotal yet.
      return clamp(+current.value, clicksStart, this.total)
    },
    set current(value) {
      current.value = clamp(+value, clicksStart, this.total)
    },
    clicksStart,
    relativeOffsets,
    map,
    onMounted() { },
    resolve(at, size = 1) {
      const [isRelative, value] = normalizeAtProp(at)
      if (isRelative) {
        const offset = this.currentOffset
        return {
          start: offset + value,
          end: offset + value + size - 1,
          delta: value + size - 1,
        }
      }
      else {
        return {
          start: value,
          end: value + size - 1,
          delta: 0,
        }
      }

Review comment (suggestion):
I am not very sure if this is OK. Because `current` can be accessed at any time, but `this.total` is only available after the whole slide has been mounted (As mentioned in L18). The same to other changes in this commit.

Code after:
import { clamp, sum } from '@antfu/utils'
import type { ClicksContext, SlideRoute } from '@slidev/types'
import type { Ref } from 'vue'
import { ref, shallowReactive } from 'vue'
import { normalizeAtProp } from '../logic/utils'
import { routeForceRefresh } from '../logic/route'

export function createClicksContextBase(
  current: Ref<number>,
  clicksStart = 0,
  clicksTotalOverrides?: number,
): ClicksContext {
  const relativeOffsets: ClicksContext['relativeOffsets'] = new Map()
  const map: ClicksContext['map'] = shallowReactive(new Map())

  return {
    get current() {
      // Here we haven't know clicksTotal yet.
      return clamp(+current.value, clicksStart, this.total)
    },
    set current(value) {
      current.value = clamp(+value, clicksStart, this.total)
    },
    clicksStart,
    relativeOffsets,
    map,
    onMounted() { },
    resolve(at, size = 1) {
      const [isRelative, value] = normalizeAtProp(at)
      if (isRelative) {
        const offset = this.currentOffset
        return {
          start: offset + value,
          end: offset + value + size - 1,
          delta: value + size - 1,
        }
      }
      else {
        return {
          start: value,
          end: value + size - 1,
          delta: 0,
        }
      }
Repository: slidevjs/slidev
File: packages/slidev/node/virtual/titles.ts (TypeScript)

Code before:
import type { VirtualModuleTemplate } from './types'

export const templateTitleRendererMd: VirtualModuleTemplate = {
  id: '/@slidev/title-renderer.md',
  async getContent({ data }) {
    const lines = data.slides
      .map(({ title }, i) => `<template ${i === 0 ? 'v-if' : 'v-else-if'}="no === ${i + 1}">\n\n${title}\n\n</template>`)

    lines.push(
      [
        `<script setup lang="ts">`,
        `import { useSlideContext } from '@slidev/client'`,
        `import { computed } from 'vue'`,
        `const props = defineProps<{ no?: number | string }>()`,
        `const { $page } = useSlideContext()`,
        `const no = computed(() => +(props.no ?? $page.value))`,
        `</script>`,
      ].join('\n'),
    )

    return lines.join('\n')
  },
}

export const templateTitleRenderer: VirtualModuleTemplate = {
  id: '/@slidev/title-renderer',
  async getContent() {
    return 'export { default } from "/@slidev/title-renderer.md"'
  },
}


Review comment (suggestion):
```suggestion
    lines.push(
      `<script setup lang="ts">`,
      `import { useSlideContext } from '@slidev/client'`,
      `import { computed } from 'vue'`,
      `const props = defineProps<{ no?: number | string }>()`,
      `const { $page } = useSlideContext()`,
      `const no = computed(() => +(props.no ?? $page.value))`,
      `</script>`,
    )
```

Code after:
import type { VirtualModuleTemplate } from './types'

export const templateTitleRendererMd: VirtualModuleTemplate = {
  id: '/@slidev/title-renderer.md',
  async getContent({ data }) {
    const lines = data.slides
      .map(({ title }, i) => `<template ${i === 0 ? 'v-if' : 'v-else-if'}="no === ${i + 1}">\n\n${title}\n\n</template>`)

    lines.push(
      `<script setup lang="ts">`,
      `import { useSlideContext } from '@slidev/client'`,
      `import { computed } from 'vue'`,
      `const props = defineProps<{ no?: number | string }>()`,
      `const { $page } = useSlideContext()`,
      `const no = computed(() => +(props.no ?? $page.value))`,
      `</script>`,
    )

    return lines.join('\n')
  },
}

export const templateTitleRenderer: VirtualModuleTemplate = {
  id: '/@slidev/title-renderer',
  async getContent() {
    return 'export { default } from "/@slidev/title-renderer.md"'
  },
}
Repository: slidevjs/slidev
File: packages/types/src/types.ts (TypeScript)

Code before:

export interface SlidevMarkdown {
  filepath: string
  raw: string
  /**
   * All slides in this markdown file
   */
  slides: SourceSlideInfo[]
}

export interface SlidevData {
  /**
   * Slides that should be rendered (disabled slides excluded)
   */
  slides: SlideInfo[]
  entry: SlidevMarkdown
  config: SlidevConfig
  headmatter: Record<string, unknown>
  features: SlidevDetectedFeatures
  themeMeta?: SlidevThemeMeta
  markdownFiles: Record<string, SlidevMarkdown>
  watchFiles: string[]
}

export interface SlidevPreparserExtension {
  name?: string
  transformRawLines?: (lines: string[]) => Promise<void> | void
  transformSlide?: (content: string, frontmatter: any) => Promise<string | undefined>
}

export type PreparserExtensionLoader = (headmatter?: Record<string, unknown>, filepath?: string, mode?: string) => Promise<SlidevPreparserExtension[]>

export type RenderContext = 'none' | 'slide' | 'overview' | 'presenter' | 'previewNext'

export interface SlideRoute {
  no: number
  meta: RouteMeta & Required<Pick<RouteMeta, 'slide'>>
  component: () => Promise<{ default: RouteComponent }>
}

export type LoadedSnippets = Record<string, string>


Review comment (suggestion):
I wasn't sure about this one, but I think `name` is optional. At least there's no type error and the example doesn't use a `name` either.

Code after:

export interface SlidevMarkdown {
  filepath: string
  raw: string
  /**
   * All slides in this markdown file
   */
  slides: SourceSlideInfo[]
}

export interface SlidevData {
  /**
   * Slides that should be rendered (disabled slides excluded)
   */
  slides: SlideInfo[]
  entry: SlidevMarkdown
  config: SlidevConfig
  headmatter: Record<string, unknown>
  features: SlidevDetectedFeatures
  themeMeta?: SlidevThemeMeta
  markdownFiles: Record<string, SlidevMarkdown>
  watchFiles: string[]
}

export interface SlidevPreparserExtension {
  name?: string
  transformRawLines?: (lines: string[]) => Promise<void> | void
  transformSlide?: (content: string, frontmatter: any) => Promise<string | undefined>
}

export type PreparserExtensionLoader = (headmatter: Record<string, unknown>, filepath: string, mode?: string) => Promise<SlidevPreparserExtension[]>

export type RenderContext = 'none' | 'slide' | 'overview' | 'presenter' | 'previewNext'

export interface SlideRoute {
  no: number
  meta: RouteMeta & Required<Pick<RouteMeta, 'slide'>>
  component: () => Promise<{ default: RouteComponent }>
}

export type LoadedSnippets = Record<string, string>