Skip to content

Conversation

@mrubens
Copy link
Collaborator

@mrubens mrubens commented Aug 28, 2025

Important

Change default image filename to a more generic format in saveImage() function in image-handler.ts.

  • Behavior:
    • Change default image filename from mermaid_diagram_<timestamp> to img_<timestamp> in saveImage() function in image-handler.ts.

This description was created by Ellipsis for a68bb2a. You can customize this summary. It will automatically update as commits are pushed.

@mrubens mrubens requested review from cte and jr as code owners August 28, 2025 06:16
@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Aug 28, 2025
Copy link
Contributor

@roomote roomote bot left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! The change from mermaid_diagram_ to img_ makes perfect sense since the saveImage function is generic and handles all types of images, not just Mermaid diagrams. This also improves consistency with the openImage function which uses temp_image_ as a prefix.

A few additional observations about the module:

  • The module currently lacks test coverage. Consider adding unit tests for both openImage and saveImage functions to ensure they handle various scenarios correctly.
  • JSDoc documentation would improve code maintainability by explaining the purpose and parameters of these functions.

const workspacePath = getWorkspacePath()
const defaultPath = workspacePath || os.homedir()
const defaultFileName = `mermaid_diagram_${Date.now()}.${format}`
const defaultFileName = `img_${Date.now()}.${format}`
Copy link
Contributor

Choose a reason for hiding this comment

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

Good change! The generic img_ prefix is more appropriate since this function saves any type of image, not just Mermaid diagrams.

Have you considered making this prefix configurable? Something like:

Suggested change
const defaultFileName = `img_${Date.now()}.${format}`
const defaultFileName = `${options?.filenamePrefix || 'img'}_${Date.now()}.${format}`

This would allow callers to specify context-appropriate prefixes (e.g., "screenshot_", "diagram_") while maintaining "img_" as the default.

@mrubens mrubens merged commit 548d3b4 into main Aug 28, 2025
15 checks passed
@mrubens mrubens deleted the generic_default_image branch August 28, 2025 06:22
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Aug 28, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants