Bug: Editor Opener doesn't work on vim

[AndreCostaaa]

This is because it waits for the process to exit.

This was done with vscode and intellij in mind but it doesn't work with vim or nano

To simplify, I think we can assume that if the process launches, we can assume the editor was opened

What do you think @samuelroland ?

[AndreCostaaa]

Reproducible example:

Add the following test to opener.rs

    #[test]
    fn open_vim() {
        let (tx, rx) = mpsc::channel();
        let should_stop = Arc::new(AtomicBool::new(false));
        let worker = EditorOpener::new(String::from("vi"), ".opener.rs".into());
        worker.run(tx.clone(), should_stop.clone());
        assert_eq!(rx.recv().unwrap(), Event::EditorOpened);
    }

and run cargo test

Result:

Terminal will stay blocked

[samuelroland]

Thanks for raising this bug. There are 3 cases to consider IMO

1. Opening the editor to start the exo (we want to have a separated OS window or terminal window)
2. Opening an other file in the existing editor window (vscode and idea CLI seem to reuse the existing window IRRC)
3. Opening an exo metadata for quick edition by pressing e (for teachers) -> in this case, blocking PLX and opening Vim is what I would expect (like in Gitui e). When quitting the editor, PLX would parse the exo again.

Sooo, as some GUI editors are not blocking, it already works for 1 and 2, but not for 3 (we don't know when the edition is done as we don't know when the GUI is closed).

All TUI editors (nano/vi/vim/nvim/...) will block I guess so it's fine for 3, but for 1 we need a way to either launch a new terminal window first or for 1-2 reuse an existing instance...

I tested to open nvim in a separated window with nohup kitty nvim README.md &, it works.

That's not a trivial issue...

A few useful links

• Vi is not built with a clientserver by default I guess (at least on Fedora build) https://vi.stackexchange.com/questions/317/can-i-open-a-file-in-an-existing-vim-instance-from-an-external-command
• A possible solution for Neovim to open a file in existing window, final command would be like nvr --remote $FILE instead of nvim $FILE: https://github.com/mhinz/neovim-remote

[AndreCostaaa]

A quick fix could be an extra argument that tells the worker when it should consider the editor open

impl EditorOpener {
    pub fn run(&self, tx: Sender<Event>, should_stop: Arc<AtomicBool>) {
        let child = process_handler::spawn_process(
            &self.editor,
            vec![self.file_path.display().to_string()],
        );
>       if (!self.wait_for_child_to_quit) {
>           tx.send(Event::EditorOpened);
>           return;
>       }
        let _ = match child {
            Ok(mut child) => match wait_child(&mut child, should_stop.clone()) {
                Ok(_) => tx.send(Event::EditorOpened),
                Err(_) => tx.send(Event::CouldNotOpenEditor),
            },
            Err(_) => tx.send(Event::CouldNotOpenEditor),
        };
    }

Ultimately we would have to define whose responsibility it is to know when to stop