sqweek / dialog

Simple cross-platform dialog API for go-lang
ISC License
516 stars 78 forks source link

[Windows] Load() changes the program's working directory #63

Closed PixelOmen closed 2 years ago

PixelOmen commented 2 years ago

I have a test app that just creates a text file and writes a line to it:

package main

import "os"

func main() {
    fp, err := os.OpenFile("test.txt", os.O_APPEND|os.O_CREATE, 0777)
    if err != nil {
        panic(err)
    }
    defer fp.Close()
    fp.WriteString("This is a test")
}

I built this into an exe as "test.exe" and when I run it, it works as expected.

I have another app that uses the dialog package:

package main

import (
    "fmt"
    "os/exec"

    "github.com/sqweek/dialog"
)

func main() {
    filedialog := dialog.File()
    filename, err := filedialog.Load()
    if err != nil {
        fmt.Println(err)
    }
    fmt.Println(filename)
    testapp := exec.Command("test.exe")
    testapp.Start()

}

If i select a file through the filedialog, it correctly assigns the path as a string into the filename variable, but text.exe never executes, i.e. "text.txt" never gets created. If i cancel the filedialog, "Cancelled" gets printed to stdout, and the "test.txt" file gets created.

For the record, I initially ran into this when running a different executable, I just created "test.exe" for illustrative purposes. What am I missing here?

PixelOmen commented 2 years ago

I found the problem. It seems like dialog and/or w32 is changing the working directory of the Go app to the parent directory of whatever file is selected via the dialog window.

Since I'm not using an absolute path to "test.exe", it becomes "pathToSelectedFile/test.exe". I also happened to have a "text.exe" in my PATH env variable, so testapp.Start() doesn't return any errors unless I call it with a relative path like "./test.exe".

Simply changing the working directory back (via os.Chdir) is a simple and straight forward workaround, but I assume this isn't intended behavior?

sqweek commented 2 years ago

Haha, amazing!

I still haven’t had time to look into this but nice detective work. My initial suspicion is that it’s an unintended side effect of one of the win32 api calls.

-sqweek

On 18 Jan 2022, at 06:25, PixelOmen @.***> wrote:

 I found the problem. It seems like dialog and/or w32 is changing the working directory of the Go app to the parent directory of whatever file is selected via the dialog window.

Since I'm not using an absolute path to "test.exe", it becomes "pathToSelectedFile/test.exe". Also since I wasn't calling it directly via dot syntax ("./text.exe"), it seems it was just failing silently.

Simply changing the working directory back (via os.Chdir) is a simple and straight forward workaround, but I assume this isn't intended behavior?

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you are subscribed to this thread.

redraskal commented 2 years ago

The issue impacts the openfile call (both load & save)- pushed a fix. Windows is weird.

sqweek commented 2 years ago

This issue is half-fixed -- thanks for doing the legwork @redraskal! The example code in the first comment here will now work, but unfortunately I don't see a way to prevent the underlying WIN32 calls from changing the process's working directory which means concurrent goroutines may still see surprises. I used this code to test that scenario:

package main

import (
    "fmt"
    "time"
    "runtime"
    "os"

    "github.com/sqweek/dialog"
)

func main() {
    runtime.GOMAXPROCS(2)

    path, _ := os.Getwd()
    fmt.Println("starting dir:", path)
    go func() {
        ticker := time.NewTicker(1 * time.Second)
        for _ = range ticker.C {
            p, _ := os.Getwd()
            fmt.Println("current dir:", p)
        }
    }()

    file, err := dialog.File().Title("Open").Filter("All Files", "*").Load()
    fmt.Println("Selected file/Error:", file, err)

    file, err = dialog.File().Title("Save").Filter("All Files", "*").Save()
    fmt.Println("Selected file/Error:", file, err)

    path, _ = os.Getwd()
    fmt.Println("final dir:", path)
}

I've added a note to the readme about this behaviour.

HACKERALERT commented 2 years ago

@sqweek @redraskal I'm still having this issue.

f := dialog.File()
f.Title("Save as:")
f.SetStartDir(filepath.Dir(onlyFiles[0])}
file, _ := f.Save()

The first time the block above runs, all is fine and the start directory is correct. But when I choose to save the file in my Downloads, the next time the block above runs again, it goes to my Downloads instead of the directory I set. Any ideas? Thanks.

HACKERALERT commented 2 years ago

Found the issue: https://docs.microsoft.com/en-ca/windows/win32/api/commdlg/ns-commdlg-openfilenamea?redirectedfrom=MSDN, Ctrl+F "The initial directory. The algorithm for selecting the initial directory varies on different platforms.":

Windows 7: If lpstrInitialDir has the same value as was passed the first time the application used an Open or Save As dialog box, the path most recently selected by the user is used as the initial directory.

So if I SetStartDir to the same path, the last location where the user chose to save a file will be used instead of what I set.

HACKERALERT commented 2 years ago

https://stackoverflow.com/questions/4117222/on-win7-getopenfilename-function-ignores-the-lpstrinitialdir-arguament

HACKERALERT commented 2 years ago

Solved it. In dlgswindows.go, I changed `d.opf.InitialDir, = syscall.UTF16PtrFromString(b.StartDir) to d.opf.File, _ = syscall.UTF16PtrFromString(b.StartDir+"\file")` Now it always opens in the directory I choose with "file" prefilled in the filename box. Thanks for the great library!

Edit: The code above I just realized doesn't work because it sets a new pointer that dialog will not be able to retrieve the chosen path as. See the link below for the simple solution.

if b.StartDir != "" {
    tmp,_ := syscall.UTF16FromString(b.StartDir+"\\"+b.InitFilename)
    copy(d.buf,tmp)
}

works fine.

https://github.com/HACKERALERT/dialog/blob/master/dlgs_windows.go#L101 for reference.

redraskal commented 2 years ago

Solved it. In dlgswindows.go, I changed `d.opf.InitialDir, = syscall.UTF16PtrFromString(b.StartDir)tod.opf.File, _ = syscall.UTF16PtrFromString(b.StartDir+"\file")` Now it always opens in the directory I choose with "file" prefilled in the filename box. Thanks for the great library!

Edit: The code above I just realized doesn't work because it sets a new pointer that dialog will not be able to retrieve the chosen path as. See the link below for the simple solution.

if b.StartDir != "" {
    tmp,_ := syscall.UTF16FromString(b.StartDir+"\\"+b.InitFilename)
    copy(d.buf,tmp)
}

works fine.

https://github.com/HACKERALERT/dialog/blob/master/dlgs_windows.go#L101 for reference.

I appreciate the work! I haven't had time to look into this issue & would have likely run into it as well in the future.

sqweek commented 2 years ago

Ah damn, I almost pointed you in that direction suspecting it was the standard windows behaviour, but then I was like "ah but surely that doesn't apply when you explicitly set the start directory!" Thanks W32API, love being confounded xD

That said, I can understand the reasoning behind it because as a user if I had to open a dialog a bunch of times in a row for files in the same directory, it would be pretty annoying if it reset to some application defined default directory every time?