Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 0 additions & 77 deletions src/render/components/Tools/GitCheatsheet/git-memo.content.md

This file was deleted.

247 changes: 239 additions & 8 deletions src/render/components/Tools/GitCheatsheet/index.vue
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
<script setup lang="ts">
import { I18nT } from '@lang/index'
import Memo from './git-memo.content.md?raw'
import markdownit from 'markdown-it'
const md = markdownit()
const result = md.render(Memo)
</script>
<template>
<div class="host-edit tools">
<div class="nav p-0">
Expand All @@ -16,8 +9,246 @@

<div class="pb-0 overflow-hidden flex-1">
<el-scrollbar>
<article class="select-text prose prose-slate dark:prose-invert" v-html="result"></article>
<article
ref="contentRef"
class="select-text prose prose-slate dark:prose-invert"
v-html="result"
></article>
</el-scrollbar>
</div>
</div>
</template>
<script setup lang="ts">
import { computed, h, onMounted, onUnmounted, render, ref, markRaw } from 'vue'
import { AppI18n, I18nT } from '@lang/index'
import MemoEn from './lang/git-memo.en.md?raw'
import MemoVi from './lang/git-memo.vi.md?raw'
import markdownit from 'markdown-it'
import { MessageSuccess } from '@/util/Element'
import { clipboard } from '@/util/NodeFn'
import { ElButton } from 'element-plus'
import { CopyDocument, Check } from '@element-plus/icons-vue'
import { createHighlighter, type Highlighter } from 'shiki'

const i18n = AppI18n()
const highlighter = ref<Highlighter | null>(null)

let observer: MutationObserver | null = null
const resetTimers = new Map<Element, ReturnType<typeof setTimeout>>()

onMounted(async () => {
highlighter.value = await createHighlighter({
themes: ['github-dark'],
langs: ['bash', 'shell']
})

if (contentRef.value) {
observer = new MutationObserver(() => {
mountButtons()
})
observer.observe(contentRef.value, { childList: true, subtree: true })
}
mountButtons()
})

onUnmounted(() => {
observer?.disconnect()
resetTimers.forEach((id) => clearTimeout(id))
resetTimers.clear()
})
const md = markdownit({
html: false,
linkify: true,
typographer: true
})
Comment on lines +59 to +63
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

The markdown-it instance is configured with html: true, which allows raw HTML to be rendered from markdown content. This content is then rendered into the DOM using the v-html directive on line 12. While the current markdown sources are local files, this configuration introduces a security risk if the files are ever updated with untrusted content or if the component is modified to accept external input. In an Electron environment, XSS can have a significant impact. It is recommended to set html: false unless raw HTML support is explicitly required for the cheatsheet content.

  const md = markdownit({
    html: false,
    linkify: true,
    typographer: true
  })

Comment on lines +59 to +63
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

Enabling html: true in markdown-it allows raw HTML in the markdown content to be rendered. This is a security risk when combined with v-html and also causes functional issues where text enclosed in angle brackets (like <type>) is treated as an HTML tag and hidden by the browser. It is recommended to set this to false unless raw HTML support is explicitly required.

  const md = markdownit({
    html: false,
    linkify: true,
    typographer: true
  })


md.renderer.rules.fence = function (tokens, idx) {
const token = tokens[idx]
const code = token.content.trim()
const lines = code.split('\n')

const renderedLines = lines
.map((line) => {
const trimmed = line.trim()
if (trimmed.startsWith('#') || !trimmed) {
return `<div class="code-line comment"><span class="line-content">${md.utils.escapeHtml(line)}</span></div>`
}

let highlighted = md.utils.escapeHtml(line)
if (highlighter.value) {
try {
const html = highlighter.value.codeToHtml(line, {
lang: 'bash',
theme: 'github-dark'
})
// Extract the core content from shiki's pre/code wrapper
const match = html.match(/<code[^>]*>([\s\S]*?)<\/code>/)
if (match) {
highlighted = match[1]
}
} catch (e) {
console.error('Shiki highlighting failed:', e)
}
}

// Return a single line string to prevent pre-tag whitespace issues
return `<div class="code-line command"><span class="line-content">${highlighted}</span><div class="copy-btn-placeholder" data-code="${encodeURIComponent(trimmed)}"></div></div>`
})
.join('')

return `<div class="code-block-wrapper"><pre><code>${renderedLines}</code></pre></div>`
}

const memos = {
en: MemoEn,
vi: MemoVi
}

const memo = computed(() => {
// Ensure i18n.global.locale is reactive. For example, if it's a ref, use i18n.global.locale.value
const locale = i18n.global.locale as keyof typeof memos
return memos[locale] ?? MemoEn
})
Comment on lines +107 to +111
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The memo computed property has two issues:

  1. Reactivity Bug: It depends on i18n.global.locale, which is not a reactive source in your current setup. This means when the app's language is changed, the cheatsheet content will not update. This is a significant bug for the multi-language feature. You should use a reactive source for the locale.
  2. Scalability: The if/else structure for selecting the language is not easily scalable. If you add more languages, you'll need to modify this logic every time.

I suggest refactoring this to use a map for scalability and ensuring you use a reactive locale.

  const memos = {
    en: MemoEn,
    vi: MemoVi
  }

  const memo = computed(() => {
    // Ensure i18n.global.locale is reactive. For example, if it's a ref, use i18n.global.locale.value
    const locale = i18n.global.locale as keyof typeof memos
    return memos[locale] ?? MemoEn
  })

Comment on lines +107 to +111
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The memo computed property depends on i18n.global.locale. With the legacy vue-i18n API being used, i18n.global.locale is a simple string property and not reactive. This means when the application's language is changed, this component will not update to display the cheatsheet in the new language. The code comment on line 104 suggests awareness of this potential issue.

To fix this, you need to make the computed property depend on a reactive source for the locale. For example, if you have a global reactive state for the current language (e.g., in a Pinia store), you should use that.

Example with a reactive store:

// Example with a reactive store
import { useLanguageStore } from '@/stores/language'

const languageStore = useLanguageStore()

const memo = computed(() => {
  const locale = languageStore.locale as keyof typeof memos
  return memos[locale] ?? MemoEn
})


const result = computed(() => {
return highlighter.value ? md.render(memo.value) : ''
})
Comment on lines +113 to +115
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The result computed property is calculated before the shiki highlighter is initialized asynchronously in onMounted. Since result does not depend on highlighter, it is not re-evaluated when the highlighter becomes available. This causes code blocks to not be syntax-highlighted on initial load. They will only be highlighted if the locale changes, which forces a re-render.

  const result = computed(() => {
    return highlighter.value ? md.render(memo.value) : ''
  })


const contentRef = ref<HTMLElement | null>(null)

const copy = (v: string) => {
clipboard.writeText(v)
MessageSuccess(md.utils.escapeHtml(I18nT('base.copySuccess')))
}

const mountButtons = () => {
const el = contentRef.value
if (!el) return
const placeholders = el.querySelectorAll('.copy-btn-placeholder')
placeholders.forEach((p) => {
if (p.children.length > 0) return
p.innerHTML = ''
const code = decodeURIComponent(p.getAttribute('data-code') || '')
const btnIcon = ref(markRaw(CopyDocument))

const renderBtn = () => {
const vnode = h(
ElButton,
{
link: true,
icon: btnIcon.value,
onClick: () => {
copy(code)
btnIcon.value = markRaw(Check)
renderBtn()
Comment on lines +134 to +143
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Dynamically mounted button components using render() are not unmounted when the locale changes, causing a memory leak as old component instances accumulate with each switch.
Severity: MEDIUM

Suggested Fix

Implement cleanup logic for the dynamically rendered components. Before the v-html content is replaced, or within the onUnmounted hook, iterate through the mounted components and explicitly unmount them by calling render(null, container) for each one. This will ensure their memory is properly released.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/render/components/Tools/GitCheatsheet/index.vue#L134-L143

Potential issue: The `mountButtons` function uses Vue's `render()` to dynamically mount
button components into placeholders. When the locale changes, the `v-html` directive
replaces the entire content, removing the old placeholder elements from the DOM.
However, the components rendered into those placeholders are not explicitly unmounted.
This results in orphaned component instances, refs, and event listeners remaining in
memory. Each language switch accumulates more orphaned components, leading to a gradual
memory leak that can degrade application performance over time for users who frequently
change languages.

Did we get this right? 👍 / 👎 to inform future reviews.

const existing = resetTimers.get(p)
if (existing) clearTimeout(existing)
const timeoutId = setTimeout(() => {
btnIcon.value = markRaw(CopyDocument)
renderBtn()
resetTimers.delete(p)
}, 2000)
Comment on lines +140 to +150
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Keep one icon-reset timer per button and clear on unmount.

Rapid clicks queue multiple timers; older timers can reset the icon too early. Also, timers are not cleared on unmount.

💡 Suggested fix
-  let observer: MutationObserver | null = null
+  let observer: MutationObserver | null = null
+  const resetTimers = new Map<Element, ReturnType<typeof setTimeout>>()

   onMounted(async () => {
@@
   onUnmounted(() => {
     observer?.disconnect()
+    resetTimers.forEach((id) => clearTimeout(id))
+    resetTimers.clear()
   })
@@
             onClick: () => {
               copy(code)
               btnIcon.value = markRaw(Check)
               renderBtn()
-              setTimeout(() => {
+              const existing = resetTimers.get(p)
+              if (existing) clearTimeout(existing)
+              const timeoutId = setTimeout(() => {
                 btnIcon.value = markRaw(CopyDocument)
                 renderBtn()
+                resetTimers.delete(p)
               }, 2000)
+              resetTimers.set(p, timeoutId)
             }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/render/components/Tools/GitCheatsheet/index.vue` around lines 136 - 143,
The click handler currently spawns a new setTimeout each click and never clears
timers on unmount; modify the onClick logic to keep a single timer id (e.g., a
local ref or per-button Map) and before creating a new setTimeout call
clearTimeout(existingTimerId), then store the new timer id; also add an
onBeforeUnmount hook that clears the stored timer(s) with clearTimeout to avoid
orphaned timers. Update references to btnIcon, markRaw(Check),
markRaw(CopyDocument), and renderBtn to remain the same—only change how the
timer is managed and cleaned up.

resetTimers.set(p, timeoutId)
}
},
() => []
)
render(vnode, p)
}
renderBtn()
})
}
</script>

<style lang="scss">
.code-block-wrapper {
position: relative;
margin-bottom: 1.5rem;
background: #1e293b;
border-radius: 8px;
overflow: hidden;

pre {
margin: 0 !important;
padding: 0.5rem 1rem !important; // Reduced vertical padding
background: transparent !important;
}

code {
display: block;
padding: 0 !important;
}

.code-line {
display: flex;
align-items: center;
justify-content: space-between;
min-height: 1.2rem;
padding: 0 0.5rem;
margin: 0 !important;
border-radius: 4px;
line-height: 1; // Strict line height
transition: background 0.2s;

&:hover {
background: rgba(255, 255, 255, 0.05);
}

&.comment {
color: #64748b;
font-style: italic;
}

.line-content {
flex: 1;
white-space: pre-wrap;
word-break: break-all;
font-family:
ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, 'Liberation Mono', 'Courier New',
monospace;
font-size: 0.9rem;
line-height: 1.4;
}
}

.copy-btn-placeholder {
flex-shrink: 0;
display: flex;
align-items: center;
justify-content: center;
margin-left: 1rem;
min-width: 32px; // Ensure space even before mount
min-height: 24px;
opacity: 0.8;
transition: all 0.2s ease-in-out;

&:hover {
opacity: 1;
}

.el-button {
padding: 4px;
height: auto;
color: #94a3b8;

&:hover {
color: #409eff;
}
}
}

.code-line:hover .copy-btn-placeholder {
opacity: 1;
}
}

.dark {
.code-block-wrapper {
background: #0f172a;

.code-line:hover {
background: rgba(255, 255, 255, 0.03);
}
}
}
</style>
Loading