Skip to content

Add context menu functionality. AI-assisted, tested manually. #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
roleohibachi wants to merge 1 commit into from
Closed

Add context menu functionality. AI-assisted, tested manually. #1

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
86 changes: 86 additions & 0 deletions main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,92 @@ export default class MarkitdownPlugin extends Plugin {
}
});


// Register file menu event handler for context menu
this.registerEvent(
Comment on lines +91 to +92
Copy link

@greptile-apps greptile-apps bot Jan 18, 2026

Choose a reason for hiding this comment

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

style: inconsistent indentation - file uses tabs but these lines use spaces

Suggested change
// Register file menu event handler for context menu
this.registerEvent(
// Register file menu event handler for context menu
this.registerEvent(

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI 
This is a comment left during a code review.
Path: main.ts
Line: 91:92

Comment:
**style:** inconsistent indentation - file uses tabs but these lines use spaces

```suggestion
		
		// Register file menu event handler for context menu
		this.registerEvent(
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

this.app.workspace.on('file-menu', (menu, file) => {
// Only show for files, not folders
if (!(file instanceof TFile)) return;

// Get file extension
const extension = file.extension.toLowerCase();

// List of supported extensions
const supportedExtensions = ['pdf', 'docx', 'pptx', 'xlsx', 'xls',
'html', 'htm', 'txt', 'csv', 'json', 'xml',
'jpg', 'jpeg', 'png', 'gif', 'wav', 'mp3', 'zip'];
Comment on lines +101 to +103
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

The list of supported extensions is duplicated here and in the file input accept attribute at line 314. Consider extracting this to a constant at the class level (e.g., SUPPORTED_EXTENSIONS) to maintain a single source of truth and make future updates easier.

Copilot uses AI. Check for mistakes.

// Only show for supported file types
if (!supportedExtensions.includes(extension)) return;

menu.addItem((item) => {
item
.setTitle('Convert to Markdown with Markitdown')
.setIcon('file-text')
.onClick(async () => {
if (!this.markitdownInstalled) {
new MarkitdownSetupModal(this.app, this).open();
return;
}

try {
// Get vault path
let vaultPath = '';
if (this.app.vault.adapter instanceof FileSystemAdapter) {
vaultPath = this.app.vault.adapter.getBasePath();
}

if (!vaultPath) {
new Notice('Could not determine vault path. This plugin requires a local vault.');
return;
}

// Get file path
const filePath = path.join(vaultPath, file.path);

// Determine output path
let outputFolder = this.settings.outputPath || '';
if (!outputFolder) {
outputFolder = path.join(vaultPath, 'markitdown-output');
if (!fs.existsSync(outputFolder)) {
fs.mkdirSync(outputFolder, { recursive: true });
}
} else {
outputFolder = path.join(vaultPath, outputFolder);
if (!fs.existsSync(outputFolder)) {
fs.mkdirSync(outputFolder, { recursive: true });
}
}

// Create output filename
const baseName = path.basename(file.path, path.extname(file.path));
const outputPath = path.join(outputFolder, `${baseName}.md`);

Comment on lines +147 to +150
Copy link

@coderabbitai coderabbitai bot Jan 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid overwriting conversions for same base filename.

Line 148 builds outputPath solely from the basename, so two files with the same name in different folders will overwrite each other in the output folder. Preserve relative directories (or add a suffix) to avoid silent collisions.

🔧 Proposed fix (preserve relative folder structure) 
-								  const baseName = path.basename(file.path, path.extname(file.path));
-								  const outputPath = path.join(outputFolder, `${baseName}.md`);
+								  const baseName = path.basename(file.path, path.extname(file.path));
+								  const relDir = path.dirname(file.path);
+								  const outputDir = path.join(outputFolder, relDir);
+								  if (!fs.existsSync(outputDir)) {
+										fs.mkdirSync(outputDir, { recursive: true });
+								  }
+								  const outputPath = path.join(outputDir, `${baseName}.md`);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Create output filename
const baseName = path.basename(file.path, path.extname(file.path));
const outputPath = path.join(outputFolder, `${baseName}.md`);
// Create output filename
const baseName = path.basename(file.path, path.extname(file.path));
const relDir = path.dirname(file.path);
const outputDir = path.join(outputFolder, relDir);
if (!fs.existsSync(outputDir)) {
fs.mkdirSync(outputDir, { recursive: true });
}
const outputPath = path.join(outputDir, `${baseName}.md`);
🤖 Prompt for AI Agents 
In `@main.ts` around lines 147 - 150, The outputPath construction uses only the
basename (baseName) which causes collisions when different source files share
the same name; change the logic in main.ts around baseName/outputPath to
preserve the source file's relative directory (e.g., compute a relativePath from
the input root using file.path and include it under outputFolder) or, if
preserving structure isn't desired, append a unique suffix (timestamp or
increment) to baseName before joining with outputFolder; update any creation of
directories accordingly so the directory tree exists before writing.

new Notice('Converting file...');

// Convert the file
await this.convertFile(filePath, outputPath);
Copy link

@coderabbitai coderabbitai bot Jan 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Potential command injection vulnerability in file paths.

The convertFile method (lines 227-255) passes filePath directly into shell commands using string interpolation. If a filename contains shell metacharacters (e.g., $(cmd), backticks, or semicolons), arbitrary commands could be executed.

🔒 Recommended mitigation

Use spawn with an argument array instead of exec with string interpolation to avoid shell interpretation:

// In convertFile method, replace exec with spawn
import { spawn } from 'child_process';

async convertFile(filePath: string, outputPath: string): Promise<string> {
    return new Promise((resolve, reject) => {
        const outputStream = fs.createWriteStream(outputPath);
        const proc = spawn('markitdown', [filePath], { shell: false });
        
        proc.stdout.pipe(outputStream);
        proc.stderr.on('data', (data) => { /* handle stderr */ });
        proc.on('close', (code) => {
            if (code === 0) resolve('');
            else reject(new Error(`markitdown exited with code ${code}`));
        });
    });
}

Alternatively, validate/sanitize file paths before use.

🤖 Prompt for AI Agents 
In `@main.ts` at line 154, The convertFile implementation currently interpolates
filePath into a shell command (using exec) which allows command injection;
update convertFile to avoid shell interpretation by using child_process.spawn
(or spawnSync) with an argument array and shell: false, passing filePath and any
flags as separate arguments, pipe spawn.stdout into the output stream for
outputPath, handle spawn.stderr and exit code to resolve/reject the Promise, and
remove any use of exec or string interpolation of filePath; alternatively,
validate/sanitize filePath before use to ensure it contains no metacharacters
(refer to convertFile, filePath, outputPath, and the external tool name
markitdown in your changes).


// Refresh the vault to see the new file
await this.app.vault.adapter.exists(outputPath);

new Notice(`File converted and saved to ${outputPath}`);
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

The error message displays the full system file path which may expose sensitive information about the user's file system structure. Consider using only the relative path within the vault (as computed on line 162) for the success message to be consistent with Obsidian's typical UX and avoid exposing absolute paths.

Suggested change
new Notice(`File converted and saved to ${outputPath}`);
new Notice(`File converted and saved to ${relativePath}`);

Copilot uses AI. Check for mistakes.

// Try to open the converted file
const relativePath = path.relative(vaultPath, outputPath).replace(/\\/g, '/');
const existingFile = this.app.vault.getAbstractFileByPath(relativePath);
if (existingFile instanceof TFile) {
this.app.workspace.getLeaf().openFile(existingFile);
}
} catch (error) {
console.error('Error during conversion:', error);
new Notice(`Error: ${error.message}`);
Comment on lines +167 to +169
Copy link

@coderabbitai coderabbitai bot Jan 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Handle unknown error type safely.

In TypeScript strict mode, caught errors are typed as unknown. Accessing error.message directly may fail if the error is not an Error instance.

Suggested fix 
 							 } catch (error) {
 								  console.error('Error during conversion:', error);
-								  new Notice(`Error: ${error.message}`);
+								  const message = error instanceof Error ? error.message : String(error);
+								  new Notice(`Error: ${message}`);
 							 }

This pattern should also be applied at lines 391 and 598.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} catch (error) {
console.error('Error during conversion:', error);
new Notice(`Error: ${error.message}`);
} catch (error) {
console.error('Error during conversion:', error);
const message = error instanceof Error ? error.message : String(error);
new Notice(`Error: ${message}`);
}
🤖 Prompt for AI Agents 
In `@main.ts` around lines 167 - 169, The catch blocks that log errors and create
Notices (e.g., the one with console.error('Error during conversion:', error) and
new Notice(`Error: ${error.message}`)) assume caught values are Error instances;
change them to safely extract a message by using a type guard/normalizer (e.g.,
const msg = error instanceof Error ? error.message : String(error)) and use msg
for both console.error and new Notice; apply the same change to the other
similar catch blocks mentioned (the ones that currently access error.message at
the other two locations).

}
Comment on lines +118 to +170
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

This conversion logic (lines 118-170) largely duplicates the logic in MarkitdownFileModal (lines 329-388). Consider extracting the shared conversion logic into a reusable method on the plugin class to reduce duplication and improve maintainability. For example, create a method like async performConversion(file: TFile, vaultPath: string) that handles the output folder creation, path resolution, conversion, and file opening logic.

Copilot uses AI. Check for mistakes.
});
});
})
);
Comment on lines +91 to +174
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

Inconsistent indentation: this section uses spaces for indentation while the rest of the file appears to use tabs (based on the surrounding code at lines 65-88). This creates a mixed indentation style. Consider using tabs to match the existing code style.

Copilot uses AI. Check for mistakes.

// Add settings tab
this.addSettingTab(new MarkitdownSettingTab(this.app, this));
}
Expand Down
Loading