-
Notifications
You must be signed in to change notification settings - Fork 2
player: Fix Export Current Track and add format option #126
base: master
Are you sure you want to change the base?
Conversation
build.gradle.kts
Outdated
@@ -169,3 +170,11 @@ tasks { | |||
|
|||
println("Java version: ${System.getProperty("java.version")}") | |||
println("Version: $version") | |||
val compileKotlin: KotlinCompile by tasks | |||
compileKotlin.kotlinOptions { | |||
jvmTarget = "1.8" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tasks.with<KotlinCompile>...
JavaVersion.VERSION_1_8.toString()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea what that piece of code is. My IDE apparently added it from nowhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted in 32bff7a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and in 4051b69 added back newline
Files.newBufferedWriter(Settings.PLAYEREXPORTFILE(), | ||
StandardOpenOption.TRUNCATE_EXISTING, | ||
StandardOpenOption.WRITE, | ||
StandardOpenOption.CREATE | ||
).close() | ||
Files.write(Settings.PLAYEREXPORTFILE(), track.toString().toByteArray()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried kotlin's inbuild file writing? Also, I think you can pass the StandardOpenOption
directly to Files.write
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't work on Windows. Seems to be an bug within JVM. I've tried passing StandardOpenOption.TRUNCATE_EXISTING which should in theory rewrite the file completely but it simply writes over it, so writing smaller chunk simply writes one character over the old one.
Default options of Files.write should be TRUNCATE_EXISTING, WRITE, CREATE tho. Don't know why I forcefully put them in the bufferedwriter and not the write.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 9a4e5f5
@@ -114,7 +114,11 @@ object Player: FadingHBox(true, targetHeight = 25) { | |||
fun reset() { | |||
fadeOut() | |||
if(!Files.isDirectory(Settings.PLAYEREXPORTFILE())) { | |||
Files.write(Settings.PLAYEREXPORTFILE(), arrayListOf(""), StandardOpenOption.CREATE) | |||
Files.newBufferedWriter(Settings.PLAYEREXPORTFILE(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a better way to clear it. Why StandardOpenOption.CREATE
? Why not simply delete it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleting it and then trying to recreating it causes security exception on Windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 9a4e5f5
@@ -114,7 +114,11 @@ object Player: FadingHBox(true, targetHeight = 25) { | |||
fun reset() { | |||
fadeOut() | |||
if(!Files.isDirectory(Settings.PLAYEREXPORTFILE())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if it is a directory? Doesn't look like that's handled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why should it need to? If it's a directory, we could show an error message but that would be horrible for the user as it'll show each time a write is made
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But silent failure seems like unexpected behavior as well. How about adding a debug log message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about adding an extension to the file in case it's already a folder?
Changing to draft; going to implement some new feature as well. |
91e319f
to
32bff7a
Compare
Really strange; Java on Windows doesn't clear the file before writing to it. You have to do it manually.