search

n10v / id3v2

🎵 ID3 decoding and encoding library for Go
https://pkg.go.dev/github.com/bogem/id3v2/v2
MIT License
334 stars 50 forks source link

Changes to tag saving to work windows #12

Closed ghost closed 7 years ago

ghost commented 7 years ago

Changed a couple lines in tag.go that are not compatible with windows to be more cross-compatible. Tested on Windows 10 and Ubuntu 16.04

n10v commented 7 years ago

Thanks for PR! I'll merge it, but I have one question: why did you move file closings from defer to the end of the function?

mro commented 7 years ago

I think the cause may be to avoid the rename of open files. A test-case to demontrate the issue (and be able to try on windows) would be nice.

n10v commented 7 years ago

@mro exactly. @forgiv also commented it in source file. Ok, I merge it.

mro commented 7 years ago

maybe keeping the defer would still be nice? https://github.com/mro/id3v2/commit/22f4e86ddd9a6bbac1d1fabaa32d54798bf8da98

The diff mostly is due to indentlevel+=1

n10v commented 7 years ago

@mro but it looks ugly :(

mro commented 7 years ago

@bogem lol, the diff indeed looks horrible due to the increased indentation and lots of whitespace diff. But the result (anon func) is quite nice, don't you think?

https://github.com/mro/id3v2/blob/22f4e86ddd9a6bbac1d1fabaa32d54798bf8da98/tag.go#L232

Edit: I'm not sure my code is correct, so be careful in case. Variables, e.g. originalFile, being assigned inside the closure …)

mro commented 7 years ago

uh, what a garbage. Sorry for the confusion.

n10v commented 7 years ago

@mro Sorry, but I mean the anonymous function looks ugly. It shouldn't be implemented in this way. Also newFile and originalFile are assigned in this anon function, but they are used outside of this function.