Support reading .ico icon files if passed to packaging #3008
Conversation
| } | ||
|
|
||
| tmpPath := filepath.Join(os.TempDir(), "fyne-ico-tmp.png") | ||
| out, err := os.Create(tmpPath) |
There was a problem hiding this comment.
Would it be possible to remove the file once it is not necessary anymore?
There was a problem hiding this comment.
This is a good idea.
I don't think we have a hook for the processed completing at this time.
Can you think of a clean way to do that?
There was a problem hiding this comment.
doPackage use defer p.removeBuild(files). Maybe make doPackage and validate return a []string of files that need to be cleaned and set a defer in the caller to those function?
There was a problem hiding this comment.
We could create a folder in the temp directory using https://pkg.go.dev/io/ioutil#TempDir and save everything there when we create temporary files. We then simply make sure to remove the whole folder before exiting the command (we could probably even hook up the interup signal https://www.tutorialspoint.com/how-to-handle-signals-in-golang so that it removes it before exiting using ctrl+c).
|
Just a bit of concern with this new dependencies regarding its license: biessek/golang-ico#3 |
| tmpPath := filepath.Join(os.TempDir(), "fyne-ico-tmp.png") | ||
| out, err := os.Create(tmpPath) |
There was a problem hiding this comment.
How about using ioutil.TempFile() instead?
| tmpPath := filepath.Join(os.TempDir(), "fyne-ico-tmp.png") | |
| out, err := os.Create(tmpPath) | |
| out, err := ioutil.TempFile("", "fyne-ico-tmp.png") |
There was a problem hiding this comment.
I used that originally, but it's not in Go 1.14
There was a problem hiding this comment.
Are you sure about that? It seems to exist in Go 1.14 if you look at this: https://pkg.go.dev/io/[email protected]#TempFile. The os.CreateTemp stuff was added in Go 1.16 but this does the same thing.
There was a problem hiding this comment.
Oh good catch, I must have got the two confused sorry.
| } | ||
|
|
||
| tmpPath := filepath.Join(os.TempDir(), "fyne-ico-tmp.png") | ||
| out, err := os.Create(tmpPath) |
There was a problem hiding this comment.
We could create a folder in the temp directory using https://pkg.go.dev/io/ioutil#TempDir and save everything there when we create temporary files. We then simply make sure to remove the whole folder before exiting the command (we could probably even hook up the interup signal https://www.tutorialspoint.com/how-to-handle-signals-in-golang so that it removes it before exiting using ctrl+c).
|
|
||
| err = png.Encode(out, srcImg) | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to encode icon: %w", err) |
There was a problem hiding this comment.
The variable out isn't closed if we get an error here.
A future work could handle signal kills etc to be even cleaner
There was a problem hiding this comment.
This looks really good. Just one comment on a potentially incorrect defer. I also think it might be good to get this updated once fyne-io/image#1 lands.
|
FYI, there are also conflicts with the base branch that need to be resolved. |
Thanks, fixed |
Re-encode non-png packaging images
Fixes #2412
Checklist:
Where applicable:
go mod vendor).