Skip to content
Merged
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
6 changes: 3 additions & 3 deletions src/runtime/api/BunObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,7 @@ fn open_in_editor(global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResu
// `edit.name` is observed (single-threaded JS VM).
edit.name = unsafe { bun_ptr::detach_lifetime(slot.name_storage.as_slice()) };
edit.detect_editor(env);
editor_choice = edit.editor;
editor_choice = edit.found();
if editor_choice.is_none() {
slot.name_storage = prev_storage;
*edit = prev;
Expand All @@ -1017,11 +1017,11 @@ fn open_in_editor(global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResu
}
}

let editor = match editor_choice.or(edit.editor) {
let editor = match editor_choice.or_else(|| edit.found()) {
Some(e) => e,
None => {
edit.auto_detect_editor(env);
match edit.editor {
match edit.found() {
Some(e) => e,
None => {
return Err(global_this.throw(format_args!("Failed to auto-detect editor")));
Expand Down
6 changes: 6 additions & 0 deletions src/runtime/cli/open.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,12 @@ impl Default for EditorContext {
}

impl EditorContext {
/// `detect_editor` records `Editor::None` when nothing was found so the
/// search is not repeated; to callers that means "no editor".
pub(crate) fn found(&self) -> Option<Editor> {
self.editor.filter(|e| *e != Editor::None)
}

pub(crate) fn auto_detect_editor(&mut self, env: &mut dot_env::Loader) {
if self.editor.is_none() {
self.detect_editor(env);
Expand Down
11 changes: 8 additions & 3 deletions test/js/bun/util/open-in-editor-gc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ test.skipIf(!isLinux)("Bun.openInEditor survives re-entrant calls from option ge
{ get column() { reenter(); return "2"; } },
];
for (const opts of variants) {
try { Bun.openInEditor("/nonexistent/f.txt", opts); } catch {}
console.log("ok");
try { Bun.openInEditor("/nonexistent/f.txt", opts); console.log("opened"); } catch (e) { console.log(e.message); }
}
`,
});
Expand All @@ -46,7 +45,13 @@ test.skipIf(!isLinux)("Bun.openInEditor survives re-entrant calls from option ge
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stderr).toBe("");
expect(stdout.trim().split("\n")).toEqual(["ok", "ok", "ok", "ok"]);
// Nothing is found, so every call must throw rather than spawn anything.
expect(stdout.trim().split("\n")).toEqual([
'Could not find editor "zzz_no_editor"',
'Could not find editor "zzz_no_editor"',
"Failed to auto-detect editor",
"Failed to auto-detect editor",
]);
expect(proc.signalCode).toBeNull();
expect(exitCode).toBe(0);
});
Expand Down