Skip to content

Conversation

@harehare
Copy link
Owner

@harehare harehare commented Jan 1, 2026

Adds a new /edit command to the mq REPL that allows users to edit mq code in their preferred external editor ($EDITOR or $VISUAL environment variable, defaulting to vi). The command creates a temporary file, opens it in the editor, and evaluates the edited content when the editor closes. Also adds Alt+O keybinding for quick access.

Adds a new /edit command to the mq REPL that allows users to edit mq code in their preferred external editor ($EDITOR or $VISUAL environment variable, defaulting to vi). The command creates a temporary file, opens it in the editor, and evaluates the edited content when the editor closes. Also adds Alt+O keybinding for quick access.
Copilot AI review requested due to automatic review settings January 1, 2026 13:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a /edit command to the mq REPL, enabling users to edit mq code in their preferred external editor. The implementation respects the $EDITOR or $VISUAL environment variables, falling back to vi as default. Changes include command parsing, execution logic using temporary files, syntax highlighting updates, and an Alt+O keybinding for quick access.

  • Adds /edit command with external editor integration via temporary files
  • Implements Alt+O keybinding to invoke the edit command
  • Updates syntax highlighting to recognize the new command

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File Description
crates/mq-repl/src/command_context.rs Adds Edit command variant, execution logic with temporary file handling, and corresponding test cases
crates/mq-repl/src/repl.rs Updates syntax highlighting pattern and adds Alt+O keybinding for the edit command
crates/mq-repl/Cargo.toml Adds tempfile dependency for temporary file management

Copilot AI review requested due to automatic review settings January 1, 2026 23:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

let edited_content = fs::read_to_string(&temp_path).into_diagnostic()?;

// Clean up temp file
fs::remove_file(&temp_path).ok();
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

Silently ignoring cleanup errors with .ok() could hide issues. Consider logging a warning if cleanup fails, or use a comment explaining why failures are acceptable here.

Copilot generated this review using guidance from repository custom instructions.
pub fn help(&self) -> String {
match self {
Command::Copy => format!("{:<12}{}", "/copy", "Copy the execution results to the clipboard"),
Command::Edit => format!("{:<12}{}", "/edit", "Edit the current buffer in external editor"),
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The help text says 'Edit the current buffer' but the implementation starts with an empty file. Update the help text to accurately describe the behavior, such as 'Open external editor to write mq code' or similar.

Copilot generated this review using guidance from repository custom instructions.
@harehare harehare force-pushed the feat/repl-edit-command branch from 39eeb87 to fec52c4 Compare January 1, 2026 23:39
@harehare harehare force-pushed the feat/repl-edit-command branch from fec52c4 to 25bd783 Compare January 1, 2026 23:47
Copilot AI review requested due to automatic review settings January 1, 2026 23:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

Comment on lines +155 to +157
// Write current buffer to temp file (empty for now)
temp_file.write_all(b"").into_diagnostic()?;
temp_file.flush().into_diagnostic()?;
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The comment 'empty for now' suggests incomplete implementation. If the intention is to allow users to edit existing buffer content, this should populate the temporary file with current input or buffer state. Otherwise, clarify why starting with an empty file is the desired behavior.

Suggested change
// Write current buffer to temp file (empty for now)
temp_file.write_all(b"").into_diagnostic()?;
temp_file.flush().into_diagnostic()?;
// Start with an empty temp file; the user will provide code in the editor.

Copilot uses AI. Check for mistakes.
Comment on lines +177 to +178
// Clean up temp file
fs::remove_file(&temp_path).ok();
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The .ok() silently ignores any cleanup errors. Consider logging a warning if cleanup fails, as this could lead to accumulation of temporary files over time, especially if the REPL is used in long-running sessions.

Suggested change
// Clean up temp file
fs::remove_file(&temp_path).ok();
// Clean up temp file (best-effort; log on failure)
if let Err(err) = fs::remove_file(&temp_path) {
eprintln!(
"warning: failed to remove temporary file {}: {}",
temp_path.display(),
err
);
}

Copilot uses AI. Check for mistakes.
if code.is_empty() {
Ok(CommandOutput::None)
} else {
let eval_result = self.engine.eval(code, self.input.clone().into_iter()).map_err(|e| *e)?;
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

When evaluation fails, the error is propagated without additional context about the source (external editor). Consider wrapping the error with context like 'Failed to evaluate code from external editor' to help users understand where the error originated.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants