-
Notifications
You must be signed in to change notification settings - Fork 13
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.
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
Odd error message when attempting to pass full path to --editor #77
Comments
@mtimkovich my hunch seems to have been right. It can be addressed using this: diff --git a/src/main.rs b/src/main.rs
index 583ee64..b0c98f0 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -253,7 +253,11 @@ fn open_editor(
write!(tmpfile, "{}", input_files.join("\n"))?;
}
- let editor_parsed = shell_words::split(editor_string)
+ let editor_path = Path::new(editor_string);
+ let editor_quoted_path = shell_words::quote(&editor_string).to_string();
+ let editor_string = if editor_path.exists() { editor_quoted_path } else { editor_string.to_string() };
+
+ let editor_parsed = shell_words::split(&editor_string)
.expect("failed to parse command line flags in EDITOR command");
tmpfile.seek(SeekFrom::Start(0))?;
let child = Command::new(&editor_parsed[0]) What do you think? I'm sure you have a much deeper understanding of Rust than I have, so changes are improvements can be made. This is still far from ideal, because it will also give the same weird output for the PS: tested with |
Thinking about this some further, I'm more and more convinced that Any input, @mtimkovich? |
I'm doing some research into how other tools handle this issue and I'll report back |
This person ran into the same problem as us. What do you think about using their package if the user is on Windows? |
I also found that crate, would that be a valid solution to you? Personally I am fairly conservative when it comes to adding new dependencies. That's why I ask. |
I get the following (found while looking into #76), running from
cmd.exe
.The same can be got via
pwsh.exe
:It appears that either the output of the path interpolates the backslashes somehow or the path gets interpolated by
renamer
(1.6.5) removing all backslashes.@mtimkovich My current working theory is that
shell_words::split()
and/orshell_words::join()
are coming back to haunt us in this case. Mind you, I introduced those 😑. It may be sufficient to make sure that all backslashes are doubled on Windows. I'll have a look.The text was updated successfully, but these errors were encountered: