Simplify breadcrumbs generation in file dialog #5958
Conversation
Jacalz
changed the title
|
I don't know if it is just placebo but the file dialog does feel snappier :) |
dialog/file.go
Outdated
| return err | ||
| } | ||
|
|
||
| isDir, err := storage.CanList(dir) |
There was a problem hiding this comment.
I don't know if it's related to the failures but this refactor seems to change behaviour.
This particular line won't add anything because the previous line got a lister, which will error if it cannot list.
I think this code came form inside a for loop so I'm not clear if it wasn't needed or if it was moved to the wrong location.
There was a problem hiding this comment.
Good point about it not being relevant due to the line above. I don't think it should be needed in the for-loop. We can probably assume that paths higher up in the file system tree are listable and if they are not, we will get an error when we try to change to them. Given that we want to show all of the path up to where we are, I think that is fair.
But yeah, this is not related to the test failures. I think I am handling the volume name incorrectly.
Many thanks to @mbaklor for figuring this out. Should unblock fyne-io#5958 failing Windows tests.
|
Windows tests are now passing after the .Name() method of URIs was fixed :) |
3 participants
Description:
This simplifies the logic for creating breadcrumbs a lot. Instead of doing a big allocation when splitting the path and then building the path again for each location, we can just go backwards and get the parent URI until we reach the root. Lastly, we just reverse the slice and boom, bob's your uncle. Also, move some logic around so we exit early. The new code is also less reliant on platform path conventions and can more easily handle non
file://URIs.Checklist: