Skip to content
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

Jump to bottom

Vim hangs when opening from gitu on Windows #186

Open
McPhale opened this issue May 29, 2024 · 7 comments · May be fixed by #189
Open

Vim hangs when opening from gitu on Windows #186

McPhale opened this issue May 29, 2024 · 7 comments · May be fixed by #189

Comments

@McPhale
Copy link

McPhale commented May 29, 2024 •
edited
Loading

When opening vim from gitu (when committing, for instance) on windows, the editor does not respond to keystrokes. The graphical version of vim seems to work just fine. Not sure if this is a gitu issue or not, but vim works fine in other scenarios. I've tried setting the editor variable in both git config and the system environment variables to see if that makes a difference, and have tried removing my vimrc to see if there was a problem there, but it didn't change the behavior. The issue also appears in both cmd and windows terminal running powershell core. The editor variable is set to "C:/Tools/Vim/vim91/vim.exe".

Any thoughts?

Thanks for the very cool utility!
@altsem
Copy link
Owner

altsem commented May 29, 2024 •
edited
Loading

Hi! Was just a matter of time before this would happen I feel, the implementation is not ideal.

It seems that Gitu is unable to realize we're actually opening Vim due to the .exe at the end. Defaulting to the file/line format: vim file.txt:120 instead of vim +120 file.txt.

If you try opening a file like vim file.txt:120, does it also hang?

Got the idea of checking the output of vim --version (or whichever editor is configured) instead of the path, which might be more reliable.
But I wonder if there's an even better way.

The existing code is found here:

gitu/src/ops/show.rs

Lines 67 to 103 in 9d9ea52

fn parse_editor_command(editor: &str, file: &str, maybe_line: Option<u32>) -> Command {
let args = &editor.split_whitespace().collect::<Vec<_>>();
let mut cmd = Command::new(args[0]);
cmd.args(&args[1..]);
let lower = args[0].to_lowercase();
if let Some(line) = maybe_line {
if lower.ends_with("vi")
|| lower.ends_with("vim")
|| lower.ends_with("nvim")
|| lower.ends_with("nano")
{
cmd.args([&format!("+{}", line), file]);
} else {
cmd.args([&format!("{}:{}", file, line)]);
}
} else {
cmd.args([file.to_string()]);
}
cmd
}
#[cfg(test)]
mod tests {
use std::ffi::OsStr;
#[test]
fn parse_editor_command_test() {
let cmd = super::parse_editor_command("/bin/nAnO -f", "README.md", Some(42));
assert_eq!(cmd.get_program(), OsStr::new("/bin/nAnO"));
assert_eq!(
&cmd.get_args().collect::<Vec<_>>(),
&["-f", "+42", "README.md"]
);
}
}

Happy you like it! :)

altsem added a commit that referenced this issue Jun 3, 2024
@altsem
fix: identify editor via --version instead of inspecting cmd path
43acd75 
e.g. run `nano --version` and inspect the output

closes: #186
@altsem altsem linked a pull request Jun 3, 2024 that will close this issue
Open
@altsem
Copy link
Owner

altsem commented Jun 3, 2024

@McPhale I potentially fixed this in #189. Just needs some testing.

@McPhale
Copy link
Author

McPhale commented Jun 3, 2024

Cool! If I have some extra free time this week I can test it out, thanks for taking a look

@McPhale
Copy link
Author

McPhale commented Jun 7, 2024

Ok, I managed to get a debugger set up and stepped through the code in the new branch. It doesn't look like the changes you made in show.rs are ever getting hit; with the editor set to vim, it gets as far as the function "run_cmd_interactive" in state.rs, where it hangs as as soon as vim spawns. I tried setting it to nano, and while it works fine from the shell it spits out an error code when gitu tries to open it. Sorry I don't know the first thing about rust, I'm not sure how helpful this is! (I took a screen capture of this but it's too large to upload. If you think that would be helpful I can zip it up or something).

@altsem
Copy link
Owner

altsem commented Jun 8, 2024

Interesting! I'm not sure what's going on.

Else I'd double-check the environment variables: GIT_EDITOR, VISUAL, EDITOR. These (in this order) dictate which editor to launch.

Screenshot would be helpful, or (if it's just text), I guess you could copy-paste it?

And some notes for myself/others that happen to read this:
It does open the process (vim/nano) and then pipes all input/output from Gitu to it.
Perhaps there's a better way for Gitu spawn and handle the process?

@McPhale
Copy link
Author

McPhale commented Jun 24, 2024

Hello; had an issue with my machine and had to get a new one and set it all up, so I'm just now looking at this again. Here is my environment variable:

image

@altsem
Copy link
Owner

altsem commented Jul 1, 2024 •
edited
Loading

I tried setting it to nano, and while it works fine from the shell it spits out an error code when gitu tries to open it. Sorry I don't know the first thing about rust, I'm not sure how helpful this is! (I took a screen capture of this but it's too large to upload.

I mean the error :) Do you have the error code that you saw?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: identify editor via --version instead of inspecting cmd path altsem/gitu
2 participants
@altsem @McPhale