Byron / open-rs

Open a path or URL with the system-defined program
http://byron.github.io/open-rs
MIT License
319 stars 49 forks source link

In Windows, opening a file without default program does nothing (in VSCode) #73

Open Zheoni opened 1 year ago

Zheoni commented 1 year ago

When trying to open a file that has no association with any program, nothing happens and no error is returned. Normally in Windows a selector is displayed that allows the user to choose which application to use, but this does not happen.

Byron commented 1 year ago

I tried this on Windows 11 and it worked as expected.

# in git bash
cargo run -- https://google.com

Can you provide more information on how to reproduce this? The Windows version may affect the outcome as well, the open version should be named too, along with everything else that might be relevant.

Thank you

Zheoni commented 1 year ago

Ok, now I'm really confused. It works. But when nested deep in my program it doesn't. I have checked that the path argument is exactly the same. It's a big async program with tokio and an axum web server. It stops working correctly depending on where I call open::that. I have not been able to isolate the issue. If I manage to do it I will write another comment on the issue.

I'm testing:

Byron commented 1 year ago

That's interesting and curious indeed! Spawn is spawn, one would think. Since tokio is used here, you can also open::commands(…) and turn these into async commands, for opening within tokio proper. It shouldn't make a difference, but these are desperate times. Alternatively, you could see if using the opener crate makes a different. It's mostly the same, but maybe details matter here.

Zheoni commented 1 year ago

I found a new quirk. I was running the program inside the built-in VSCode terminal. Running it directly in the windows terminal works as expected. The opener crate and tokio::process::Command behave the same.

Byron commented 1 year ago

That's not at all the first time I am hearing that from within VSCode, opening won't work. It's even more curious that opening with a custom program works, but maybe that means there is something special about cmd.exe start.

Fundevoge commented 1 year ago

I have the same problem when trying to open a local .html file.

Calling open::that(filename).unwrap(); yields to thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Custom { kind: Other, error: "Launcher \"cmd\" \"/c\" \"start\" \"\" \""C:\\Users\\me\\AppData\\Local\\Temp\\7b8dc1ba-dfcd-4150-bd59-9ed03f93c730.html\"\" failed with ExitStatus(ExitStatus(1))" }', src\main.rs:59:21

My fix (reusing code from src/windows.rs) was spawning a cmd process that opens a browser using the explorer command instead of the start command:

    Command::new("cmd")
        .arg("/c")
        .arg("explorer")
        .raw_arg(wrap_in_quotes(filename))
        .creation_flags(CREATE_NO_WINDOW)
        .status()
        .unwrap();
Byron commented 1 year ago

Thanks for sharing, it's great to have a workaround!

Now I wonder if on Windows, one could just add this invocation as another option, treating the explorer invocation as additional launcher? Is this something @Fundevoge would like to contribute? Thanks for your consideration 🙏.

Byron commented 1 year ago

Actually I couldn't wait but would love it if someone could try out the branch with the fix. Once confirmed working, I'd make a patch release immediately :). Thanks for your help.

Fundevoge commented 1 year ago

First of all, thanks for responding and working on it. I have tried the fix and got mixed results:

  1. I still get the error mentioned above.
  2. When I change the order of the return values of the commands function in src/windows.rs to vec![alt_cmd, cmd], it at least opens the window but still throws an error:
    thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Custom { kind: Other, error: "Launcher \"cmd\" \"/c\" \"explorer\" \"C:\\Users\\me\\AppData\\Local\\Temp\\test.html\" failed with ExitStatus(ExitStatus(1))" }', src\main.rs:32:21

    The message is identical except for that the command now is explorer.

  3. For some reason, the cmd command returns ExitStatus 1, which according to Microsoft means ERROR_INVALID_FUNCTION, although the command worked.
  4. Even when I run cmd /c explorer directly in the windows command prompt, it successfully opens the Exploer, but returns ExitStatus 1.
  5. So maybe this is intended behavior? In this case, a fix would be to add an exception for Windows and exit code 1 in src/lib.rs impl IntoResult<io::Result<()>> for io::Result<std::process::ExitStatus>? But ignoring ExitStatus 1 still does not feel like a nice solution to me.
Fundevoge commented 1 year ago

explorer.exe has weird behavior: It does not write anything into %errorlevel% (https://github.com/microsoft/WSL/issues/6565). Therefore, when explorer is used to open i.e. a html file %errorlevel% is unchanged and calling .status() does not say anything about whether it successfully opened the file. If explorer has a problem opening the file, it will just open the Windows Explorer File Manager instead.

A possible fix would be to wrap the std::process::Command returned by src/windows.rs in the function commands in an enum indicating whether it is the explorer command and then use that information in src/lib.rs: impl IntoResult<io::Result<()>> for io::Result<std::process::ExitStatus> to ignore the exit status in case of an explorer command.

If someone has a better idea, please let me know. Anyway, I can certainly contribute a solution, once the path is clear. A possible fix would be to wrap the std::process::Command returned by src/windows.rs in the function commands in an enum indicating whether it is the explorer command and then use that information in src/lib.rs: impl IntoResult<io::Result<()>> for io::Result<std::process::ExitStatus> to ignore the exit status in case of an explorer command.

If someone has a better idea, please let me know. Anyway, I can certainly contribute a solution, once the path is decided.

Byron commented 1 year ago

Thanks a lot for looking into this and for sharing your research.

It's interesting as the issue with the exit code being inverted is happening only in VSCode, right? Because these problems simply don't exist under 'normal' circumstances. Is this true or am I getting something wrong here?

Regarding the solution, it's possible to let windows have its very own open::that() function that knows its commands() well. My feeling here is though that it even needs a special case for VSCode on windows to do the right thing. Thanks for your help!

Fundevoge commented 1 year ago

When trying to open a file that has no association with any program, nothing happens and no error is returned. Normally in Windows a selector is displayed that allows the user to choose which application to use, but this does not happen.

When using the explorer command, I get this popup, even from VSCode, with integrated PowerShell or CMD Prompt.

It's interesting as the issue with the exit code being inverted is happening only in VSCode, right? Because these problems simply don't exist under 'normal' circumstances. Is this true or am I getting something wrong here?

The explorer command not having an exit code seems to be a windows bug, as this also happens inside a regular command prompt.

Byron commented 1 year ago

The explorer command not having an exit code seems to be a windows bug, as this also happens inside a regular command prompt.

That sounds like explorer is the same, but cmd might be different? Anyway, for now I will assume both have the same exit-code behaviour no matter where they are run, which means that a fix should be adjust open::that() on windows to be able to deal with return codes. For that it might have to know exactly what's in commands(), but I see no problem with that.

I'd hope that the order of [cmd, alt_cmd] can still be maintained as to not have accidental breakage for existing users. If necessary, the order of [alt_cmd, cmd] should be used only when running within VSCode.

I hope it makes sense and that it enables you to submit a first PR for comments. Thanks again for all your help, you are really pushing this topic forward (and hopefully soon, to completion) 🙏.

Fundevoge commented 1 year ago

I think I have found one source of the initial bug: start has a problem with recently created / modified files. Assuming temp.html already exists, this code works as expected:

let file_name = String::from(r"temp.html");
open::that(file_name).unwrap();

But this code does not:

let file_name = String::from(r"temp.html");
let _file = File::create(file_name.clone()).unwrap();
open::that(file_name).unwrap();

explorer does not have this issue.

Byron commented 1 year ago

Oh that's a wonderful revelation as it means there is nothing special with VSCode on Windows in particular, but it's all about the circumstances of the open operation. Shortly after writing the file is still unavailable for reading on typical windows filesystem as these are nondeterministic from a users point of view (even closing a file will not actually have it closed once the close call returns). I could imagine that the start command checks if a file is readable before trying to open it, but explorer does not. And by the time explorer actually gets to reading the file, all is well. That's the best explanation I can construct from my experience.

eugenesvk commented 5 months ago

Tried to add some "sleep" for a second to make sure the file is written, as well as flush it, didn't help

But then just dropping the file made it work, so it's not about a file being unavailable for reading, just that it needs to be not opened for writing?

let file_name = String::from(r"temp.html");
{let _file = File::create(file_name.clone()).unwrap();}
open::that(file_name).unwrap();

this also fails for an existing file when it's opened for writing let _f = File::options().write(true).append(true).open(file_name.clone()).unwrap();

eugenesvk commented 5 months ago

Interestingly enough, the https://docs.rs/opener/latest/opener/fn.open.html alternative doesn't suffer from this issue and can open the file even if it's being opened for writing. Its docs mention that it uses ShellExecuteW API on Windows

Byron commented 5 months ago

It's great you mention this as there is a shellexecute-on-windows feature that you could try. tauri uses it for example. helix on the other hand relies on tokio where the command-based approach is preferred, so it can't be the default.

eugenesvk commented 5 months ago

That way, it should be possible to open currently opened (for writing) files as well. This feature is only effective on Windows.

Ah, so I've just rediscovered a know thing, then I guess I don't get the availability comment

(FYI this API also shows the selector for unknown file types)

Byron commented 5 months ago

Does this mean you tried the feature and it worked for you? If so, this issue can probably be closed with reference to this comment.

eugenesvk commented 5 months ago

yes it does, though only for that_detached (this is documented in the feature description, just FYI since opener doesn't have this distinction and you can use the same open)

might be worth adding a note to the doc that files opened for writing represent an issue

Also, those tokio folks would still need a fix?

Byron commented 5 months ago

yes it does, though only for that_detached (this is documented in the feature description, just FYI since opener doesn't have this distinction and you can use the same open)

Thanks for highlighting this, I wasn't aware. The API itself feels a bit messy with that_in_background and that_detached, and of course that which also claims to be non-blocking, most of the time.

Also, those tokio folks would still need a fix?

Those who use commands() will always get commands and thus have no fix, but from what I see, they could also, at least on Windows, use that_detached instead, knowing that it won't block. And if it does, ever so slightly, that call might even be performed from tokio::task_blocking(). It's an application-based workaround, and I don't think anything else can be done here.