From 9ec1249a783059ab662b38624579d16e49b36e03 Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Mon, 15 Jun 2020 15:30:50 -0400 Subject: [PATCH 01/28] file dialog: use ./ as start dir by default This swaps ~/ and ./ as the default directory. I have also made / the failover if both of those error. That *shouldn't* happen though. Note that / should be safe even on Windows, since NT supports POSIX-style paths. `/` should expand to NTFS's root, which is almost always `C:\`. XXX: might be worth setting the failover dir in the platform driver. `/` is probably not a good failover choice for iOS, although I'm not sure the file dialog even makes sense there, since they have their own system one? I'm not an iOS dev, so I'm not sure. --- dialog/file.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index db6cb0cbe8..acdbe8a3fa 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -265,10 +265,20 @@ func (f *fileDialog) setSelected(file *fileDialogItem) { func showFile(file *FileDialog) *fileDialog { d := &fileDialog{file: file} ui := d.makeUI() - dir, err := os.UserHomeDir() + + // by default, use ./ as the starting directory + dir, err := os.Getwd() if err != nil { - fyne.LogError("Could not load user home dir", err) - dir, _ = os.Getwd() //fallback + + // if that doesn't work, fail-over to ~/ + fyne.LogError("Could not load CWD", err) + dir, err = os.Getwd() + if err != nil { + + // if that dosen't work, fail over to / + fyne.LogError("Could not load user home dir", err) + dir = "/" + } } d.setDirectory(dir) From d0e337a9bf73b5d44b3c3554df72f4285b11dfb2 Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Mon, 15 Jun 2020 15:38:43 -0400 Subject: [PATCH 02/28] file dialog: Add ShowFile*At methods This adds the concept of a "starting directory", which may be set with a new setter method. If there is a problem with this directory, or it is an empty string, then the CWD is preferred, followed by the user home, followed by `/`. This also adds two new utility methods, ShowFileOpenAt, and ShowFileSaveAt which allow the starting directory to be explicitly set as arguments. This should not break the existing API. --- dialog/file.go | 70 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 55 insertions(+), 15 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index acdbe8a3fa..a71f43406f 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -35,13 +35,14 @@ type fileDialog struct { // FileDialog is a dialog containing a file picker for use in opening or saving files. type FileDialog struct { - save bool - callback interface{} - onClosedCallback func(bool) - filter storage.FileFilter - parent fyne.Window - dialog *fileDialog - dismissText string + save bool + callback interface{} + onClosedCallback func(bool) + filter storage.FileFilter + parent fyne.Window + dialog *fileDialog + dismissText string + startingDirectory string } // Declare conformity to Dialog interface @@ -266,20 +267,39 @@ func showFile(file *FileDialog) *fileDialog { d := &fileDialog{file: file} ui := d.makeUI() - // by default, use ./ as the starting directory - dir, err := os.Getwd() - if err != nil { + dir := "" + + if file.startingDirectory != "" { + // the starting directory is set explicitly + if _, err := os.Stat(file.startingDirectory); err != nil { + fyne.LogError("Error with startingDirectory", err) + } else { + dir = file.startingDirectory + } + } - // if that doesn't work, fail-over to ~/ - fyne.LogError("Could not load CWD", err) + if dir == "" { + // Either no custom starting directory was set, or there + // was an error with it... + // + // Try to use CWD + var err error = nil dir, err = os.Getwd() if err != nil { - // if that dosen't work, fail over to / - fyne.LogError("Could not load user home dir", err) - dir = "/" + // if that doesn't work, fail-over to ~/ + fyne.LogError("Could not load CWD", err) + dir, err = os.UserHomeDir() + if err != nil { + + // if that dosen't work, fail over to / + fyne.LogError("Could not load user home dir", err) + dir = "/" + } } + } + d.setDirectory(dir) size := ui.MinSize().Add(fyne.NewSize(fileIconCellWidth*2+theme.Padding()*4, @@ -355,6 +375,12 @@ func (f *FileDialog) SetFilter(filter storage.FileFilter) { } } +// Configure the directory that the file dialog should show by default. If set +// to the empty string ./ (CWD) is assumed. +func (f *FileDialog) SetStartingDirectory(path string) { + f.startingDirectory = path +} + // NewFileOpen creates a file dialog allowing the user to choose a file to open. // The dialog will appear over the window specified when Show() is called. func NewFileOpen(callback func(fyne.URIReadCloser, error), parent fyne.Window) *FileDialog { @@ -373,10 +399,17 @@ func NewFileSave(callback func(fyne.URIWriteCloser, error), parent fyne.Window) // ShowFileOpen creates and shows a file dialog allowing the user to choose a file to open. // The dialog will appear over the window specified. func ShowFileOpen(callback func(fyne.URIReadCloser, error), parent fyne.Window) { + ShowFileOpenAt(callback, parent, "") +} + +// ShowFileOpenAt works similarly to ShowFileOpen(), but with a custom starting +// directory. +func ShowFileOpenAt(callback func(fyne.URIReadCloser, error), parent fyne.Window, where string) { dialog := NewFileOpen(callback, parent) if fileOpenOSOverride(dialog) { return } + dialog.SetStartingDirectory(where) dialog.Show() } @@ -384,9 +417,16 @@ func ShowFileOpen(callback func(fyne.URIReadCloser, error), parent fyne.Window) // If the user chooses an existing file they will be asked if they are sure. // The dialog will appear over the window specified. func ShowFileSave(callback func(fyne.URIWriteCloser, error), parent fyne.Window) { + ShowFileSaveAt(callback, parent, "") +} + +// ShoeFileSaveAt works simialrly to ShowFileSave(), but with a custom starting +// directory. +func ShowFileSaveAt(callback func(fyne.URIWriteCloser, error), parent fyne.Window, where string) { dialog := NewFileSave(callback, parent) if fileSaveOSOverride(dialog) { return } + dialog.SetStartingDirectory(where) dialog.Show() } From c5561ca2b08ecd9b1bd53eeab290d42ae4c8cba2 Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Mon, 15 Jun 2020 15:59:01 -0400 Subject: [PATCH 03/28] fixed linter errors --- dialog/file.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index a71f43406f..8b36b00128 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -375,8 +375,8 @@ func (f *FileDialog) SetFilter(filter storage.FileFilter) { } } -// Configure the directory that the file dialog should show by default. If set -// to the empty string ./ (CWD) is assumed. +// SetStartingDirectory configures the directory that the file dialog should +// show by default. If set to the empty string ./ (CWD) is assumed. func (f *FileDialog) SetStartingDirectory(path string) { f.startingDirectory = path } @@ -420,7 +420,7 @@ func ShowFileSave(callback func(fyne.URIWriteCloser, error), parent fyne.Window) ShowFileSaveAt(callback, parent, "") } -// ShoeFileSaveAt works simialrly to ShowFileSave(), but with a custom starting +// ShowFileSaveAt works simialrly to ShowFileSave(), but with a custom starting // directory. func ShowFileSaveAt(callback func(fyne.URIWriteCloser, error), parent fyne.Window, where string) { dialog := NewFileSave(callback, parent) From db857e1bf46ec5eaa2d8eb4020b16a05ffc83d93 Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Mon, 15 Jun 2020 17:39:21 -0400 Subject: [PATCH 04/28] file dialog: break out effectiveStartingDir Per @andydotxyz I have moved the logic to calculate the starting directory for the file dialog widget to it's own function. --- dialog/file.go | 80 ++++++++++++++++++++++++++------------------------ 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index 8b36b00128..03f545d17a 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -42,7 +42,7 @@ type FileDialog struct { parent fyne.Window dialog *fileDialog dismissText string - startingDirectory string + StartingDirectory string } // Declare conformity to Dialog interface @@ -263,44 +263,52 @@ func (f *fileDialog) setSelected(file *fileDialogItem) { } } -func showFile(file *FileDialog) *fileDialog { - d := &fileDialog{file: file} - ui := d.makeUI() - - dir := "" - - if file.startingDirectory != "" { +// effectiveStartingDir calculates the directory at which the file dialog should +// open, based on the values of StartingDirectory, CWD, home, and any error +// conditions which occur. +// +// Order of precedence is: +// +// * file.StartingDirectory if non-empty and os.Stat()-able +// * os.Getwd() +// * os.UserHomeDir() +// * "/" (should be filesystem root on all supported platforms) +func (file *FileDialog) effectiveStartingDir() string { + if file.StartingDirectory != "" { // the starting directory is set explicitly - if _, err := os.Stat(file.startingDirectory); err != nil { - fyne.LogError("Error with startingDirectory", err) + if _, err := os.Stat(file.StartingDirectory); err != nil { + fyne.LogError("Error with StartingDirectory", err) } else { - dir = file.startingDirectory + return file.StartingDirectory } } - if dir == "" { - // Either no custom starting directory was set, or there - // was an error with it... - // - // Try to use CWD - var err error = nil - dir, err = os.Getwd() + // Either no custom starting directory was set, or there + // was an error with it... + // + // Try to use CWD + var err error = nil + dir, err := os.Getwd() + if err != nil { + // if that doesn't work, fail-over to ~/ + fyne.LogError("Could not load CWD", err) + dir, err = os.UserHomeDir() if err != nil { - // if that doesn't work, fail-over to ~/ - fyne.LogError("Could not load CWD", err) - dir, err = os.UserHomeDir() - if err != nil { - - // if that dosen't work, fail over to / - fyne.LogError("Could not load user home dir", err) - dir = "/" - } + // if that dosen't work, fail over to / + fyne.LogError("Could not load user home dir", err) + dir = "/" } - } - d.setDirectory(dir) + return dir +} + +func showFile(file *FileDialog) *fileDialog { + d := &fileDialog{file: file} + ui := d.makeUI() + + d.setDirectory(file.effectiveStartingDir()) size := ui.MinSize().Add(fyne.NewSize(fileIconCellWidth*2+theme.Padding()*4, (fileIconSize+fileTextSize)+theme.Padding()*4)) @@ -375,12 +383,6 @@ func (f *FileDialog) SetFilter(filter storage.FileFilter) { } } -// SetStartingDirectory configures the directory that the file dialog should -// show by default. If set to the empty string ./ (CWD) is assumed. -func (f *FileDialog) SetStartingDirectory(path string) { - f.startingDirectory = path -} - // NewFileOpen creates a file dialog allowing the user to choose a file to open. // The dialog will appear over the window specified when Show() is called. func NewFileOpen(callback func(fyne.URIReadCloser, error), parent fyne.Window) *FileDialog { @@ -404,12 +406,12 @@ func ShowFileOpen(callback func(fyne.URIReadCloser, error), parent fyne.Window) // ShowFileOpenAt works similarly to ShowFileOpen(), but with a custom starting // directory. -func ShowFileOpenAt(callback func(fyne.URIReadCloser, error), parent fyne.Window, where string) { +func ShowFileOpenAt(callback func(fyne.URIReadCloser, error), parent fyne.Window, startdir string) { dialog := NewFileOpen(callback, parent) if fileOpenOSOverride(dialog) { return } - dialog.SetStartingDirectory(where) + dialog.StartingDirectory = startdir dialog.Show() } @@ -422,11 +424,11 @@ func ShowFileSave(callback func(fyne.URIWriteCloser, error), parent fyne.Window) // ShowFileSaveAt works simialrly to ShowFileSave(), but with a custom starting // directory. -func ShowFileSaveAt(callback func(fyne.URIWriteCloser, error), parent fyne.Window, where string) { +func ShowFileSaveAt(callback func(fyne.URIWriteCloser, error), parent fyne.Window, startdir string) { dialog := NewFileSave(callback, parent) if fileSaveOSOverride(dialog) { return } - dialog.SetStartingDirectory(where) + dialog.StartingDirectory = startdir dialog.Show() } From 6fbdb2fe6332a186aadd250b5b2dcc13c0bce02a Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Mon, 15 Jun 2020 17:58:56 -0400 Subject: [PATCH 05/28] add tests for effectiveStartingDir() --- dialog/file_test.go | 87 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/dialog/file_test.go b/dialog/file_test.go index 174b93eb4d..ed058ec5e7 100644 --- a/dialog/file_test.go +++ b/dialog/file_test.go @@ -15,6 +15,93 @@ import ( "fyne.io/fyne/widget" ) +// comparePaths compares if two file paths point to the same thing, and calls +// t.Fatalf() if there is an error in performing the comparison. +// +// Returns true of both paths point to the same thing. +// +// You should use this if you need to compare file paths, since it explicitly +// normalizes the paths to a stable canonical form. It also nicely +// abstracts out the requisite error handling. +// +// You should only call this function on paths that you expect to be valid. +func comparePaths(t *testing.T, p1, p2 string) bool { + a1, err := filepath.Abs(p1) + if err != nil { + t.Fatalf("Failed to normalize path '%s'", p1) + } + + a2, err := filepath.Abs(p2) + if err != nil { + t.Fatalf("Failed to normalize path '%s'", p2) + } + + return a1 == a2 +} + +func TestEffectiveStartingDir(t *testing.T) { + wd, err := os.Getwd() + if err != nil { + t.Skipf("os.Getwd() failed, cannot run this test on this system (error getting ./), error was '%s'", err) + } + + parent := filepath.Dir(wd) + _, err = os.Stat(parent) + if err != nil { + t.Skipf("os.Getwd() failed, cannot run this test on this system (error stat()-ing ../) error was '%s'", err) + } + + dialog := &FileDialog{} + + // test that we get ./ when running with the default struct values + res := dialog.effectiveStartingDir() + expect := wd + if err != nil { + t.Skipf("os.Getwd() failed, cannot run this test on this system, error was '%s'", err) + } + if !comparePaths(t, res, expect) { + t.Errorf("Expected effectiveStartingDir() to be '%s', but it was '%s'", + expect, res) + } + + // this should always be equivalent to the preceding test + dialog.StartingDirectory = "" + res = dialog.effectiveStartingDir() + expect = wd + if err != nil { + t.Skipf("os.Getwd() failed, cannot run this test on this system, error was '%s'", err) + } + if !comparePaths(t, res, expect) { + t.Errorf("Expected effectiveStartingDir() to be '%s', but it was '%s'", + expect, res) + } + + // check using StartingDirectory with some other directory + dialog.StartingDirectory = parent + res = dialog.effectiveStartingDir() + expect = parent + if err != nil { + t.Skipf("os.Getwd() failed, cannot run this test on this system, error was '%s'", err) + } + if res != expect { + t.Errorf("Expected effectiveStartingDir() to be '%s', but it was '%s'", + expect, res) + } + + // make sure we fail over if the specified directory does not exist + dialog.StartingDirectory = "/some/file/that/does/not/exist" + res = dialog.effectiveStartingDir() + expect = wd + if err != nil { + t.Skipf("os.Getwd() failed, cannot run this test on this system, error was '%s'", err) + } + if res != expect { + t.Errorf("Expected effectiveStartingDir() to be '%s', but it was '%s'", + expect, res) + } + +} + func TestShowFileOpen(t *testing.T) { var chosen fyne.URIReadCloser var openErr error From be9c0181630164f6079ac5073339a72a77eaa04b Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Tue, 16 Jun 2020 09:29:19 -0400 Subject: [PATCH 06/28] break out just effectiveStartingDir This implements just the effectiveStartingDir() and a basic test from PR #1108. It does provide the ability to set a custom starting dir. Cherry picked from #1111 to #1108 to avoid future merge conflicts. --- dialog/file.go | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index 03f545d17a..41dc731cc0 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -273,35 +273,32 @@ func (f *fileDialog) setSelected(file *fileDialogItem) { // * os.Getwd() // * os.UserHomeDir() // * "/" (should be filesystem root on all supported platforms) -func (file *FileDialog) effectiveStartingDir() string { - if file.StartingDirectory != "" { +func (f *FileDialog) effectiveStartingDir() string { + if f.StartingDirectory != "" { // the starting directory is set explicitly - if _, err := os.Stat(file.StartingDirectory); err != nil { + if _, err := os.Stat(f.StartingDirectory); err != nil { fyne.LogError("Error with StartingDirectory", err) } else { - return file.StartingDirectory + return f.StartingDirectory } } - // Either no custom starting directory was set, or there - // was an error with it... - // // Try to use CWD var err error = nil dir, err := os.Getwd() - if err != nil { - // if that doesn't work, fail-over to ~/ - fyne.LogError("Could not load CWD", err) - dir, err = os.UserHomeDir() - if err != nil { - - // if that dosen't work, fail over to / - fyne.LogError("Could not load user home dir", err) - dir = "/" - } + if err == nil { + return dir + } + fyne.LogError("Could not load CWD", err) + + // fail over to home dir + dir, err = os.UserHomeDir() + if err == nil { + return dir } + fyne.LogError("Could not load user home dir", err) - return dir + return "/" } func showFile(file *FileDialog) *fileDialog { From 7794089e8f116852eaec534d8eceacc08b882b48 Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Fri, 4 Sep 2020 19:51:48 -0400 Subject: [PATCH 07/28] make public API use ListableURI --- dialog/file.go | 63 +++++++++++++++++++++++++++++++-------------- dialog/file_test.go | 28 ++++++++++---------- 2 files changed, 58 insertions(+), 33 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index 4749bd5815..97b6923b0c 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -35,14 +35,17 @@ type fileDialog struct { // FileDialog is a dialog containing a file picker for use in opening or saving files. type FileDialog struct { - save bool - callback interface{} - onClosedCallback func(bool) - filter storage.FileFilter - parent fyne.Window - dialog *fileDialog - dismissText string - StartingDirectory string + save bool + callback interface{} + onClosedCallback func(bool) + filter storage.FileFilter + parent fyne.Window + dialog *fileDialog + dismissText string + + // StartingLocation allows overriding the default location where the + // file dialog should "view" when it is first opened. + StartingLocation fyne.ListableURI } // Declare conformity to Dialog interface @@ -274,15 +277,27 @@ func (f *fileDialog) setSelected(file *fileDialogItem) { // * os.UserHomeDir() // * "/" (should be filesystem root on all supported platforms) func (f *FileDialog) effectiveStartingDir() string { - if f.StartingDirectory != "" { + startdir := "" + + if f.StartingLocation != nil { + startdir = f.StartingLocation.String()[len(f.StartingLocation.Scheme())+3:] + } + + if startdir != "" { // the starting directory is set explicitly - if _, err := os.Stat(f.StartingDirectory); err != nil { - fyne.LogError("Error with StartingDirectory", err) + if _, err := os.Stat(startdir); err != nil { + fyne.LogError("Error with StartingLocation", err) } else { - return f.StartingDirectory + return startdir } } + // Try to get ./ + wd, err := os.Getwd() + if err != nil { + return wd + } + // Try home dir dir, err := os.UserHomeDir() if err == nil { @@ -409,17 +424,21 @@ func NewFileSave(callback func(fyne.URIWriteCloser, error), parent fyne.Window) // ShowFileOpen creates and shows a file dialog allowing the user to choose a file to open. // The dialog will appear over the window specified. func ShowFileOpen(callback func(fyne.URIReadCloser, error), parent fyne.Window) { - ShowFileOpenAt(callback, parent, "") + dialog := NewFileOpen(callback, parent) + if fileOpenOSOverride(dialog) { + return + } + dialog.Show() } // ShowFileOpenAt works similarly to ShowFileOpen(), but with a custom starting -// directory. -func ShowFileOpenAt(callback func(fyne.URIReadCloser, error), parent fyne.Window, startdir string) { +// location. +func ShowFileOpenAt(callback func(fyne.URIReadCloser, error), parent fyne.Window, startlocation fyne.ListableURI) { dialog := NewFileOpen(callback, parent) if fileOpenOSOverride(dialog) { return } - dialog.StartingDirectory = startdir + dialog.StartingLocation = startlocation dialog.Show() } @@ -427,17 +446,21 @@ func ShowFileOpenAt(callback func(fyne.URIReadCloser, error), parent fyne.Window // If the user chooses an existing file they will be asked if they are sure. // The dialog will appear over the window specified. func ShowFileSave(callback func(fyne.URIWriteCloser, error), parent fyne.Window) { - ShowFileSaveAt(callback, parent, "") + dialog := NewFileSave(callback, parent) + if fileSaveOSOverride(dialog) { + return + } + dialog.Show() } // ShowFileSaveAt works simialrly to ShowFileSave(), but with a custom starting -// directory. -func ShowFileSaveAt(callback func(fyne.URIWriteCloser, error), parent fyne.Window, startdir string) { +// location. +func ShowFileSaveAt(callback func(fyne.URIWriteCloser, error), parent fyne.Window, startlocation fyne.ListableURI) { dialog := NewFileSave(callback, parent) if fileSaveOSOverride(dialog) { return } - dialog.StartingDirectory = startdir + dialog.StartingLocation = startlocation dialog.Show() } diff --git a/dialog/file_test.go b/dialog/file_test.go index ce2bf4c811..35fbb4f368 100644 --- a/dialog/file_test.go +++ b/dialog/file_test.go @@ -47,10 +47,15 @@ func TestEffectiveStartingDir(t *testing.T) { t.Skipf("os.UserHomeDir) failed, cannot run this test on this system, error was '%s'", err) } + wd, err := os.Getwd() + if err != nil { + t.Skipf("os.Getwd() failed, cannot run this test on this system (error stat()-ing ../) error was '%s'", err) + } + parent := filepath.Dir(wd) _, err = os.Stat(parent) if err != nil { - t.Skipf("os.Getwd() failed, cannot run this test on this system (error stat()-ing ../) error was '%s'", err) + t.Skipf("os.Stat(wd) failed, cannot run this test on this system (error stat()-ing ../) error was '%s'", err) } dialog := &FileDialog{} @@ -64,36 +69,33 @@ func TestEffectiveStartingDir(t *testing.T) { } // this should always be equivalent to the preceding test - dialog.StartingDirectory = "" + dialog.StartingLocation = nil res = dialog.effectiveStartingDir() expect = wd - if err != nil { - t.Skipf("os.Getwd() failed, cannot run this test on this system, error was '%s'", err) - } if !comparePaths(t, res, expect) { t.Errorf("Expected effectiveStartingDir() to be '%s', but it was '%s'", expect, res) } // check using StartingDirectory with some other directory - dialog.StartingDirectory = parent - res = dialog.effectiveStartingDir() - expect = parent + dialog.StartingLocation, err = storage.ListerForURI(storage.NewURI("file://" + parent)) if err != nil { - t.Skipf("os.Getwd() failed, cannot run this test on this system, error was '%s'", err) + t.Error(err) } + res = dialog.effectiveStartingDir() + expect = parent if res != expect { t.Errorf("Expected effectiveStartingDir() to be '%s', but it was '%s'", expect, res) } // make sure we fail over if the specified directory does not exist - dialog.StartingDirectory = "/some/file/that/does/not/exist" - res = dialog.effectiveStartingDir() - expect = wd + dialog.StartingLocation, err = storage.ListerForURI(storage.NewURI("file:///some/file/that/does/not/exist")) if err != nil { - t.Skipf("os.Getwd() failed, cannot run this test on this system, error was '%s'", err) + t.Error(err) } + res = dialog.effectiveStartingDir() + expect = wd if res != expect { t.Errorf("Expected effectiveStartingDir() to be '%s', but it was '%s'", expect, res) From 7ee7b21a9edcc5b8a499a771bae00bc25e925131 Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Fri, 4 Sep 2020 19:59:33 -0400 Subject: [PATCH 08/28] fix conditional check for ./ --- dialog/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dialog/file.go b/dialog/file.go index 97b6923b0c..e269216699 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -294,7 +294,7 @@ func (f *FileDialog) effectiveStartingDir() string { // Try to get ./ wd, err := os.Getwd() - if err != nil { + if err == nil { return wd } From 7a4a4a5fb4346e96cbfa0e6963fd03ff194d7725 Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Tue, 8 Sep 2020 01:11:45 -0400 Subject: [PATCH 09/28] Migrate file dialog to use URIs throughout --- dialog/file.go | 173 +++++++++++++++++++++++++++++----------- dialog/file_other.go | 8 +- dialog/file_test.go | 69 +++++++++------- dialog/fileitem.go | 11 ++- dialog/fileitem_test.go | 42 ++++++---- storage/uri.go | 12 +++ storage/uri_test.go | 8 ++ test/testfile.go | 45 ++++++++++- 8 files changed, 271 insertions(+), 97 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index e269216699..c2c8011f22 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -1,7 +1,7 @@ package dialog import ( - "io/ioutil" + "fmt" "os" "path/filepath" "strings" @@ -30,7 +30,7 @@ type fileDialog struct { win *widget.PopUp selected *fileDialogItem - dir string + dir fyne.ListableURI } // FileDialog is a dialog containing a file picker for use in opening or saving files. @@ -82,15 +82,17 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { if f.file.save { callback := f.file.callback.(func(fyne.URIWriteCloser, error)) name := f.fileName.(*widget.Entry).Text - path := filepath.Join(f.dir, name) + path := storage.Join(f.dir, name) - info, err := os.Stat(path) + // this assumes the file:// type, which is enforced + // when we set `dir` + info, err := os.Stat(path.String()[len(path.Scheme())+3:]) if os.IsNotExist(err) { f.win.Hide() if f.file.onClosedCallback != nil { f.file.onClosedCallback(true) } - callback(storage.SaveFileToURI(storage.NewURI("file://" + path))) + callback(storage.SaveFileToURI(path)) return } else if info.IsDir() { ShowInformation("Cannot overwrite", @@ -106,7 +108,7 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { return } - callback(storage.SaveFileToURI(storage.NewURI("file://" + path))) + callback(storage.SaveFileToURI(path)) if f.file.onClosedCallback != nil { f.file.onClosedCallback(true) } @@ -117,7 +119,7 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { if f.file.onClosedCallback != nil { f.file.onClosedCallback(true) } - callback(storage.OpenFileFromURI(storage.NewURI("file://" + f.selected.path))) + callback(storage.OpenFileFromURI(f.selected.path)) } }) f.open.Style = widget.PrimaryButton @@ -156,55 +158,92 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { body := fyne.NewContainerWithLayout(layout.NewBorderLayout(scrollBread, nil, nil, nil), scrollBread, f.fileScroll) header := widget.NewLabelWithStyle(label+" File", fyne.TextAlignLeading, fyne.TextStyle{Bold: true}) - favorites := widget.NewGroup("Favorites", f.loadFavorites()...) - return fyne.NewContainerWithLayout(layout.NewBorderLayout(header, footer, favorites, nil), - favorites, header, footer, body) + + favorites, err := f.loadFavorites() + if err != nil { + // only generate the Favorites group if we were able to load + // them successfully + favorites = []fyne.CanvasObject{} + fyne.LogError("Unable to load favorites", err) + } + + favoritesGroup := widget.NewGroup("Favorites", favorites...) + return fyne.NewContainerWithLayout(layout.NewBorderLayout(header, footer, favoritesGroup, nil), + favoritesGroup, header, footer, body) + } -func (f *fileDialog) loadFavorites() []fyne.CanvasObject { - home, _ := os.UserHomeDir() +func (f *fileDialog) loadFavorites() ([]fyne.CanvasObject, error) { + osHome, err := os.UserHomeDir() + if err != nil { + return nil, err + } + + home, err := storage.ListerForURI(storage.NewURI("file://" + osHome)) + if err != nil { + return nil, err + } + + documents, err := storage.ListerForURI(storage.Join(home, "Documents")) + if err != nil { + return nil, err + } + + downloads, err := storage.ListerForURI(storage.Join(home, "Downloads")) + if err != nil { + return nil, err + } + places := []fyne.CanvasObject{ + makeFavoriteButton("Home", theme.HomeIcon(), func() { f.setDirectory(home) }), makeFavoriteButton("Documents", theme.DocumentIcon(), func() { - f.setDirectory(filepath.Join(home, "Documents")) + f.setDirectory(documents) }), makeFavoriteButton("Downloads", theme.DownloadIcon(), func() { - f.setDirectory(filepath.Join(home, "Downloads")) + f.setDirectory(downloads) }), } places = append(places, f.loadPlaces()...) - return places + return places, nil } -func (f *fileDialog) refreshDir(dir string) { +func (f *fileDialog) refreshDir(dir fyne.ListableURI) { f.files.Objects = nil - files, err := ioutil.ReadDir(dir) + files, err := dir.List() if err != nil { - fyne.LogError("Unable to read path "+dir, err) + fyne.LogError("Unable to read path "+dir.String(), err) return } var icons []fyne.CanvasObject - parent := filepath.Dir(dir) - if parent != dir { + parent, err := storage.Parent(dir) + if err != nil { + fyne.LogError("Unable to get parent of "+dir.String(), err) + return + } + if parent.String() != dir.String() { fi := &fileDialogItem{picker: f, icon: canvas.NewImageFromResource(theme.FolderOpenIcon()), - name: "(Parent)", path: filepath.Dir(dir), dir: true} + name: "(Parent)", path: parent, dir: true} fi.ExtendBaseWidget(fi) icons = append(icons, fi) } for _, file := range files { - if isHidden(file.Name(), dir) { + if isHidden(file.Name(), dir.Name()) { continue } - itemPath := filepath.Join(dir, file.Name()) - if file.IsDir() { - icons = append(icons, f.newFileItem(itemPath, true)) - } else if f.file.filter == nil || f.file.filter.Matches(storage.NewURI("file://"+itemPath)) { - icons = append(icons, f.newFileItem(itemPath, false)) + + _, err := storage.ListerForURI(file) + if err == nil { + // URI points to a directory + icons = append(icons, f.newFileItem(file, true)) + + } else if f.file.filter == nil || f.file.filter.Matches(file) { + icons = append(icons, f.newFileItem(file, false)) } } @@ -214,13 +253,19 @@ func (f *fileDialog) refreshDir(dir string) { f.fileScroll.Refresh() } -func (f *fileDialog) setDirectory(dir string) { +func (f *fileDialog) setDirectory(dir fyne.ListableURI) error { f.setSelected(nil) f.dir = dir f.breadcrumb.Children = nil - buildDir := filepath.VolumeName(dir) - for i, d := range strings.Split(dir, string(filepath.Separator)) { + + if dir.Scheme() != "file" { + return fmt.Errorf("Scheme for directory was not file://") + } + + localdir := dir.String()[len(dir.Scheme())+3:] + buildDir := filepath.VolumeName(localdir) + for i, d := range strings.Split(localdir, string(filepath.Separator)) { if d == "" { if i > 0 { // what we get if we split "/" break @@ -234,15 +279,23 @@ func (f *fileDialog) setDirectory(dir string) { buildDir = d + string(os.PathSeparator) } - newDir := buildDir + newDir, err := storage.ListerForURI(storage.NewURI("file://" + buildDir)) + if err != nil { + return err + } f.breadcrumb.Append( widget.NewButton(d, func() { - f.setDirectory(newDir) + err := f.setDirectory(newDir) + if err != nil { + fyne.LogError("Failed to set directory", err) + } }), ) } f.refreshDir(dir) + + return nil } func (f *fileDialog) setSelected(file *fileDialogItem) { @@ -251,17 +304,21 @@ func (f *fileDialog) setSelected(file *fileDialogItem) { f.selected.Refresh() } if file != nil && file.isDirectory() { - f.setDirectory(file.path) + lister, err := storage.ListerForURI(file.path) + if err != nil { + fyne.LogError("Failed to create lister for URI"+file.path.String(), err) + } + f.setDirectory(lister) return } f.selected = file - if file == nil || file.path == "" { + if file == nil || file.path.String()[len(file.path.Scheme())+3:] == "" { f.fileName.SetText("") f.open.Disable() } else { file.isCurrent = true - f.fileName.SetText(filepath.Base(file.path)) + f.fileName.SetText(file.path.Name()) f.open.Enable() } } @@ -272,40 +329,60 @@ func (f *fileDialog) setSelected(file *fileDialogItem) { // // Order of precedence is: // -// * file.StartingDirectory if non-empty and os.Stat()-able +// * file.StartingDirectory if non-empty, os.Stat()-able, and uses the file:// +// URI scheme // * os.Getwd() // * os.UserHomeDir() // * "/" (should be filesystem root on all supported platforms) -func (f *FileDialog) effectiveStartingDir() string { - startdir := "" +// +func (f *FileDialog) effectiveStartingDir() fyne.ListableURI { + var startdir fyne.ListableURI = nil if f.StartingLocation != nil { - startdir = f.StartingLocation.String()[len(f.StartingLocation.Scheme())+3:] + startdir = f.StartingLocation } - if startdir != "" { - // the starting directory is set explicitly - if _, err := os.Stat(startdir); err != nil { - fyne.LogError("Error with StartingLocation", err) - } else { - return startdir + if startdir != nil { + if startdir.Scheme() == "file" { + path := startdir.String()[len(startdir.Scheme())+3:] + + // the starting directory is set explicitly + if _, err := os.Stat(path); err != nil { + fyne.LogError("Error with StartingLocation", err) + } else { + return startdir + } } + } // Try to get ./ wd, err := os.Getwd() if err == nil { - return wd + lister, err := storage.ListerForURI(storage.NewURI("file://" + wd)) + if err == nil { + return lister + } + fyne.LogError("Could not create lister for working dir", err) } // Try home dir dir, err := os.UserHomeDir() if err == nil { - return dir + lister, err := storage.ListerForURI(storage.NewURI("file://" + dir)) + if err == nil { + return lister + } + fyne.LogError("Could not create lister for user home dir", err) } fyne.LogError("Could not load user home dir", err) - return "/" + lister, err := storage.ListerForURI(storage.NewURI("file:///")) + if err != nil { + fyne.LogError("could not create lister for /", err) + return nil + } + return lister } func showFile(file *FileDialog) *fileDialog { diff --git a/dialog/file_other.go b/dialog/file_other.go index 368737befd..27359d4e49 100644 --- a/dialog/file_other.go +++ b/dialog/file_other.go @@ -4,12 +4,18 @@ package dialog import ( "fyne.io/fyne" + "fyne.io/fyne/storage" "fyne.io/fyne/theme" ) func (f *fileDialog) loadPlaces() []fyne.CanvasObject { return []fyne.CanvasObject{makeFavoriteButton("Computer", theme.ComputerIcon(), func() { - f.setDirectory("/") + lister, err := storage.ListerForURI(storage.NewURI("file:///")) + if err != nil { + fyne.LogError("could not create lister for /", err) + return + } + f.setDirectory(lister) })} } diff --git a/dialog/file_test.go b/dialog/file_test.go index 35fbb4f368..523620d0d2 100644 --- a/dialog/file_test.go +++ b/dialog/file_test.go @@ -27,7 +27,10 @@ import ( // abstracts out the requisite error handling. // // You should only call this function on paths that you expect to be valid. -func comparePaths(t *testing.T, p1, p2 string) bool { +func comparePaths(t *testing.T, u1, u2 fyne.ListableURI) bool { + p1 := u1.String()[len(u1.Scheme())+3:] + p2 := u2.String()[len(u2.Scheme())+3:] + a1, err := filepath.Abs(p1) if err != nil { t.Fatalf("Failed to normalize path '%s'", p1) @@ -42,27 +45,33 @@ func comparePaths(t *testing.T, p1, p2 string) bool { } func TestEffectiveStartingDir(t *testing.T) { - home, err := os.UserHomeDir() + + wdString, err := os.Getwd() + if err != nil { + t.Skipf("os.Getwd() failed, cannot run this test on this system (error stat()-ing ../) error was '%s'", err) + } + wd, err := storage.ListerForURI(storage.NewURI("file://" + wdString)) if err != nil { - t.Skipf("os.UserHomeDir) failed, cannot run this test on this system, error was '%s'", err) + t.Skipf("could not get lister for working directory: %s", err) } - wd, err := os.Getwd() + parentURI, err := storage.Parent(wd) if err != nil { - t.Skipf("os.Getwd() failed, cannot run this test on this system (error stat()-ing ../) error was '%s'", err) + t.Skipf("Could not get parent of working directory: %s", err) } - parent := filepath.Dir(wd) - _, err = os.Stat(parent) + parent, err := storage.ListerForURI(parentURI) + t.Log(parentURI) + t.Log(parent) if err != nil { - t.Skipf("os.Stat(wd) failed, cannot run this test on this system (error stat()-ing ../) error was '%s'", err) + t.Skipf("Could not get lister for parent of working directory: %s", err) } dialog := &FileDialog{} - // test that we get $HOME when running with the default struct values + // test that we get wd when running with the default struct values res := dialog.effectiveStartingDir() - expect := home + expect := wd if !comparePaths(t, res, expect) { t.Errorf("Expected effectiveStartingDir() to be '%s', but it was '%s'", expect, res) @@ -78,10 +87,7 @@ func TestEffectiveStartingDir(t *testing.T) { } // check using StartingDirectory with some other directory - dialog.StartingLocation, err = storage.ListerForURI(storage.NewURI("file://" + parent)) - if err != nil { - t.Error(err) - } + dialog.StartingLocation = parent res = dialog.effectiveStartingDir() expect = parent if res != expect { @@ -96,7 +102,7 @@ func TestEffectiveStartingDir(t *testing.T) { } res = dialog.effectiveStartingDir() expect = wd - if res != expect { + if res.String() != expect.String() { t.Errorf("Expected effectiveStartingDir() to be '%s', but it was '%s'", expect, res) } @@ -196,13 +202,13 @@ func TestShowFileOpen(t *testing.T) { // This will only execute if we have a file in the home path. // Until we have a way to set the directory of an open file dialog. test.Tap(target) - assert.Equal(t, filepath.Base(target.path), nameLabel.Text) + assert.Equal(t, target.path.Name(), nameLabel.Text) assert.False(t, open.Disabled()) test.Tap(open) assert.Nil(t, win.Canvas().Overlays().Top()) assert.Nil(t, openErr) - assert.Equal(t, "file://"+target.path, chosen.URI().String()) + assert.Equal(t, target.path.String(), chosen.URI().String()) err := chosen.Close() assert.Nil(t, err) @@ -251,7 +257,7 @@ func TestShowFileSave(t *testing.T) { // This will only execute if we have a file in the home path. // Until we have a way to set the directory of an open file dialog. test.Tap(target) - assert.Equal(t, filepath.Base(target.path), nameEntry.Text) + assert.Equal(t, target.path.Name(), nameEntry.Text) assert.False(t, save.Disabled()) // we are about to overwrite, a warning will show @@ -265,12 +271,17 @@ func TestShowFileSave(t *testing.T) { test.Tap(save) assert.Nil(t, win.Canvas().Overlays().Top()) assert.Nil(t, saveErr) - expectedPath := filepath.Join(filepath.Dir(target.path), "v2_"+filepath.Base(target.path)) - assert.Equal(t, "file://"+expectedPath, chosen.URI().String()) + targetParent, err := storage.Parent(target.path) + if err != nil { + t.Error(err) + } + expectedPath := storage.Join(targetParent, "v2_"+target.path.Name()) + assert.Equal(t, expectedPath.String(), chosen.URI().String()) - err := chosen.Close() + err = chosen.Close() assert.Nil(t, err) - err = os.Remove(expectedPath) + pathString := expectedPath.String()[len(expectedPath.Scheme())+3:] + err = os.Remove(pathString) assert.Nil(t, err) } @@ -287,14 +298,18 @@ func TestFileFilters(t *testing.T) { fyne.LogError("Could not get current working directory", err) t.FailNow() } - testDataDir := filepath.Join(workingDir, "testdata") + testDataDir := storage.NewURI("file://" + filepath.Join(workingDir, "testdata")) + testDataLister, err := storage.ListerForURI(testDataDir) + if err != nil { + t.Error(err) + } - f.dialog.setDirectory(testDataDir) + f.dialog.setDirectory(testDataLister) count := 0 for _, icon := range f.dialog.files.Objects { if icon.(*fileDialogItem).dir == false { - uri := storage.NewURI("file://" + icon.(*fileDialogItem).path) + uri := icon.(*fileDialogItem).path assert.Equal(t, uri.Extension(), ".png") count++ } @@ -306,7 +321,7 @@ func TestFileFilters(t *testing.T) { count = 0 for _, icon := range f.dialog.files.Objects { if icon.(*fileDialogItem).dir == false { - uri := storage.NewURI("file://" + icon.(*fileDialogItem).path) + uri := icon.(*fileDialogItem).path assert.Equal(t, uri.MimeType(), "image/jpeg") count++ } @@ -318,7 +333,7 @@ func TestFileFilters(t *testing.T) { count = 0 for _, icon := range f.dialog.files.Objects { if icon.(*fileDialogItem).dir == false { - uri := storage.NewURI("file://" + icon.(*fileDialogItem).path) + uri := icon.(*fileDialogItem).path mimeType := strings.Split(uri.MimeType(), "/")[0] assert.Equal(t, mimeType, "image") count++ diff --git a/dialog/fileitem.go b/dialog/fileitem.go index 941f69d808..57440e33bc 100644 --- a/dialog/fileitem.go +++ b/dialog/fileitem.go @@ -6,7 +6,6 @@ import ( "fyne.io/fyne" "fyne.io/fyne/canvas" - "fyne.io/fyne/storage" "fyne.io/fyne/theme" "fyne.io/fyne/widget" ) @@ -24,7 +23,7 @@ type fileDialogItem struct { icon fyne.CanvasObject name string - path string + path fyne.URI dir bool } @@ -56,15 +55,15 @@ func (i *fileDialogItem) isDirectory() bool { return i.dir } -func (f *fileDialog) newFileItem(path string, dir bool) *fileDialogItem { +func (f *fileDialog) newFileItem(path fyne.URI, dir bool) *fileDialogItem { var icon fyne.CanvasObject var name string if dir { icon = canvas.NewImageFromResource(theme.FolderIcon()) - name = filepath.Base(path) + name = path.Name() } else { - icon = NewFileIcon(storage.NewURI("file://" + path)) - name = fileName(path) + icon = NewFileIcon(path) + name = path.Name() } ret := &fileDialogItem{ diff --git a/dialog/fileitem_test.go b/dialog/fileitem_test.go index ebabf59b8e..b77dd95cf6 100644 --- a/dialog/fileitem_test.go +++ b/dialog/fileitem_test.go @@ -5,6 +5,7 @@ import ( "testing" "fyne.io/fyne/canvas" + "fyne.io/fyne/storage" "fyne.io/fyne/test" "fyne.io/fyne/theme" @@ -15,13 +16,13 @@ func TestFileItem_Name(t *testing.T) { f := &fileDialog{file: &FileDialog{}} _ = f.makeUI() - item := f.newFileItem("/path/to/filename.txt", false) + item := f.newFileItem(storage.NewURI("file:///path/to/filename.txt"), false) assert.Equal(t, "filename", item.name) - item = f.newFileItem("/path/to/MyFile.jpeg", false) + item = f.newFileItem(storage.NewURI("file:///path/to/MyFile.jpeg"), false) assert.Equal(t, "MyFile", item.name) - item = f.newFileItem("/path/to/.maybeHidden.txt", false) + item = f.newFileItem(storage.NewURI("file:///path/to/.maybeHidden.txt"), false) assert.Equal(t, ".maybeHidden", item.name) } @@ -29,20 +30,20 @@ func TestFileItem_FolderName(t *testing.T) { f := &fileDialog{file: &FileDialog{}} _ = f.makeUI() - item := f.newFileItem("/path/to/foldername/", true) + item := f.newFileItem(storage.NewURI("file:///path/to/foldername/"), true) assert.Equal(t, "foldername", item.name) - item = f.newFileItem("/path/to/myapp.app/", true) + item = f.newFileItem(storage.NewURI("file:///path/to/myapp.app/"), true) assert.Equal(t, "myapp.app", item.name) - item = f.newFileItem("/path/to/.maybeHidden/", true) + item = f.newFileItem(storage.NewURI("file:///path/to/.maybeHidden/"), true) assert.Equal(t, ".maybeHidden", item.name) } func TestNewFileItem(t *testing.T) { f := &fileDialog{file: &FileDialog{}} _ = f.makeUI() - item := f.newFileItem("/path/to/filename.txt", false) + item := f.newFileItem(storage.NewURI("file:///path/to/filename.txt"), false) assert.Equal(t, "filename", item.name) @@ -55,24 +56,37 @@ func TestNewFileItem_Folder(t *testing.T) { f := &fileDialog{file: &FileDialog{}} _ = f.makeUI() currentDir, _ := filepath.Abs(".") - parentDir := filepath.Dir(currentDir) - f.setDirectory(parentDir) - item := f.newFileItem(currentDir, true) + currentLister, err := storage.ListerForURI(storage.NewURI("file://" + currentDir)) + if err != nil { + t.Error(err) + } + + parentDir := storage.NewURI("file://" + filepath.Dir(currentDir)) + parentLister, err := storage.ListerForURI(parentDir) + if err != nil { + t.Error(err) + } + f.setDirectory(parentLister) + item := f.newFileItem(currentLister, true) assert.Equal(t, filepath.Base(currentDir), item.name) test.Tap(item) assert.False(t, item.isCurrent) assert.Equal(t, (*fileDialogItem)(nil), f.selected) - assert.Equal(t, currentDir, f.dir) + assert.Equal(t, currentDir, f.dir.String()[len(f.dir.Scheme())+3:]) } func TestNewFileItem_ParentFolder(t *testing.T) { f := &fileDialog{file: &FileDialog{}} _ = f.makeUI() currentDir, _ := filepath.Abs(".") - parentDir := filepath.Dir(currentDir) - f.setDirectory(currentDir) + currentLister, err := storage.ListerForURI(storage.NewURI("file://" + currentDir)) + if err != nil { + t.Error(err) + } + parentDir := storage.NewURI("file://" + filepath.Dir(currentDir)) + f.setDirectory(currentLister) item := &fileDialogItem{picker: f, icon: canvas.NewImageFromResource(theme.FolderOpenIcon()), name: "(Parent)", path: parentDir, dir: true} @@ -83,5 +97,5 @@ func TestNewFileItem_ParentFolder(t *testing.T) { test.Tap(item) assert.False(t, item.isCurrent) assert.Equal(t, (*fileDialogItem)(nil), f.selected) - assert.Equal(t, parentDir, f.dir) + assert.Equal(t, parentDir.String(), f.dir.String()) } diff --git a/storage/uri.go b/storage/uri.go index c7fc697643..d00749b616 100644 --- a/storage/uri.go +++ b/storage/uri.go @@ -109,3 +109,15 @@ func Parent(u fyne.URI) (fyne.URI, error) { parent = parent + strings.Join(components[0:len(components)-1], "/") + "/" return NewURI(parent), nil } + +// Join appends a new path element to a URI, separated by a '/' character. +func Join(u fyne.URI, component string) fyne.URI { + s := u.String() + + // guarantee that there will be a path separator + if s[len(s)-1:] != "/" { + s += "/" + } + + return NewURI(s + component) +} diff --git a/storage/uri_test.go b/storage/uri_test.go index 8439093d99..204af54344 100644 --- a/storage/uri_test.go +++ b/storage/uri_test.go @@ -82,3 +82,11 @@ func TestURI_Extension(t *testing.T) { assert.Equal(t, ".JPEG", storage.NewURI("file://C:/image.JPEG").Extension()) assert.Equal(t, "", storage.NewURI("file://C:/directory/").Extension()) } + +func TestURI_Join(t *testing.T) { + p := storage.Join(storage.NewURI("file:///foo/bar"), "baz") + assert.Equal(t, "file:///foo/bar/baz", p.String()) + + p = storage.Join(storage.NewURI("file:///foo/bar/"), "baz") + assert.Equal(t, "file:///foo/bar/baz", p.String()) +} diff --git a/test/testfile.go b/test/testfile.go index 59f1564a9d..891869999e 100644 --- a/test/testfile.go +++ b/test/testfile.go @@ -3,6 +3,7 @@ package test import ( "fmt" "io" + "io/ioutil" "os" "path/filepath" @@ -15,6 +16,13 @@ type file struct { path string } +type directory struct { + fyne.URI +} + +// Declare conformity to the ListableURI interface +var _ fyne.ListableURI = (*directory)(nil) + func (f *file) Open() (io.ReadCloser, error) { return os.Open(f.path) } @@ -57,5 +65,40 @@ func (d *testDriver) FileWriterForURI(uri fyne.URI) (fyne.URIWriteCloser, error) } func (d *testDriver) ListerForURI(uri fyne.URI) (fyne.ListableURI, error) { - return nil, fmt.Errorf("test driver does support creating listable URIs yet") + if uri.Scheme() != "file" { + return nil, fmt.Errorf("unsupported URL protocol") + } + + path := uri.String()[len(uri.Scheme())+3 : len(uri.String())] + s, err := os.Stat(path) + if err != nil { + return nil, err + } + + if !s.IsDir() { + return nil, fmt.Errorf("Path '%s' is not a directory, cannot convert to listable URI", path) + } + + return &directory{URI: uri}, nil +} + +func (d *directory) List() ([]fyne.URI, error) { + if d.Scheme() != "file" { + return nil, fmt.Errorf("unsupported URL protocol") + } + + path := d.String()[len(d.Scheme())+3 : len(d.String())] + files, err := ioutil.ReadDir(path) + if err != nil { + return nil, err + } + + urilist := []fyne.URI{} + + for _, f := range files { + uri := storage.NewURI("file://" + filepath.Join(path, f.Name())) + urilist = append(urilist, uri) + } + + return urilist, nil } From 78c6984e497079ae24611d2496a576b2237cd726 Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Sun, 13 Sep 2020 08:19:16 -0400 Subject: [PATCH 10/28] feedback for #1108 --- dialog/file.go | 39 ++++++++++++++++++++++++--------------- dialog/file_test.go | 20 ++++++++++---------- dialog/fileitem.go | 31 ++++++++++++++++--------------- dialog/fileitem_test.go | 2 +- storage/uri.go | 33 ++++++++++++++++++++++++++++++--- storage/uri_test.go | 6 +++--- 6 files changed, 84 insertions(+), 47 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index c2c8011f22..4a84294e1d 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -82,19 +82,20 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { if f.file.save { callback := f.file.callback.(func(fyne.URIWriteCloser, error)) name := f.fileName.(*widget.Entry).Text - path := storage.Join(f.dir, name) + path, _ := storage.Child(f.dir, name) - // this assumes the file:// type, which is enforced - // when we set `dir` - info, err := os.Stat(path.String()[len(path.Scheme())+3:]) - if os.IsNotExist(err) { + exists, _ := storage.Exists(path) + _, err := storage.ListerForURI(path) + + if !exists { f.win.Hide() if f.file.onClosedCallback != nil { f.file.onClosedCallback(true) } callback(storage.SaveFileToURI(path)) return - } else if info.IsDir() { + } else if err != nil { + // check if the directory exists ShowInformation("Cannot overwrite", "Files cannot replace a directory,\ncheck the file name and try again", f.file.parent) return @@ -104,7 +105,7 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { func(ok bool) { f.win.Hide() if !ok { - callback(nil, nil) + // callback(nil, nil) return } @@ -119,7 +120,7 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { if f.file.onClosedCallback != nil { f.file.onClosedCallback(true) } - callback(storage.OpenFileFromURI(f.selected.path)) + callback(storage.OpenFileFromURI(f.selected.location)) } }) f.open.Style = widget.PrimaryButton @@ -184,12 +185,20 @@ func (f *fileDialog) loadFavorites() ([]fyne.CanvasObject, error) { return nil, err } - documents, err := storage.ListerForURI(storage.Join(home, "Documents")) + documentsURI, err := storage.Child(home, "Documents") + if err != nil { + return nil, err + } + documents, err := storage.ListerForURI(documentsURI) if err != nil { return nil, err } - downloads, err := storage.ListerForURI(storage.Join(home, "Downloads")) + downloadsURI, err := storage.Child(home, "Downloads") + if err != nil { + return nil, err + } + downloads, err := storage.ListerForURI(downloadsURI) if err != nil { return nil, err } @@ -228,7 +237,7 @@ func (f *fileDialog) refreshDir(dir fyne.ListableURI) { } if parent.String() != dir.String() { fi := &fileDialogItem{picker: f, icon: canvas.NewImageFromResource(theme.FolderOpenIcon()), - name: "(Parent)", path: parent, dir: true} + name: "(Parent)", location: parent, dir: true} fi.ExtendBaseWidget(fi) icons = append(icons, fi) } @@ -304,21 +313,21 @@ func (f *fileDialog) setSelected(file *fileDialogItem) { f.selected.Refresh() } if file != nil && file.isDirectory() { - lister, err := storage.ListerForURI(file.path) + lister, err := storage.ListerForURI(file.location) if err != nil { - fyne.LogError("Failed to create lister for URI"+file.path.String(), err) + fyne.LogError("Failed to create lister for URI"+file.location.String(), err) } f.setDirectory(lister) return } f.selected = file - if file == nil || file.path.String()[len(file.path.Scheme())+3:] == "" { + if file == nil || file.location.String()[len(file.location.Scheme())+3:] == "" { f.fileName.SetText("") f.open.Disable() } else { file.isCurrent = true - f.fileName.SetText(file.path.Name()) + f.fileName.SetText(file.location.Name()) f.open.Enable() } } diff --git a/dialog/file_test.go b/dialog/file_test.go index 523620d0d2..3b98d83f5f 100644 --- a/dialog/file_test.go +++ b/dialog/file_test.go @@ -97,8 +97,8 @@ func TestEffectiveStartingDir(t *testing.T) { // make sure we fail over if the specified directory does not exist dialog.StartingLocation, err = storage.ListerForURI(storage.NewURI("file:///some/file/that/does/not/exist")) - if err != nil { - t.Error(err) + if err == nil { + t.Errorf("Should have failed to create lister for nonexistant file") } res = dialog.effectiveStartingDir() expect = wd @@ -202,13 +202,13 @@ func TestShowFileOpen(t *testing.T) { // This will only execute if we have a file in the home path. // Until we have a way to set the directory of an open file dialog. test.Tap(target) - assert.Equal(t, target.path.Name(), nameLabel.Text) + assert.Equal(t, target.location.Name(), nameLabel.Text) assert.False(t, open.Disabled()) test.Tap(open) assert.Nil(t, win.Canvas().Overlays().Top()) assert.Nil(t, openErr) - assert.Equal(t, target.path.String(), chosen.URI().String()) + assert.Equal(t, target.location.String(), chosen.URI().String()) err := chosen.Close() assert.Nil(t, err) @@ -257,7 +257,7 @@ func TestShowFileSave(t *testing.T) { // This will only execute if we have a file in the home path. // Until we have a way to set the directory of an open file dialog. test.Tap(target) - assert.Equal(t, target.path.Name(), nameEntry.Text) + assert.Equal(t, target.location.Name(), nameEntry.Text) assert.False(t, save.Disabled()) // we are about to overwrite, a warning will show @@ -271,11 +271,11 @@ func TestShowFileSave(t *testing.T) { test.Tap(save) assert.Nil(t, win.Canvas().Overlays().Top()) assert.Nil(t, saveErr) - targetParent, err := storage.Parent(target.path) + targetParent, err := storage.Parent(target.location) if err != nil { t.Error(err) } - expectedPath := storage.Join(targetParent, "v2_"+target.path.Name()) + expectedPath, _ := storage.Child(targetParent, "v2_"+target.location.Name()) assert.Equal(t, expectedPath.String(), chosen.URI().String()) err = chosen.Close() @@ -309,7 +309,7 @@ func TestFileFilters(t *testing.T) { count := 0 for _, icon := range f.dialog.files.Objects { if icon.(*fileDialogItem).dir == false { - uri := icon.(*fileDialogItem).path + uri := icon.(*fileDialogItem).location assert.Equal(t, uri.Extension(), ".png") count++ } @@ -321,7 +321,7 @@ func TestFileFilters(t *testing.T) { count = 0 for _, icon := range f.dialog.files.Objects { if icon.(*fileDialogItem).dir == false { - uri := icon.(*fileDialogItem).path + uri := icon.(*fileDialogItem).location assert.Equal(t, uri.MimeType(), "image/jpeg") count++ } @@ -333,7 +333,7 @@ func TestFileFilters(t *testing.T) { count = 0 for _, icon := range f.dialog.files.Objects { if icon.(*fileDialogItem).dir == false { - uri := icon.(*fileDialogItem).path + uri := icon.(*fileDialogItem).location mimeType := strings.Split(uri.MimeType(), "/")[0] assert.Equal(t, mimeType, "image") count++ diff --git a/dialog/fileitem.go b/dialog/fileitem.go index 57440e33bc..5fb7bd73c2 100644 --- a/dialog/fileitem.go +++ b/dialog/fileitem.go @@ -21,10 +21,10 @@ type fileDialogItem struct { picker *fileDialog isCurrent bool - icon fyne.CanvasObject - name string - path fyne.URI - dir bool + icon fyne.CanvasObject + name string + location fyne.URI + dir bool } func (i *fileDialogItem) Tapped(_ *fyne.PointEvent) { @@ -43,8 +43,9 @@ func (i *fileDialogItem) CreateRenderer() fyne.WidgetRenderer { img: i.icon, text: text, objects: []fyne.CanvasObject{i.icon, text}} } -func fileName(path string) (name string) { - name = filepath.Base(path) +func fileName(path fyne.URI) (name string) { + pathstr := path.String()[len(path.Scheme())+3:] + name = filepath.Base(pathstr) ext := filepath.Ext(name[1:]) name = name[:len(name)-len(ext)] @@ -55,23 +56,23 @@ func (i *fileDialogItem) isDirectory() bool { return i.dir } -func (f *fileDialog) newFileItem(path fyne.URI, dir bool) *fileDialogItem { +func (f *fileDialog) newFileItem(location fyne.URI, dir bool) *fileDialogItem { var icon fyne.CanvasObject var name string if dir { icon = canvas.NewImageFromResource(theme.FolderIcon()) - name = path.Name() + name = location.Name() } else { - icon = NewFileIcon(path) - name = path.Name() + icon = NewFileIcon(location) + name = fileName(location) } ret := &fileDialogItem{ - picker: f, - icon: icon, - name: name, - path: path, - dir: dir, + picker: f, + icon: icon, + name: name, + location: location, + dir: dir, } ret.ExtendBaseWidget(ret) return ret diff --git a/dialog/fileitem_test.go b/dialog/fileitem_test.go index b77dd95cf6..4398dc9f56 100644 --- a/dialog/fileitem_test.go +++ b/dialog/fileitem_test.go @@ -89,7 +89,7 @@ func TestNewFileItem_ParentFolder(t *testing.T) { f.setDirectory(currentLister) item := &fileDialogItem{picker: f, icon: canvas.NewImageFromResource(theme.FolderOpenIcon()), - name: "(Parent)", path: parentDir, dir: true} + name: "(Parent)", location: parentDir, dir: true} item.ExtendBaseWidget(item) assert.Equal(t, "(Parent)", item.name) diff --git a/storage/uri.go b/storage/uri.go index d00749b616..640ae1e2f3 100644 --- a/storage/uri.go +++ b/storage/uri.go @@ -2,7 +2,9 @@ package storage import ( "bufio" + "fmt" "mime" + "os" "path/filepath" "regexp" "strings" @@ -110,8 +112,13 @@ func Parent(u fyne.URI) (fyne.URI, error) { return NewURI(parent), nil } -// Join appends a new path element to a URI, separated by a '/' character. -func Join(u fyne.URI, component string) fyne.URI { +// Child appends a new path element to a URI, separated by a '/' character. +func Child(u fyne.URI, component string) (fyne.URI, error) { + // While as implemented this does not need to return an error, it is + // reasonable to expect that future implementations of this, especially + // once it gets moved into the URI interface will need to do so. This + // also brings it in line with Parent(). + s := u.String() // guarantee that there will be a path separator @@ -119,5 +126,25 @@ func Join(u fyne.URI, component string) fyne.URI { s += "/" } - return NewURI(s + component) + return NewURI(s + component), nil +} + +// Exists will return true if the resource the URI refers to exists, and false +// otherwise. If an error occurs while checking, false is returned as the first +// return. +func Exists(u fyne.URI) (bool, error) { + if u.Scheme() != "file" { + return false, fmt.Errorf("Don't know how to check existence of %s scheme", u.Scheme()) + } + + _, err := os.Stat(u.String()[len(u.Scheme())+3:]) + if os.IsNotExist(err) { + return false, nil + } + + if err != nil { + return false, err + } + + return true, nil } diff --git a/storage/uri_test.go b/storage/uri_test.go index 204af54344..d51b697bcd 100644 --- a/storage/uri_test.go +++ b/storage/uri_test.go @@ -83,10 +83,10 @@ func TestURI_Extension(t *testing.T) { assert.Equal(t, "", storage.NewURI("file://C:/directory/").Extension()) } -func TestURI_Join(t *testing.T) { - p := storage.Join(storage.NewURI("file:///foo/bar"), "baz") +func TestURI_Child(t *testing.T) { + p, _ := storage.Child(storage.NewURI("file:///foo/bar"), "baz") assert.Equal(t, "file:///foo/bar/baz", p.String()) - p = storage.Join(storage.NewURI("file:///foo/bar/"), "baz") + p, _ = storage.Child(storage.NewURI("file:///foo/bar/"), "baz") assert.Equal(t, "file:///foo/bar/baz", p.String()) } From e8a22692ec481d6593938bd5ffc1bd2405c2ae9c Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Sun, 13 Sep 2020 08:56:57 -0400 Subject: [PATCH 11/28] try to fix WIndows build failures --- dialog/file_windows.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dialog/file_windows.go b/dialog/file_windows.go index f257dfeb5f..bf53cb6d0e 100644 --- a/dialog/file_windows.go +++ b/dialog/file_windows.go @@ -6,6 +6,7 @@ import ( "syscall" "fyne.io/fyne" + "fyne.io/fyne/storage" "fyne.io/fyne/theme" ) @@ -51,7 +52,7 @@ func (f *fileDialog) loadPlaces() []fyne.CanvasObject { for _, drive := range listDrives() { driveRoot := drive + string(os.PathSeparator) // capture loop var places = append(places, makeFavoriteButton(drive, theme.StorageIcon(), func() { - f.setDirectory(driveRoot) + f.setDirectory(storage.NewURI("file://" + driveRoot)) })) } return places From 59c5fb7847dec0396cd029801af5c6e67c48d184 Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Sun, 13 Sep 2020 09:04:51 -0400 Subject: [PATCH 12/28] try again to fix Windows build failure --- dialog/file_windows.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dialog/file_windows.go b/dialog/file_windows.go index bf53cb6d0e..bae024c8d1 100644 --- a/dialog/file_windows.go +++ b/dialog/file_windows.go @@ -51,8 +51,9 @@ func (f *fileDialog) loadPlaces() []fyne.CanvasObject { for _, drive := range listDrives() { driveRoot := drive + string(os.PathSeparator) // capture loop var + driveRootURI, _ := storage.ListerForURI(storage.NewURI("file://" + driveRoot)) places = append(places, makeFavoriteButton(drive, theme.StorageIcon(), func() { - f.setDirectory(storage.NewURI("file://" + driveRoot)) + f.setDirectory(driveRootURI) })) } return places From de97723840c2ee39e12e756748851e5e110055e7 Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Thu, 17 Sep 2020 09:12:08 -0400 Subject: [PATCH 13/28] try to implement Windows path support --- storage/uri.go | 89 +++++++++++++++++++++++++++++++-------------- storage/uri_test.go | 29 +++++++++++---- 2 files changed, 83 insertions(+), 35 deletions(-) diff --git a/storage/uri.go b/storage/uri.go index 640ae1e2f3..5e98346cf8 100644 --- a/storage/uri.go +++ b/storage/uri.go @@ -6,7 +6,7 @@ import ( "mime" "os" "path/filepath" - "regexp" + "runtime" "strings" "unicode/utf8" @@ -61,6 +61,43 @@ func (u *uri) String() string { return u.raw } +// parentGeneric is a generic function that returns the last element of a +// path after splitting it on "/". It should be suitable for most URIs. +func parentGeneric(location string) (string, error) { + + // trim leading forward slashes + trimmed := 0 + for location[0] == '/' { + location = location[1:len(location)] + trimmed++ + + // if all we have left is an empty string, than this URI + // pointed to a UNIX-style root + if len(location) == 0 { + return "", URIRootError + } + } + + components := strings.Split(location, "/") + + if len(components) == 1 { + // trimmed <= 2 makes sure we handle UNIX-style paths on + // Windows correctly + return "", URIRootError + } + + parent := "" + if trimmed > 2 && len(components) > 1 { + // Because we trimmed all the leading '/' characters, for UNIX + // style paths we want to insert one back in. Presumably we + // trimmed two instances of / for the scheme. + parent = parent + "/" + } + parent = parent + strings.Join(components[0:len(components)-1], "/") + "/" + + return parent, nil +} + // Parent gets the parent of a URI by splitting it along '/' separators and // removing the last item. func Parent(u fyne.URI) (fyne.URI, error) { @@ -71,45 +108,41 @@ func Parent(u fyne.URI) (fyne.URI, error) { s = s[0 : len(s)-1] } - // trim the scheme (and +1 for the :) - s = s[len(u.Scheme())+1 : len(s)] + // trim the scheme + s = s[len(u.Scheme())+3:] // Completely empty URI with just a scheme if len(s) == 0 { return nil, URIRootError } - // trim leading forward slashes - trimmed := 0 - for s[0] == '/' { - s = s[1:len(s)] - trimmed++ + parent := "" + if u.Scheme() == "file" { + // use the system native path resolution + parent = filepath.Dir(s) - // if all we have left is an empty string, than this URI - // pointed to a UNIX-style root - if len(s) == 0 { + // only root is it's own parent + if filepath.Clean(parent) == filepath.Clean(s) { return nil, URIRootError } - } - // handle Windows drive letters - r := regexp.MustCompile("[A-Za-z][:]") - components := strings.Split(s, "/") - if len(components) == 1 && r.MatchString(components[0]) && trimmed <= 2 { - // trimmed <= 2 makes sure we handle UNIX-style paths on - // Windows correctly - return nil, URIRootError - } + // URIs are supposed to use forward slashes. On Windows, it + // should be OK to use the platform native filepath with UNIX + // or NT sytle paths, with / or \, but when we reconstruct + // the URI, we want to have / only. + if runtime.GOOS == "windows" { + s = strings.ReplaceAll(s, "\\", "/") + } - parent := u.Scheme() + "://" - if trimmed > 2 && len(components) > 1 { - // Because we trimmed all the leading '/' characters, for UNIX - // style paths we want to insert one back in. Presumably we - // trimmed two instances of / for the scheme. - parent = parent + "/" + } else { + var err error + parent, err = parentGeneric(s) + if err != nil { + return nil, err + } } - parent = parent + strings.Join(components[0:len(components)-1], "/") + "/" - return NewURI(parent), nil + + return NewURI(u.Scheme() + "://" + parent), nil } // Child appends a new path element to a URI, separated by a '/' character. diff --git a/storage/uri_test.go b/storage/uri_test.go index d51b697bcd..3b9190091c 100644 --- a/storage/uri_test.go +++ b/storage/uri_test.go @@ -1,6 +1,7 @@ package storage_test import ( + "runtime" "testing" "fyne.io/fyne/storage" @@ -52,23 +53,37 @@ func TestURI_Parent(t *testing.T) { parent, err := storage.Parent(storage.NewURI("file:///foo/bar/baz")) assert.Nil(t, err) - assert.Equal(t, "file:///foo/bar/", parent.String()) + assert.Equal(t, "file:///foo/bar", parent.String()) parent, err = storage.Parent(storage.NewURI("file:///foo/bar/baz/")) assert.Nil(t, err) - assert.Equal(t, "file:///foo/bar/", parent.String()) + assert.Equal(t, "file:///foo/bar", parent.String()) parent, err = storage.Parent(storage.NewURI("file://C:/foo/bar/baz/")) assert.Nil(t, err) - assert.Equal(t, "file://C:/foo/bar/", parent.String()) + assert.Equal(t, "file://C:/foo/bar", parent.String()) + + if runtime.GOOS == "windows" { + // Only the Windows version of filepath will know how to handle + // backslashes. + parent, err = storage.Parent(storage.NewURI("file://C:\\foo\\bar\\baz\\")) + assert.Nil(t, err) + assert.Equal(t, "file://C:/foo/bar", parent.String()) + } parent, err = storage.Parent(storage.NewURI("file:///")) assert.Equal(t, storage.URIRootError, err) - // This should cause an error, since this is a Windows-style path and - // thus we can't get the parent of a drive letter. - parent, err = storage.Parent(storage.NewURI("file://C:/")) - assert.Equal(t, storage.URIRootError, err) + if runtime.GOOS == "windows" { + // This is only an error under Windows, on *NIX this is a + // relative path to a directory named "C:", which is completely + // valid. + + // This should cause an error, since this is a Windows-style + // path and thus we can't get the parent of a drive letter. + parent, err = storage.Parent(storage.NewURI("file://C:/")) + assert.Equal(t, storage.URIRootError, err) + } // Windows supports UNIX-style paths. /C:/ is also a valid path. parent, err = storage.Parent(storage.NewURI("file:///C:/")) From 4d39cfd911bef2f540493f3474c01c00f44bdeb6 Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Thu, 17 Sep 2020 09:21:43 -0400 Subject: [PATCH 14/28] fix staticcheck errors --- dialog/file.go | 2 +- storage/uri.go | 4 ++-- storage/uri_test.go | 2 +- test/testfile.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index 58419666b9..d6f72c43dd 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -268,7 +268,7 @@ func (f *fileDialog) setDirectory(dir fyne.ListableURI) error { f.breadcrumb.Children = nil if dir.Scheme() != "file" { - return fmt.Errorf("Scheme for directory was not file://") + return fmt.Errorf("scheme for directory was not file://") } localdir := dir.String()[len(dir.Scheme())+3:] diff --git a/storage/uri.go b/storage/uri.go index 5e98346cf8..7da37c1d7a 100644 --- a/storage/uri.go +++ b/storage/uri.go @@ -68,7 +68,7 @@ func parentGeneric(location string) (string, error) { // trim leading forward slashes trimmed := 0 for location[0] == '/' { - location = location[1:len(location)] + location = location[1:] trimmed++ // if all we have left is an empty string, than this URI @@ -131,7 +131,7 @@ func Parent(u fyne.URI) (fyne.URI, error) { // or NT sytle paths, with / or \, but when we reconstruct // the URI, we want to have / only. if runtime.GOOS == "windows" { - s = strings.ReplaceAll(s, "\\", "/") + parent = strings.ReplaceAll(parent, "\\", "/") } } else { diff --git a/storage/uri_test.go b/storage/uri_test.go index 86a0c182d4..c66784698b 100644 --- a/storage/uri_test.go +++ b/storage/uri_test.go @@ -81,7 +81,7 @@ func TestURI_Parent(t *testing.T) { // This should cause an error, since this is a Windows-style // path and thus we can't get the parent of a drive letter. - parent, err = storage.Parent(storage.NewURI("file://C:/")) + _, err = storage.Parent(storage.NewURI("file://C:/")) assert.Equal(t, storage.URIRootError, err) } diff --git a/test/testfile.go b/test/testfile.go index 891869999e..eaa7d8045a 100644 --- a/test/testfile.go +++ b/test/testfile.go @@ -76,7 +76,7 @@ func (d *testDriver) ListerForURI(uri fyne.URI) (fyne.ListableURI, error) { } if !s.IsDir() { - return nil, fmt.Errorf("Path '%s' is not a directory, cannot convert to listable URI", path) + return nil, fmt.Errorf("path '%s' is not a directory, cannot convert to listable URI", path) } return &directory{URI: uri}, nil From d46f9ff9eb4de61cd464332d9001171387c292fc Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Thu, 17 Sep 2020 09:28:39 -0400 Subject: [PATCH 15/28] try to fix windows paths --- storage/uri.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/storage/uri.go b/storage/uri.go index 7da37c1d7a..ec4f819b9b 100644 --- a/storage/uri.go +++ b/storage/uri.go @@ -131,6 +131,9 @@ func Parent(u fyne.URI) (fyne.URI, error) { // or NT sytle paths, with / or \, but when we reconstruct // the URI, we want to have / only. if runtime.GOOS == "windows" { + // seems that sometimes we end up with + // double-backslashes + parent = strings.ReplaceAll(parent, "\\\\", "/") parent = strings.ReplaceAll(parent, "\\", "/") } @@ -167,7 +170,7 @@ func Child(u fyne.URI, component string) (fyne.URI, error) { // return. func Exists(u fyne.URI) (bool, error) { if u.Scheme() != "file" { - return false, fmt.Errorf("Don't know how to check existence of %s scheme", u.Scheme()) + return false, fmt.Errorf("don't know how to check existence of %s scheme", u.Scheme()) } _, err := os.Stat(u.String()[len(u.Scheme())+3:]) From 210fe343353f3e30b567f99205fa730e5f29cbef Mon Sep 17 00:00:00 2001 From: PucklaMotzer09 Date: Mon, 21 Sep 2020 13:37:12 +0200 Subject: [PATCH 16/28] Fix some issues related to ListableURI and dialog --- dialog/file.go | 4 ++-- dialog/file_windows.go | 10 ++++++---- storage/uri.go | 5 +++++ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index d6f72c43dd..9a9e6878d6 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -230,11 +230,11 @@ func (f *fileDialog) refreshDir(dir fyne.ListableURI) { var icons []fyne.CanvasObject parent, err := storage.Parent(dir) - if err != nil { + if err != nil && err != storage.URIRootError { fyne.LogError("Unable to get parent of "+dir.String(), err) return } - if parent.String() != dir.String() { + if parent != nil && parent.String() != dir.String() { fi := &fileDialogItem{picker: f, name: "(Parent)", location: parent, dir: true} fi.ExtendBaseWidget(fi) diff --git a/dialog/file_windows.go b/dialog/file_windows.go index bae024c8d1..fd64c9d3c7 100644 --- a/dialog/file_windows.go +++ b/dialog/file_windows.go @@ -51,10 +51,12 @@ func (f *fileDialog) loadPlaces() []fyne.CanvasObject { for _, drive := range listDrives() { driveRoot := drive + string(os.PathSeparator) // capture loop var - driveRootURI, _ := storage.ListerForURI(storage.NewURI("file://" + driveRoot)) - places = append(places, makeFavoriteButton(drive, theme.StorageIcon(), func() { - f.setDirectory(driveRootURI) - })) + driveRootURI, err := storage.ListerForURI(storage.NewURI("file://" + driveRoot)) + if err == nil { + places = append(places, makeFavoriteButton(drive, theme.StorageIcon(), func() { + f.setDirectory(driveRootURI) + })) + } } return places } diff --git a/storage/uri.go b/storage/uri.go index ec4f819b9b..3629784a51 100644 --- a/storage/uri.go +++ b/storage/uri.go @@ -104,6 +104,11 @@ func Parent(u fyne.URI) (fyne.URI, error) { s := u.String() // trim trailing slash + if runtime.GOOS == "windows" { + if s[len(s)-1] == '\\' { + s = s[0 : len(s)-1] + } + } if s[len(s)-1] == '/' { s = s[0 : len(s)-1] } From 29166289e2f4e1ec5349c78bf1c21ce6aadfc7d5 Mon Sep 17 00:00:00 2001 From: PucklaMotzer09 Date: Mon, 21 Sep 2020 14:21:19 +0200 Subject: [PATCH 17/28] Replace fws with bws in setDirectory for windows --- dialog/file.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dialog/file.go b/dialog/file.go index 9a9e6878d6..fe5e55693f 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" "fyne.io/fyne" @@ -272,6 +273,10 @@ func (f *fileDialog) setDirectory(dir fyne.ListableURI) error { } localdir := dir.String()[len(dir.Scheme())+3:] + if runtime.GOOS == "windows" { + localdir = strings.ReplaceAll(localdir, "/", "\\") + } + buildDir := filepath.VolumeName(localdir) for i, d := range strings.Split(localdir, string(filepath.Separator)) { if d == "" { From 9c055c7bad5edeca98f6f9009db8bf1bf92404b8 Mon Sep 17 00:00:00 2001 From: PucklaMotzer09 Date: Mon, 21 Sep 2020 15:16:48 +0200 Subject: [PATCH 18/28] Fix save and open dialog tests --- dialog/file.go | 23 ++++++++++++++++++++++- dialog/file_test.go | 11 ++++++++++- dialog/file_windows.go | 10 ++++------ 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index fe5e55693f..23f45adab6 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -84,7 +84,16 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { name := f.fileName.(*widget.Entry).Text path, _ := storage.Child(f.dir, name) + // On windows replace '\\' with '/' + if runtime.GOOS == "windows" { + pathString := path.String() + pathString = strings.ReplaceAll(pathString, "\\\\", "/") + pathString = strings.ReplaceAll(pathString, "\\", "/") + path = storage.NewURI(pathString) + } + exists, _ := storage.Exists(path) + _, err := storage.ListerForURI(path) if !exists { @@ -120,7 +129,15 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { if f.file.onClosedCallback != nil { f.file.onClosedCallback(true) } - callback(storage.OpenFileFromURI(f.selected.location)) + path := f.selected.location + // On windows replace '\\' with '/' + if runtime.GOOS == "windows" { + pathString := path.String() + pathString = strings.ReplaceAll(pathString, "\\\\", "/") + pathString = strings.ReplaceAll(pathString, "\\", "/") + path = storage.NewURI(pathString) + } + callback(storage.OpenFileFromURI(path)) } }) f.open.Style = widget.PrimaryButton @@ -263,6 +280,10 @@ func (f *fileDialog) refreshDir(dir fyne.ListableURI) { } func (f *fileDialog) setDirectory(dir fyne.ListableURI) error { + if dir == nil { + return fmt.Errorf("failed to open nil directory") + } + f.setSelected(nil) f.dir = dir diff --git a/dialog/file_test.go b/dialog/file_test.go index 3b98d83f5f..d7ac574e3e 100644 --- a/dialog/file_test.go +++ b/dialog/file_test.go @@ -4,6 +4,7 @@ import ( "log" "os" "path/filepath" + "runtime" "strings" "testing" @@ -208,7 +209,15 @@ func TestShowFileOpen(t *testing.T) { test.Tap(open) assert.Nil(t, win.Canvas().Overlays().Top()) assert.Nil(t, openErr) - assert.Equal(t, target.location.String(), chosen.URI().String()) + + targetLocationString := target.location.String() + // Replace all '\\' with '/' on windows since the dialog + // will return a URI with only '/' + if runtime.GOOS == "windows" { + targetLocationString = strings.ReplaceAll(targetLocationString, "\\\\", "/") + targetLocationString = strings.ReplaceAll(targetLocationString, "\\", "/") + } + assert.Equal(t, targetLocationString, chosen.URI().String()) err := chosen.Close() assert.Nil(t, err) diff --git a/dialog/file_windows.go b/dialog/file_windows.go index fd64c9d3c7..bae024c8d1 100644 --- a/dialog/file_windows.go +++ b/dialog/file_windows.go @@ -51,12 +51,10 @@ func (f *fileDialog) loadPlaces() []fyne.CanvasObject { for _, drive := range listDrives() { driveRoot := drive + string(os.PathSeparator) // capture loop var - driveRootURI, err := storage.ListerForURI(storage.NewURI("file://" + driveRoot)) - if err == nil { - places = append(places, makeFavoriteButton(drive, theme.StorageIcon(), func() { - f.setDirectory(driveRootURI) - })) - } + driveRootURI, _ := storage.ListerForURI(storage.NewURI("file://" + driveRoot)) + places = append(places, makeFavoriteButton(drive, theme.StorageIcon(), func() { + f.setDirectory(driveRootURI) + })) } return places } From 347e49a3f3bf88d63299217aaeac482456d72be1 Mon Sep 17 00:00:00 2001 From: PucklaMotzer09 Date: Fri, 25 Sep 2020 12:31:30 +0200 Subject: [PATCH 19/28] Replace backslashes in NewURI --- dialog/file.go | 14 -------------- dialog/file_test.go | 10 +--------- dialog/fileitem_test.go | 2 +- storage/uri.go | 26 ++++++++++---------------- storage/uri_test.go | 7 +++++++ 5 files changed, 19 insertions(+), 40 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index 23f45adab6..7384412cbc 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -84,14 +84,6 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { name := f.fileName.(*widget.Entry).Text path, _ := storage.Child(f.dir, name) - // On windows replace '\\' with '/' - if runtime.GOOS == "windows" { - pathString := path.String() - pathString = strings.ReplaceAll(pathString, "\\\\", "/") - pathString = strings.ReplaceAll(pathString, "\\", "/") - path = storage.NewURI(pathString) - } - exists, _ := storage.Exists(path) _, err := storage.ListerForURI(path) @@ -131,12 +123,6 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { } path := f.selected.location // On windows replace '\\' with '/' - if runtime.GOOS == "windows" { - pathString := path.String() - pathString = strings.ReplaceAll(pathString, "\\\\", "/") - pathString = strings.ReplaceAll(pathString, "\\", "/") - path = storage.NewURI(pathString) - } callback(storage.OpenFileFromURI(path)) } }) diff --git a/dialog/file_test.go b/dialog/file_test.go index d7ac574e3e..a124719f69 100644 --- a/dialog/file_test.go +++ b/dialog/file_test.go @@ -4,7 +4,6 @@ import ( "log" "os" "path/filepath" - "runtime" "strings" "testing" @@ -210,14 +209,7 @@ func TestShowFileOpen(t *testing.T) { assert.Nil(t, win.Canvas().Overlays().Top()) assert.Nil(t, openErr) - targetLocationString := target.location.String() - // Replace all '\\' with '/' on windows since the dialog - // will return a URI with only '/' - if runtime.GOOS == "windows" { - targetLocationString = strings.ReplaceAll(targetLocationString, "\\\\", "/") - targetLocationString = strings.ReplaceAll(targetLocationString, "\\", "/") - } - assert.Equal(t, targetLocationString, chosen.URI().String()) + assert.Equal(t, target.location.String(), chosen.URI().String()) err := chosen.Close() assert.Nil(t, err) diff --git a/dialog/fileitem_test.go b/dialog/fileitem_test.go index 5b4abf3022..9f1c968bfd 100644 --- a/dialog/fileitem_test.go +++ b/dialog/fileitem_test.go @@ -72,7 +72,7 @@ func TestNewFileItem_Folder(t *testing.T) { test.Tap(item) assert.False(t, item.isCurrent) assert.Equal(t, (*fileDialogItem)(nil), f.selected) - assert.Equal(t, currentDir, f.dir.String()[len(f.dir.Scheme())+3:]) + assert.Equal(t, storage.NewURI("file://"+currentDir).String(), f.dir.String()) } func TestNewFileItem_ParentFolder(t *testing.T) { diff --git a/storage/uri.go b/storage/uri.go index 3629784a51..8b1b99f152 100644 --- a/storage/uri.go +++ b/storage/uri.go @@ -20,6 +20,16 @@ type uri struct { // NewURI creates a new URI from the given string representation. // This could be a URI from an external source or one saved from URI.String() func NewURI(u string) fyne.URI { + // URIs are supposed to use forward slashes. On Windows, it + // should be OK to use the platform native filepath with UNIX + // or NT sytle paths, with / or \, but when we reconstruct + // the URI, we want to have / only. + if runtime.GOOS == "windows" { + // seems that sometimes we end up with + // double-backslashes + u = strings.ReplaceAll(u, "\\\\", "/") + u = strings.ReplaceAll(u, "\\", "/") + } return &uri{raw: u} } @@ -104,11 +114,6 @@ func Parent(u fyne.URI) (fyne.URI, error) { s := u.String() // trim trailing slash - if runtime.GOOS == "windows" { - if s[len(s)-1] == '\\' { - s = s[0 : len(s)-1] - } - } if s[len(s)-1] == '/' { s = s[0 : len(s)-1] } @@ -131,17 +136,6 @@ func Parent(u fyne.URI) (fyne.URI, error) { return nil, URIRootError } - // URIs are supposed to use forward slashes. On Windows, it - // should be OK to use the platform native filepath with UNIX - // or NT sytle paths, with / or \, but when we reconstruct - // the URI, we want to have / only. - if runtime.GOOS == "windows" { - // seems that sometimes we end up with - // double-backslashes - parent = strings.ReplaceAll(parent, "\\\\", "/") - parent = strings.ReplaceAll(parent, "\\", "/") - } - } else { var err error parent, err = parentGeneric(s) diff --git a/storage/uri_test.go b/storage/uri_test.go index c66784698b..ac59ed555e 100644 --- a/storage/uri_test.go +++ b/storage/uri_test.go @@ -104,4 +104,11 @@ func TestURI_Child(t *testing.T) { p, _ = storage.Child(storage.NewURI("file:///foo/bar/"), "baz") assert.Equal(t, "file:///foo/bar/baz", p.String()) + + if runtime.GOOS == "windows" { + // Only the Windows version of filepath will know how to handle + // backslashes. + p, _ = storage.Child(storage.NewURI("file://C:\\foo\\bar\\"), "baz") + assert.Equal(t, "file://C:/foo/bar/baz", p.String()) + } } From 568188208ea1193a8174249da7ec4172e62724d9 Mon Sep 17 00:00:00 2001 From: PucklaMotzer09 Date: Fri, 25 Sep 2020 13:14:47 +0200 Subject: [PATCH 20/28] Add tests for parentGeneric --- storage/uri_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/storage/uri_test.go b/storage/uri_test.go index ac59ed555e..605e58d37f 100644 --- a/storage/uri_test.go +++ b/storage/uri_test.go @@ -63,6 +63,24 @@ func TestURI_Parent(t *testing.T) { assert.Nil(t, err) assert.Equal(t, "file://C:/foo/bar", parent.String()) + parent, err = storage.Parent(storage.NewURI("http://foo/bar/baz/")) + assert.Nil(t, err) + assert.Equal(t, "http://foo/bar/", parent.String()) + + parent, err = storage.Parent(storage.NewURI("http:////foo/bar/baz/")) + assert.Nil(t, err) + assert.Equal(t, "http://foo/bar/", parent.String()) + + parent, err = storage.Parent(storage.NewURI("http://foo")) + assert.Equal(t, storage.URIRootError, err) + + parent, err = storage.Parent(storage.NewURI("http:///")) + assert.Equal(t, storage.URIRootError, err) + + parent, err = storage.Parent(storage.NewURI("https://///foo/bar/")) + assert.Nil(t, err) + assert.Equal(t, "https:///foo/", parent.String()) + if runtime.GOOS == "windows" { // Only the Windows version of filepath will know how to handle // backslashes. From 60a46e4daf0ba42457dc53f45d0765587c1311d3 Mon Sep 17 00:00:00 2001 From: PucklaMotzer09 Date: Fri, 25 Sep 2020 13:42:14 +0200 Subject: [PATCH 21/28] Improve tests and always end Parent with slash --- storage/uri.go | 5 +++-- storage/uri_test.go | 18 ++++++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/storage/uri.go b/storage/uri.go index 8b1b99f152..ffcce322b7 100644 --- a/storage/uri.go +++ b/storage/uri.go @@ -91,8 +91,6 @@ func parentGeneric(location string) (string, error) { components := strings.Split(location, "/") if len(components) == 1 { - // trimmed <= 2 makes sure we handle UNIX-style paths on - // Windows correctly return "", URIRootError } @@ -130,6 +128,9 @@ func Parent(u fyne.URI) (fyne.URI, error) { if u.Scheme() == "file" { // use the system native path resolution parent = filepath.Dir(s) + if parent[len(parent)-1] != filepath.Separator { + parent += "/" + } // only root is it's own parent if filepath.Clean(parent) == filepath.Clean(s) { diff --git a/storage/uri_test.go b/storage/uri_test.go index 605e58d37f..0dada47c79 100644 --- a/storage/uri_test.go +++ b/storage/uri_test.go @@ -53,15 +53,15 @@ func TestURI_Parent(t *testing.T) { parent, err := storage.Parent(storage.NewURI("file:///foo/bar/baz")) assert.Nil(t, err) - assert.Equal(t, "file:///foo/bar", parent.String()) + assert.Equal(t, "file:///foo/bar/", parent.String()) parent, err = storage.Parent(storage.NewURI("file:///foo/bar/baz/")) assert.Nil(t, err) - assert.Equal(t, "file:///foo/bar", parent.String()) + assert.Equal(t, "file:///foo/bar/", parent.String()) parent, err = storage.Parent(storage.NewURI("file://C:/foo/bar/baz/")) assert.Nil(t, err) - assert.Equal(t, "file://C:/foo/bar", parent.String()) + assert.Equal(t, "file://C:/foo/bar/", parent.String()) parent, err = storage.Parent(storage.NewURI("http://foo/bar/baz/")) assert.Nil(t, err) @@ -84,9 +84,12 @@ func TestURI_Parent(t *testing.T) { if runtime.GOOS == "windows" { // Only the Windows version of filepath will know how to handle // backslashes. - parent, err = storage.Parent(storage.NewURI("file://C:\\foo\\bar\\baz\\")) + uri := storage.NewURI("file://C:\\foo\\bar\\baz\\") + assert.Equal(t, "file://C:/foo/bar/baz/", uri.String()) + + parent, err = storage.Parent(uri) assert.Nil(t, err) - assert.Equal(t, "file://C:/foo/bar", parent.String()) + assert.Equal(t, "file://C:/foo/bar/", parent.String()) } _, err = storage.Parent(storage.NewURI("file:///")) @@ -126,7 +129,10 @@ func TestURI_Child(t *testing.T) { if runtime.GOOS == "windows" { // Only the Windows version of filepath will know how to handle // backslashes. - p, _ = storage.Child(storage.NewURI("file://C:\\foo\\bar\\"), "baz") + uri := storage.NewURI("file://C:\\foo\\bar\\") + assert.Equal(t, "file://C:/foo/bar/", uri.String()) + + p, _ = storage.Child(uri, "baz") assert.Equal(t, "file://C:/foo/bar/baz", p.String()) } } From b7a6dd8e055f7dc633109a1b9ba8f7e5487b76e5 Mon Sep 17 00:00:00 2001 From: PucklaMotzer09 Date: Fri, 25 Sep 2020 13:59:15 +0200 Subject: [PATCH 22/28] Don't return as soon as one fails in loadFavorites --- dialog/file.go | 61 ++++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index 7384412cbc..a6b390c6bc 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -178,49 +178,46 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { } func (f *fileDialog) loadFavorites() ([]fyne.CanvasObject, error) { - osHome, err := os.UserHomeDir() - if err != nil { - return nil, err - } - - home, err := storage.ListerForURI(storage.NewURI("file://" + osHome)) - if err != nil { - return nil, err - } + var home fyne.ListableURI + var documents fyne.ListableURI + var downloads fyne.ListableURI - documentsURI, err := storage.Child(home, "Documents") - if err != nil { - return nil, err - } - documents, err := storage.ListerForURI(documentsURI) - if err != nil { - return nil, err - } + osHome, err := os.UserHomeDir() - downloadsURI, err := storage.Child(home, "Downloads") - if err != nil { - return nil, err - } - downloads, err := storage.ListerForURI(downloadsURI) - if err != nil { - return nil, err + if err == nil { + home, err = storage.ListerForURI(storage.NewURI("file://" + osHome)) + if err == nil { + documentsURI, err := storage.Child(home, "Documents") + if err == nil { + documents, err = storage.ListerForURI(documentsURI) + } + downloadsURI, err := storage.Child(home, "Downloads") + if err == nil { + downloads, err = storage.ListerForURI(downloadsURI) + } + } } - places := []fyne.CanvasObject{ + var places []fyne.CanvasObject - makeFavoriteButton("Home", theme.HomeIcon(), func() { + if home != nil { + places = append(places, makeFavoriteButton("Home", theme.HomeIcon(), func() { f.setDirectory(home) - }), - makeFavoriteButton("Documents", theme.DocumentIcon(), func() { + })) + } + if documents != nil { + places = append(places, makeFavoriteButton("Documents", theme.DocumentIcon(), func() { f.setDirectory(documents) - }), - makeFavoriteButton("Downloads", theme.DownloadIcon(), func() { + })) + } + if downloads != nil { + places = append(places, makeFavoriteButton("Downloads", theme.DownloadIcon(), func() { f.setDirectory(downloads) - }), + })) } places = append(places, f.loadPlaces()...) - return places, nil + return places, err } func (f *fileDialog) refreshDir(dir fyne.ListableURI) { From 20cbea52ad862919f3936c46e27de1cf5b7b5a65 Mon Sep 17 00:00:00 2001 From: PucklaMotzer09 Date: Fri, 25 Sep 2020 14:17:32 +0200 Subject: [PATCH 23/28] Update breadcrumbs creation and add test for them --- dialog/file.go | 6 +----- dialog/file_test.go | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index a6b390c6bc..52ddbc67ef 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "path/filepath" - "runtime" "strings" "fyne.io/fyne" @@ -277,12 +276,9 @@ func (f *fileDialog) setDirectory(dir fyne.ListableURI) error { } localdir := dir.String()[len(dir.Scheme())+3:] - if runtime.GOOS == "windows" { - localdir = strings.ReplaceAll(localdir, "/", "\\") - } buildDir := filepath.VolumeName(localdir) - for i, d := range strings.Split(localdir, string(filepath.Separator)) { + for i, d := range strings.Split(localdir, "/") { if d == "" { if i > 0 { // what we get if we split "/" break diff --git a/dialog/file_test.go b/dialog/file_test.go index a124719f69..ab5ddfbd47 100644 --- a/dialog/file_test.go +++ b/dialog/file_test.go @@ -4,6 +4,7 @@ import ( "log" "os" "path/filepath" + "runtime" "strings" "testing" @@ -180,6 +181,22 @@ func TestShowFileOpen(t *testing.T) { buttons := ui.Objects[2].(*fyne.Container).Objects[0].(*widget.Box) open := buttons.Children[1].(*widget.Button) + breadcrumb := ui.Objects[3].(*fyne.Container).Objects[0].(*widget.ScrollContainer).Content.(*widget.Box) + assert.Greater(t, len(breadcrumb.Children), 0) + + wd, err := os.Getwd() + if runtime.GOOS == "windows" { + // on windows os.Getwd() returns '\' + wd = strings.ReplaceAll(wd, "\\", "/") + } + assert.Nil(t, err) + components := strings.Split(wd, "/") + if assert.Equal(t, len(components), len(breadcrumb.Children)) { + for i := range components { + assert.Equal(t, components[i], breadcrumb.Children[i].(*widget.Button).Text) + } + } + files := ui.Objects[3].(*fyne.Container).Objects[1].(*widget.ScrollContainer).Content.(*fyne.Container) assert.Greater(t, len(files.Objects), 0) @@ -211,7 +228,7 @@ func TestShowFileOpen(t *testing.T) { assert.Equal(t, target.location.String(), chosen.URI().String()) - err := chosen.Close() + err = chosen.Close() assert.Nil(t, err) } From dc79d372b9bc0f383f9e26dda0b593884b496280 Mon Sep 17 00:00:00 2001 From: PucklaMotzer09 Date: Fri, 25 Sep 2020 14:22:39 +0200 Subject: [PATCH 24/28] Remove checking for "file" scheme in setDirectory --- dialog/file.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index 52ddbc67ef..076c5cb764 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -271,10 +271,6 @@ func (f *fileDialog) setDirectory(dir fyne.ListableURI) error { f.breadcrumb.Children = nil - if dir.Scheme() != "file" { - return fmt.Errorf("scheme for directory was not file://") - } - localdir := dir.String()[len(dir.Scheme())+3:] buildDir := filepath.VolumeName(localdir) From d5ce136109156c2d0ee9c4647395ba2875fad080 Mon Sep 17 00:00:00 2001 From: PucklaMotzer09 Date: Fri, 25 Sep 2020 14:40:34 +0200 Subject: [PATCH 25/28] Fix staticcheck --- dialog/file.go | 34 +++++++++++++++++++++++++--------- storage/uri_test.go | 4 ++-- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index 076c5cb764..1cf19da614 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -180,20 +180,36 @@ func (f *fileDialog) loadFavorites() ([]fyne.CanvasObject, error) { var home fyne.ListableURI var documents fyne.ListableURI var downloads fyne.ListableURI + var osHome string + var err, err1 error - osHome, err := os.UserHomeDir() + osHome, err = os.UserHomeDir() if err == nil { - home, err = storage.ListerForURI(storage.NewURI("file://" + osHome)) - if err == nil { - documentsURI, err := storage.Child(home, "Documents") - if err == nil { - documents, err = storage.ListerForURI(documentsURI) + home, err1 = storage.ListerForURI(storage.NewURI("file://" + osHome)) + if err1 == nil { + var documentsURI fyne.URI + documentsURI, err1 = storage.Child(home, "Documents") + if err1 == nil { + documents, err1 = storage.ListerForURI(documentsURI) + if err1 != nil { + err = err1 + } + } else { + err = err1 } - downloadsURI, err := storage.Child(home, "Downloads") - if err == nil { - downloads, err = storage.ListerForURI(downloadsURI) + var downloadsURI fyne.URI + downloadsURI, err1 = storage.Child(home, "Downloads") + if err1 == nil { + downloads, err1 = storage.ListerForURI(downloadsURI) + if err1 != nil { + err = err1 + } + } else { + err = err1 } + } else { + err = err1 } } diff --git a/storage/uri_test.go b/storage/uri_test.go index 0dada47c79..01d1034b0c 100644 --- a/storage/uri_test.go +++ b/storage/uri_test.go @@ -71,10 +71,10 @@ func TestURI_Parent(t *testing.T) { assert.Nil(t, err) assert.Equal(t, "http://foo/bar/", parent.String()) - parent, err = storage.Parent(storage.NewURI("http://foo")) + _, err = storage.Parent(storage.NewURI("http://foo")) assert.Equal(t, storage.URIRootError, err) - parent, err = storage.Parent(storage.NewURI("http:///")) + _, err = storage.Parent(storage.NewURI("http:///")) assert.Equal(t, storage.URIRootError, err) parent, err = storage.Parent(storage.NewURI("https://///foo/bar/")) From 45233fe5c3fbd3cffa29103d2a5757a414e1196c Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Thu, 8 Oct 2020 15:53:59 -0400 Subject: [PATCH 26/28] revert starting dir to ~, remove OpenAt functions --- dialog/file.go | 42 ++++++++++-------------------------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index 1cf19da614..c78ff6256f 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -356,8 +356,8 @@ func (f *fileDialog) setSelected(file *fileDialogItem) { // // * file.StartingDirectory if non-empty, os.Stat()-able, and uses the file:// // URI scheme -// * os.Getwd() // * os.UserHomeDir() +// * os.Getwd() // * "/" (should be filesystem root on all supported platforms) // func (f *FileDialog) effectiveStartingDir() fyne.ListableURI { @@ -381,26 +381,26 @@ func (f *FileDialog) effectiveStartingDir() fyne.ListableURI { } - // Try to get ./ - wd, err := os.Getwd() + // Try home dir + dir, err := os.UserHomeDir() if err == nil { - lister, err := storage.ListerForURI(storage.NewURI("file://" + wd)) + lister, err := storage.ListerForURI(storage.NewURI("file://" + dir)) if err == nil { return lister } - fyne.LogError("Could not create lister for working dir", err) + fyne.LogError("Could not create lister for user home dir", err) } + fyne.LogError("Could not load user home dir", err) - // Try home dir - dir, err := os.UserHomeDir() + // Try to get ./ + wd, err := os.Getwd() if err == nil { - lister, err := storage.ListerForURI(storage.NewURI("file://" + dir)) + lister, err := storage.ListerForURI(storage.NewURI("file://" + wd)) if err == nil { return lister } - fyne.LogError("Could not create lister for user home dir", err) + fyne.LogError("Could not create lister for working dir", err) } - fyne.LogError("Could not load user home dir", err) lister, err := storage.ListerForURI(storage.NewURI("file:///")) if err != nil { @@ -533,17 +533,6 @@ func ShowFileOpen(callback func(fyne.URIReadCloser, error), parent fyne.Window) dialog.Show() } -// ShowFileOpenAt works similarly to ShowFileOpen(), but with a custom starting -// location. -func ShowFileOpenAt(callback func(fyne.URIReadCloser, error), parent fyne.Window, startlocation fyne.ListableURI) { - dialog := NewFileOpen(callback, parent) - if fileOpenOSOverride(dialog) { - return - } - dialog.StartingLocation = startlocation - dialog.Show() -} - // ShowFileSave creates and shows a file dialog allowing the user to choose a file to save to (new or overwrite). // If the user chooses an existing file they will be asked if they are sure. // The dialog will appear over the window specified. @@ -555,17 +544,6 @@ func ShowFileSave(callback func(fyne.URIWriteCloser, error), parent fyne.Window) dialog.Show() } -// ShowFileSaveAt works simialrly to ShowFileSave(), but with a custom starting -// location. -func ShowFileSaveAt(callback func(fyne.URIWriteCloser, error), parent fyne.Window, startlocation fyne.ListableURI) { - dialog := NewFileSave(callback, parent) - if fileSaveOSOverride(dialog) { - return - } - dialog.StartingLocation = startlocation - dialog.Show() -} - func makeFavoriteButton(title string, icon fyne.Resource, f func()) *widget.Button { b := widget.NewButtonWithIcon(title, icon, f) From ae3b91f6118c48d8d82215f994510d0a3be7183c Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Thu, 8 Oct 2020 15:59:25 -0400 Subject: [PATCH 27/28] re-enable overwrite callback --- dialog/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dialog/file.go b/dialog/file.go index c78ff6256f..2b023c796d 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -105,7 +105,7 @@ func (f *fileDialog) makeUI() fyne.CanvasObject { func(ok bool) { f.win.Hide() if !ok { - // callback(nil, nil) + callback(nil, nil) return } From 5b5c3a3462720dc9e07b8c3e35c49ead167cecff Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Thu, 8 Oct 2020 16:05:04 -0400 Subject: [PATCH 28/28] update file dialog tests for ~ starting dir --- dialog/file_test.go | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/dialog/file_test.go b/dialog/file_test.go index ab5ddfbd47..afe899e7f8 100644 --- a/dialog/file_test.go +++ b/dialog/file_test.go @@ -47,16 +47,16 @@ func comparePaths(t *testing.T, u1, u2 fyne.ListableURI) bool { func TestEffectiveStartingDir(t *testing.T) { - wdString, err := os.Getwd() + homeString, err := os.UserHomeDir() if err != nil { - t.Skipf("os.Getwd() failed, cannot run this test on this system (error stat()-ing ../) error was '%s'", err) + t.Skipf("os.Gethome() failed, cannot run this test on this system (error stat()-ing ../) error was '%s'", err) } - wd, err := storage.ListerForURI(storage.NewURI("file://" + wdString)) + home, err := storage.ListerForURI(storage.NewURI("file://" + homeString)) if err != nil { t.Skipf("could not get lister for working directory: %s", err) } - parentURI, err := storage.Parent(wd) + parentURI, err := storage.Parent(home) if err != nil { t.Skipf("Could not get parent of working directory: %s", err) } @@ -72,7 +72,7 @@ func TestEffectiveStartingDir(t *testing.T) { // test that we get wd when running with the default struct values res := dialog.effectiveStartingDir() - expect := wd + expect := home if !comparePaths(t, res, expect) { t.Errorf("Expected effectiveStartingDir() to be '%s', but it was '%s'", expect, res) @@ -81,7 +81,7 @@ func TestEffectiveStartingDir(t *testing.T) { // this should always be equivalent to the preceding test dialog.StartingLocation = nil res = dialog.effectiveStartingDir() - expect = wd + expect = home if !comparePaths(t, res, expect) { t.Errorf("Expected effectiveStartingDir() to be '%s', but it was '%s'", expect, res) @@ -102,7 +102,7 @@ func TestEffectiveStartingDir(t *testing.T) { t.Errorf("Should have failed to create lister for nonexistant file") } res = dialog.effectiveStartingDir() - expect = wd + expect = home if res.String() != expect.String() { t.Errorf("Expected effectiveStartingDir() to be '%s', but it was '%s'", expect, res) @@ -184,15 +184,20 @@ func TestShowFileOpen(t *testing.T) { breadcrumb := ui.Objects[3].(*fyne.Container).Objects[0].(*widget.ScrollContainer).Content.(*widget.Box) assert.Greater(t, len(breadcrumb.Children), 0) - wd, err := os.Getwd() + home, err := os.UserHomeDir() if runtime.GOOS == "windows" { - // on windows os.Getwd() returns '\' - wd = strings.ReplaceAll(wd, "\\", "/") + // on windows os.Gethome() returns '\' + home = strings.ReplaceAll(home, "\\", "/") } + t.Logf("home='%s'", home) assert.Nil(t, err) - components := strings.Split(wd, "/") + components := strings.Split(home, "/") + // note that normally when we do a string split, it's going to come up + // "", but we actually want the path bar to show "/". + components[0] = "/" if assert.Equal(t, len(components), len(breadcrumb.Children)) { for i := range components { + t.Logf("i=%d components[i]='%s' breadcrumb...Text[i]='%s'", i, components[i], breadcrumb.Children[i].(*widget.Button).Text) assert.Equal(t, components[i], breadcrumb.Children[i].(*widget.Button).Text) } }